| Message ID | 1392138636-29240-8-git-send-email-pawel.moll@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> Use the device platform data as a regmap config > name. This is particularly useful in the regmap > debugfs when there is more than one syscon device > registered, to distinguish the register blocks. > > Cc: Samuel Ortiz <sameo@linux.intel.com> > Cc: Lee Jones <lee.jones@linaro.org> > Signed-off-by: Pawel Moll <pawel.moll@arm.com> > --- > Alternatively I could define a syscon platform data structure, > something like this: > > struct syscon_platform_data { > const char *label; > }; > > drivers/mfd/syscon.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c > index 71841f9..ea1770b 100644 > --- a/drivers/mfd/syscon.c > +++ b/drivers/mfd/syscon.c > @@ -143,6 +143,7 @@ static int syscon_probe(struct platform_device *pdev) > return -ENOMEM; > > syscon_regmap_config.max_register = res->end - res->start - 3; > + syscon_regmap_config.name = dev_get_platdata(&pdev->dev); Perhaps the answer is waiting for me in the other set, but ... Isn't this going to be NULL most of the time? > syscon->regmap = devm_regmap_init_mmio(dev, base, > &syscon_regmap_config); > if (IS_ERR(syscon->regmap)) {
Hello. ???????, 11 ??????? 2014, 17:10 UTC ?? Pawel Moll <pawel.moll@arm.com>: > Use the device platform data as a regmap config > name. This is particularly useful in the regmap > debugfs when there is more than one syscon device > registered, to distinguish the register blocks. > > Cc: Samuel Ortiz <sameo@linux.intel.com> > Cc: Lee Jones <lee.jones@linaro.org> > Signed-off-by: Pawel Moll <pawel.moll@arm.com> > --- ... > syscon_regmap_config.max_register = res->end - res->start - 3; > + syscon_regmap_config.name = dev_get_platdata(&pdev->dev); Is dev_name(&pdev->dev) can be used for such purpose? Thanks. ---
> > Use the device platform data as a regmap config > > name. This is particularly useful in the regmap > > debugfs when there is more than one syscon device > > registered, to distinguish the register blocks. > > > > Cc: Samuel Ortiz <sameo@linux.intel.com> > > Cc: Lee Jones <lee.jones@linaro.org> > > Signed-off-by: Pawel Moll <pawel.moll@arm.com> > > --- > ... > > syscon_regmap_config.max_register = res->end - res->start - 3; > > + syscon_regmap_config.name = dev_get_platdata(&pdev->dev); > > Is dev_name(&pdev->dev) can be used for such purpose? Yes of course. Either use the automatically generated name or over-ride with dev->init_name prior to registration or call dev_set_name() manually. Then retrieve with Alexander's suggestion. Is there any technical reason why this is not possible with your implementation?
On Wed, 2014-02-12 at 08:26 +0000, Lee Jones wrote: > > > Use the device platform data as a regmap config > > > name. This is particularly useful in the regmap > > > debugfs when there is more than one syscon device > > > registered, to distinguish the register blocks. > > > > > > Cc: Samuel Ortiz <sameo@linux.intel.com> > > > Cc: Lee Jones <lee.jones@linaro.org> > > > Signed-off-by: Pawel Moll <pawel.moll@arm.com> > > > --- > > ... > > > syscon_regmap_config.max_register = res->end - res->start - 3; > > > + syscon_regmap_config.name = dev_get_platdata(&pdev->dev); > > > > Is dev_name(&pdev->dev) can be used for such purpose? > > Yes of course. > > Either use the automatically generated name or over-ride with > dev->init_name prior to registration or call dev_set_name() > manually. Then retrieve with Alexander's suggestion. > > Is there any technical reason why this is not possible with your > implementation? Hold on, guys. Let me just point out that we're talking "non-DT" platform devices here (either statically defined struct platform_device-s or - my case - the MFD cells). In this case device/driver matching relies completely on device name. Either the pdev->name must be identical (strcmp) to pdrv->name, or the pdev->name must be identical (strcmp again) to one of the pdev->id_table entries. See platform_match() in driver/base/platform.c for more details. Therefore the dev_name(&pdev->dev) on a non-DT-originating sysconf devince will always return "sysconf.*", unless you're ready to maintain a growing syscon_ids[] list. If so, I will have to add three entries there ("sys_id", "sys_misc" and "sys_procid"). I hope you are not seriously considering this idea :-) After all that's what the platform_data was invented for. Pawel
> > > > Use the device platform data as a regmap config > > > > name. This is particularly useful in the regmap > > > > debugfs when there is more than one syscon device > > > > registered, to distinguish the register blocks. > > > > > > > > Cc: Samuel Ortiz <sameo@linux.intel.com> > > > > Cc: Lee Jones <lee.jones@linaro.org> > > > > Signed-off-by: Pawel Moll <pawel.moll@arm.com> > > > > --- > > > ... > > > > syscon_regmap_config.max_register = res->end - res->start - 3; > > > > + syscon_regmap_config.name = dev_get_platdata(&pdev->dev); > > > > > > Is dev_name(&pdev->dev) can be used for such purpose? > > > > Yes of course. > > > > Either use the automatically generated name or over-ride with > > dev->init_name prior to registration or call dev_set_name() > > manually. Then retrieve with Alexander's suggestion. > > > > Is there any technical reason why this is not possible with your > > implementation? > > Hold on, guys. Let me just point out that we're talking "non-DT" > platform devices here (either statically defined struct > platform_device-s or - my case - the MFD cells). > > In this case device/driver matching relies completely on device name. > Either the pdev->name must be identical (strcmp) to pdrv->name, or the > pdev->name must be identical (strcmp again) to one of the pdev->id_table > entries. See platform_match() in driver/base/platform.c for more > details. > > Therefore the dev_name(&pdev->dev) on a non-DT-originating sysconf > devince will always return "sysconf.*", unless you're ready to maintain > a growing syscon_ids[] list. If so, I will have to add three entries > there ("sys_id", "sys_misc" and "sys_procid"). I hope you are not > seriously considering this idea :-) After all that's what the > platform_data was invented for. Ah, I see your predicament. I guess that is a limitation of the was syscon works. Usually we'd use match tables to differentiate between various supported devices, but I guess the 'supported devices' list for syscon is pretty limitless. In which case I support your second implementation (adding a platform_data container) for the use of arbitrary/useful-common names.
?????, 12 ??????? 2014, 11:06 UTC ?? Pawel Moll <pawel.moll@arm.com>: > On Wed, 2014-02-12 at 08:26 +0000, Lee Jones wrote: > > > > Use the device platform data as a regmap config > > > > name. This is particularly useful in the regmap > > > > debugfs when there is more than one syscon device > > > > registered, to distinguish the register blocks. > > > > > > > > Cc: Samuel Ortiz <sameo@linux.intel.com> > > > > Cc: Lee Jones <lee.jones@linaro.org> > > > > Signed-off-by: Pawel Moll <pawel.moll@arm.com> > > > > --- > > > ... > > > > syscon_regmap_config.max_register = res->end - res->start - 3; > > > > + syscon_regmap_config.name = dev_get_platdata(&pdev->dev); > > > > > > Is dev_name(&pdev->dev) can be used for such purpose? > > > > Yes of course. > > > > Either use the automatically generated name or over-ride with > > dev->init_name prior to registration or call dev_set_name() > > manually. Then retrieve with Alexander's suggestion. > > > > Is there any technical reason why this is not possible with your > > implementation? > > Hold on, guys. Let me just point out that we're talking "non-DT" > platform devices here (either statically defined struct > platform_device-s or - my case - the MFD cells). > > In this case device/driver matching relies completely on device name. > Either the pdev->name must be identical (strcmp) to pdrv->name, or the > pdev->name must be identical (strcmp again) to one of the pdev->id_table > entries. See platform_match() in driver/base/platform.c for more > details. > > Therefore the dev_name(&pdev->dev) on a non-DT-originating sysconf > devince will always return "sysconf.*", unless you're ready to maintain > a growing syscon_ids[] list. If so, I will have to add three entries > there ("sys_id", "sys_misc" and "sys_procid"). I hope you are not > seriously considering this idea :-) After all that's what the > platform_data was invented for. Yeah, I gave up the idea to use the syscon_ids[] to separate devices for non-DT case. https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/mfd/syscon.c?id=5104d2656d4874c51868dc7182016e9501ec99ca Instead, I use a hard definition for pdev->id, so that the names of syscon-devices are different and can be obtained from the driver it uses with syscon_regmap_lookup_by_pdevname(). I understand this topic correct? ---
On Wed, 2014-02-12 at 11:27 +0000, Alexander Shiyan wrote: > Yeah, I gave up the idea to use the syscon_ids[] to separate devices for > non-DT case. > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/mfd/syscon.c?id=5104d2656d4874c51868dc7182016e9501ec99ca > Instead, I use a hard definition for pdev->id, so that the names of > syscon-devices are different and can be obtained from the driver > it uses with syscon_regmap_lookup_by_pdevname(). It is a sort-of-solution (I personally dislike magic numbers, but this is a separate discussion) for a completely-non-DT case, where you can guarantee limited number of syscon devices. In my case I have a DT system, where some of the MFD devices *may* register syscon cells... No way to enforce ordering. > I understand this topic correct? The main idea here is to attach a meaningful label to the syscon regmaps. My example: / # ls -d1 /sys/kernel/debug/regmap/syscon.* /sys/kernel/debug/regmap/syscon.0.auto /sys/kernel/debug/regmap/syscon.4.auto /sys/kernel/debug/regmap/syscon.5.auto vs / # ls -d /sys/kernel/debug/regmap/syscon.* /sys/kernel/debug/regmap/syscon.0.auto-sys_id /sys/kernel/debug/regmap/syscon.4.auto-sys_misc /sys/kernel/debug/regmap/syscon.5.auto-sys_procid Of course one could also define syscon_regmap_lookup_by_label() (I don't really need it right now so didn't go that way) Pawel
diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c index 71841f9..ea1770b 100644 --- a/drivers/mfd/syscon.c +++ b/drivers/mfd/syscon.c @@ -143,6 +143,7 @@ static int syscon_probe(struct platform_device *pdev) return -ENOMEM; syscon_regmap_config.max_register = res->end - res->start - 3; + syscon_regmap_config.name = dev_get_platdata(&pdev->dev); syscon->regmap = devm_regmap_init_mmio(dev, base, &syscon_regmap_config); if (IS_ERR(syscon->regmap)) {
Use the device platform data as a regmap config name. This is particularly useful in the regmap debugfs when there is more than one syscon device registered, to distinguish the register blocks. Cc: Samuel Ortiz <sameo@linux.intel.com> Cc: Lee Jones <lee.jones@linaro.org> Signed-off-by: Pawel Moll <pawel.moll@arm.com> --- Alternatively I could define a syscon platform data structure, something like this: struct syscon_platform_data { const char *label; }; drivers/mfd/syscon.c | 1 + 1 file changed, 1 insertion(+)