* [PATCH] soc: ti: ti_sci_pm_domains: Store device id in platform device @ 2019-09-23 3:34 Lokesh Vutla 2019-09-23 6:37 ` Tero Kristo 0 siblings, 1 reply; 5+ messages in thread From: Lokesh Vutla @ 2019-09-23 3:34 UTC (permalink / raw) To: Nishanth Menon, Tero Kristo, Santosh Shilimkar Cc: Lokesh Vutla, Sekhar Nori, Linux ARM Mailing List Device ID that is passed from power-domains is used by peripheral drivers for communicating with sysfw. Instead of individual drivers traversing power-domains entry in DT node, store the device ID in platform_device so that drivers can directly access it. Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> --- drivers/soc/ti/ti_sci_pm_domains.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/soc/ti/ti_sci_pm_domains.c b/drivers/soc/ti/ti_sci_pm_domains.c index 8c2a2f23982c..a124ac409124 100644 --- a/drivers/soc/ti/ti_sci_pm_domains.c +++ b/drivers/soc/ti/ti_sci_pm_domains.c @@ -116,6 +116,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain *domain, struct of_phandle_args pd_args; struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(domain); const struct ti_sci_handle *ti_sci = ti_sci_genpd->ti_sci; + struct platform_device *pdev = to_platform_device(dev); struct ti_sci_genpd_dev_data *sci_dev_data; struct generic_pm_domain_data *genpd_data; int idx, ret = 0; @@ -129,6 +130,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain *domain, return -EINVAL; idx = pd_args.args[0]; + pdev->id = idx; /* * Check the validity of the requested idx, if the index is not valid -- 2.22.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] soc: ti: ti_sci_pm_domains: Store device id in platform device 2019-09-23 3:34 [PATCH] soc: ti: ti_sci_pm_domains: Store device id in platform device Lokesh Vutla @ 2019-09-23 6:37 ` Tero Kristo 2019-09-24 4:45 ` Lokesh Vutla 0 siblings, 1 reply; 5+ messages in thread From: Tero Kristo @ 2019-09-23 6:37 UTC (permalink / raw) To: Lokesh Vutla, Nishanth Menon, Santosh Shilimkar Cc: Sekhar Nori, Linux ARM Mailing List On 23/09/2019 06:34, Lokesh Vutla wrote: > Device ID that is passed from power-domains is used by peripheral > drivers for communicating with sysfw. Instead of individual drivers > traversing power-domains entry in DT node, store the device ID in > platform_device so that drivers can directly access it. Uhm, isn't this kind of wrong place to allocate the ID? The power domain solution itself is a client also. In theory, someone could access the pdev->id before this. pdev->id should be assigned by bus driver so that it can be properly handled within platform_device_add. -Tero > > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > --- > drivers/soc/ti/ti_sci_pm_domains.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/soc/ti/ti_sci_pm_domains.c b/drivers/soc/ti/ti_sci_pm_domains.c > index 8c2a2f23982c..a124ac409124 100644 > --- a/drivers/soc/ti/ti_sci_pm_domains.c > +++ b/drivers/soc/ti/ti_sci_pm_domains.c > @@ -116,6 +116,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain *domain, > struct of_phandle_args pd_args; > struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(domain); > const struct ti_sci_handle *ti_sci = ti_sci_genpd->ti_sci; > + struct platform_device *pdev = to_platform_device(dev); > struct ti_sci_genpd_dev_data *sci_dev_data; > struct generic_pm_domain_data *genpd_data; > int idx, ret = 0; > @@ -129,6 +130,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain *domain, > return -EINVAL; > > idx = pd_args.args[0]; > + pdev->id = idx; > > /* > * Check the validity of the requested idx, if the index is not valid > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] soc: ti: ti_sci_pm_domains: Store device id in platform device 2019-09-23 6:37 ` Tero Kristo @ 2019-09-24 4:45 ` Lokesh Vutla 2019-09-26 3:33 ` Lokesh Vutla 0 siblings, 1 reply; 5+ messages in thread From: Lokesh Vutla @ 2019-09-24 4:45 UTC (permalink / raw) To: Tero Kristo, Nishanth Menon, Santosh Shilimkar Cc: Sekhar Nori, Linux ARM Mailing List Hi Tero, On 23/09/19 12:07 PM, Tero Kristo wrote: > On 23/09/2019 06:34, Lokesh Vutla wrote: >> Device ID that is passed from power-domains is used by peripheral >> drivers for communicating with sysfw. Instead of individual drivers >> traversing power-domains entry in DT node, store the device ID in >> platform_device so that drivers can directly access it. > > Uhm, isn't this kind of wrong place to allocate the ID? The power domain I do agree that this might not be a right place, but I couldn't find a better place to populate id. Below is the flow on how platform_device gets created. of_platform_default_populate_init ->of_platform_default_populate -> of_platform_populate ->of_platform_bus_create -> of_platform_device_create_pdata -> of_device_alloc -> platform_device_alloc("", PLATFORM_DEVID_NONE); At this point platform_device gets created with dummy device_id. Also there are no hooks to add custom device_ids. > solution itself is a client also. In theory, someone could access the pdev->id Nope, this is done in dev_pm_domain_attach which is called before driver probe in platform_drv_probe(). > before this. pdev->id should be assigned by bus driver so that it can be > properly handled within platform_device_add. DT doesn't provide any such facility for populating device_add. I am open for any suggestions :) Thanks and regards, Lokesh > > -Tero > >> >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> >> --- >> drivers/soc/ti/ti_sci_pm_domains.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/soc/ti/ti_sci_pm_domains.c >> b/drivers/soc/ti/ti_sci_pm_domains.c >> index 8c2a2f23982c..a124ac409124 100644 >> --- a/drivers/soc/ti/ti_sci_pm_domains.c >> +++ b/drivers/soc/ti/ti_sci_pm_domains.c >> @@ -116,6 +116,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain >> *domain, >> struct of_phandle_args pd_args; >> struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(domain); >> const struct ti_sci_handle *ti_sci = ti_sci_genpd->ti_sci; >> + struct platform_device *pdev = to_platform_device(dev); >> struct ti_sci_genpd_dev_data *sci_dev_data; >> struct generic_pm_domain_data *genpd_data; >> int idx, ret = 0; >> @@ -129,6 +130,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain >> *domain, >> return -EINVAL; >> idx = pd_args.args[0]; >> + pdev->id = idx; >> /* >> * Check the validity of the requested idx, if the index is not valid >> > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] soc: ti: ti_sci_pm_domains: Store device id in platform device 2019-09-24 4:45 ` Lokesh Vutla @ 2019-09-26 3:33 ` Lokesh Vutla 2019-09-26 6:20 ` Tero Kristo 0 siblings, 1 reply; 5+ messages in thread From: Lokesh Vutla @ 2019-09-26 3:33 UTC (permalink / raw) To: Tero Kristo, Nishanth Menon, Santosh Shilimkar Cc: Sekhar Nori, Linux ARM Mailing List Hi Tero, On 24/09/19 10:15 AM, Lokesh Vutla wrote: > Hi Tero, > > On 23/09/19 12:07 PM, Tero Kristo wrote: >> On 23/09/2019 06:34, Lokesh Vutla wrote: >>> Device ID that is passed from power-domains is used by peripheral >>> drivers for communicating with sysfw. Instead of individual drivers >>> traversing power-domains entry in DT node, store the device ID in >>> platform_device so that drivers can directly access it. >> >> Uhm, isn't this kind of wrong place to allocate the ID? The power domain > > I do agree that this might not be a right place, but I couldn't find a better > place to populate id. Below is the flow on how platform_device gets created. > of_platform_default_populate_init > ->of_platform_default_populate > -> of_platform_populate > ->of_platform_bus_create > -> of_platform_device_create_pdata > -> of_device_alloc > -> platform_device_alloc("", PLATFORM_DEVID_NONE); > > At this point platform_device gets created with dummy device_id. Also there are > no hooks to add custom device_ids. > >> solution itself is a client also. In theory, someone could access the pdev->id > > Nope, this is done in dev_pm_domain_attach which is called before driver probe > in platform_drv_probe(). If there are no objections, can this patch be picked? Thanks and regards, Lokesh > >> before this. pdev->id should be assigned by bus driver so that it can be >> properly handled within platform_device_add. > > DT doesn't provide any such facility for populating device_add. I am open for > any suggestions :) > > Thanks and regards, > Lokesh > >> >> -Tero >> >>> >>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> >>> --- >>> drivers/soc/ti/ti_sci_pm_domains.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/soc/ti/ti_sci_pm_domains.c >>> b/drivers/soc/ti/ti_sci_pm_domains.c >>> index 8c2a2f23982c..a124ac409124 100644 >>> --- a/drivers/soc/ti/ti_sci_pm_domains.c >>> +++ b/drivers/soc/ti/ti_sci_pm_domains.c >>> @@ -116,6 +116,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain >>> *domain, >>> struct of_phandle_args pd_args; >>> struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(domain); >>> const struct ti_sci_handle *ti_sci = ti_sci_genpd->ti_sci; >>> + struct platform_device *pdev = to_platform_device(dev); >>> struct ti_sci_genpd_dev_data *sci_dev_data; >>> struct generic_pm_domain_data *genpd_data; >>> int idx, ret = 0; >>> @@ -129,6 +130,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain >>> *domain, >>> return -EINVAL; >>> idx = pd_args.args[0]; >>> + pdev->id = idx; >>> /* >>> * Check the validity of the requested idx, if the index is not valid >>> >> >> -- >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] soc: ti: ti_sci_pm_domains: Store device id in platform device 2019-09-26 3:33 ` Lokesh Vutla @ 2019-09-26 6:20 ` Tero Kristo 0 siblings, 0 replies; 5+ messages in thread From: Tero Kristo @ 2019-09-26 6:20 UTC (permalink / raw) To: Lokesh Vutla, Nishanth Menon, Santosh Shilimkar Cc: Sekhar Nori, Linux ARM Mailing List On 26/09/2019 06:33, Lokesh Vutla wrote: > Hi Tero, > > On 24/09/19 10:15 AM, Lokesh Vutla wrote: >> Hi Tero, >> >> On 23/09/19 12:07 PM, Tero Kristo wrote: >>> On 23/09/2019 06:34, Lokesh Vutla wrote: >>>> Device ID that is passed from power-domains is used by peripheral >>>> drivers for communicating with sysfw. Instead of individual drivers >>>> traversing power-domains entry in DT node, store the device ID in >>>> platform_device so that drivers can directly access it. >>> >>> Uhm, isn't this kind of wrong place to allocate the ID? The power domain >> >> I do agree that this might not be a right place, but I couldn't find a better >> place to populate id. Below is the flow on how platform_device gets created. >> of_platform_default_populate_init >> ->of_platform_default_populate >> -> of_platform_populate >> ->of_platform_bus_create >> -> of_platform_device_create_pdata >> -> of_device_alloc >> -> platform_device_alloc("", PLATFORM_DEVID_NONE); >> >> At this point platform_device gets created with dummy device_id. Also there are >> no hooks to add custom device_ids. >> >>> solution itself is a client also. In theory, someone could access the pdev->id >> >> Nope, this is done in dev_pm_domain_attach which is called before driver probe >> in platform_drv_probe(). > > If there are no objections, can this patch be picked? I don't think this patch is still quite right, as it is clearly a hack (you modify a platform device field outside the chain that actually allocates it and uses it for some purpose.) The issue I see here is really easy potential breakage if the pdev->id is changed to be used something in the OF platform device chain. This hack would then break as it is completely TI K3 specific, and if any drivers depend on it, they would break also. So, IMHO it is a NAK for this patch from my side. It is too hackish, and there is a way to handle this from drivers currently (yes, the alternative is bit more painful but it is certain to work going forward also.) -Tero > > Thanks and regards, > Lokesh > >> >>> before this. pdev->id should be assigned by bus driver so that it can be >>> properly handled within platform_device_add. >> >> DT doesn't provide any such facility for populating device_add. I am open for >> any suggestions :) >> >> Thanks and regards, >> Lokesh >> >>> >>> -Tero >>> >>>> >>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> >>>> --- >>>> drivers/soc/ti/ti_sci_pm_domains.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/soc/ti/ti_sci_pm_domains.c >>>> b/drivers/soc/ti/ti_sci_pm_domains.c >>>> index 8c2a2f23982c..a124ac409124 100644 >>>> --- a/drivers/soc/ti/ti_sci_pm_domains.c >>>> +++ b/drivers/soc/ti/ti_sci_pm_domains.c >>>> @@ -116,6 +116,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain >>>> *domain, >>>> struct of_phandle_args pd_args; >>>> struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(domain); >>>> const struct ti_sci_handle *ti_sci = ti_sci_genpd->ti_sci; >>>> + struct platform_device *pdev = to_platform_device(dev); >>>> struct ti_sci_genpd_dev_data *sci_dev_data; >>>> struct generic_pm_domain_data *genpd_data; >>>> int idx, ret = 0; >>>> @@ -129,6 +130,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain >>>> *domain, >>>> return -EINVAL; >>>> idx = pd_args.args[0]; >>>> + pdev->id = idx; >>>> /* >>>> * Check the validity of the requested idx, if the index is not valid >>>> >>> >>> -- >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-26 6:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-23 3:34 [PATCH] soc: ti: ti_sci_pm_domains: Store device id in platform device Lokesh Vutla 2019-09-23 6:37 ` Tero Kristo 2019-09-24 4:45 ` Lokesh Vutla 2019-09-26 3:33 ` Lokesh Vutla 2019-09-26 6:20 ` Tero Kristo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).