linux-kernel - Re: [PATCH] typec: tcpm: Provide of_node pointer as part of psy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180522094719.GH10677@kuha.fi.intel.com>
Date:   Tue, 22 May 2018 12:47:19 +0300
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Adam Thomson <Adam.Thomson.Opensource@...semi.com>
Cc:     Guenter Roeck <linux@...ck-us.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Support Opensource <Support.Opensource@...semi.com>
Subject: Re: [PATCH] typec: tcpm: Provide of_node pointer as part of psy_cfg

On Mon, May 21, 2018 at 03:34:53PM +0000, Adam Thomson wrote:
> On 21 May 2018 15:56, Heikki Krogerus wrote:
> 
> > On Mon, May 21, 2018 at 01:58:16PM +0000, Adam Thomson wrote:
> > > Hi Heikki,
> > >
> > > On 21 May 2018 14:20, Heikki Krogerus wrote:
> > >
> > > > On Wed, May 16, 2018 at 05:00:46PM +0100, Adam Thomson wrote:
> > > > > For supply registration, provide of_node pointer of the port device,
> > > > > via the power_supply_config structure, to allow other psy drivers
> > > > > to add us as a supplier using the 'power-supplies' DT property.
> > > > >
> > > > > Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@...semi.com>
> > > > > ---
> > > > >  drivers/usb/typec/tcpm.c | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> > > > > index 72996cc..e7c0b95 100644
> > > > > --- a/drivers/usb/typec/tcpm.c
> > > > > +++ b/drivers/usb/typec/tcpm.c
> > > > > @@ -4500,6 +4500,7 @@ static int devm_tcpm_psy_register(struct tcpm_port
> > > > *port)
> > > > >  	char *psy_name;
> > > > >
> > > > >  	psy_cfg.drv_data = port;
> > > > > +	psy_cfg.of_node = port->dev->of_node;
> > > > >  	psy_name = devm_kzalloc(port->dev, psy_name_len, GFP_KERNEL);
> > > > >  	if (!psy_name)
> > > > >  		return -ENOMEM;
> > > >
> > > > Would it be possible to use fwnode here instead? It would mean that
> > > > you add a member for it to the struct power_supply_config, and handle
> > > > it separately in power_supply_core.c. You could just convert it to
> > > > of_node there for now.
> > > >
> > > > That is just a request, I'm fine with this, but it would prepare this
> > > > driver for all types of platforms, so less patching would be needed
> > > > once we add ACPI support to the power_supply_core.c.
> > >
> > > Would the following commit from Hans, already present in power_supply_core.c,
> > > not fit the bill:
> > >
> > > [58a36bb06891ee779074db6ef84e98347c634d38]
> > > power: supply: core: Add support for supplied-from device-property
> > >
> > > Or was that just meant as a stop gap for something more?
> > 
> > I think the main idea with that patch was that it allows us to take
> > advantage of build-in device properties, but I think we could actually
> > improve also it if we had the fwnode handle assigned to the psy. We
> > would not have to assume the parent has those device properties then.
> > 
> > But ACPI actually defines a specific object called _PCL (power
> > consumer list) for power source objects that we should one day check
> > in power_supply_core.c. That's what I meant by ACPI support.
> 
> Ok, fair enough. Makes sense to me. Thanks for clarifying.
> 
> > > I have no problems adding something further, but I don't have a means to verify
> > > anything ACPI based, beyond a simple build test, so would ideally want someone
> > > to verify that path through the code.
> > 
> > This is not really about ACPI. By using fwnode handle instead of
> > of_node, we would continue to keep tcpm.c agnostic about the type of
> > hw description.
> > 
> > So I don't expect any ACPI support to be added to the
> > power_supply_code.c at this point. You can use to_of_node(cfg->fwnode)
> > in __power_supply_register() for now:
> > 
> >         psy->of_node = cfg->of_node;
> >         if (cfg->fwnode)
> >                 psy->of_node = to_of_node(cfg->fwnode);
> 
> OK, no problem. Will take a look and add that in.

Thanks Adam!

-- 
heikki