| Message ID | 20180516160046.18FCF3FAD3@swsrvapps-01.diasemi.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Adam, 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@diasemi.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. Thanks,
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@diasemi.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 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. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@diasemi.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. > 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); Thanks,
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@diasemi.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. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@diasemi.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!
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;
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@diasemi.com> --- drivers/usb/typec/tcpm.c | 1 + 1 file changed, 1 insertion(+)