Re: [PATCH] base: soc: Export soc_device_to_device() helper

Re: [PATCH] base: soc: Export soc_device_to_device() helper

[Andreas Färber]

Am 11.11.19 um 07:40 schrieb Greg Kroah-Hartman:
> On Mon, Nov 11, 2019 at 06:42:05AM +0100, Andreas Färber wrote:
>> Hi Greg,
>>
>> Am 11.11.19 um 06:27 schrieb Greg Kroah-Hartman:
>>> On Mon, Nov 11, 2019 at 05:56:09AM +0100, Andreas Färber wrote:
>>>> Use of soc_device_to_device() in driver modules causes a build failure.
>>>> Given that the helper is nicely documented in include/linux/sys_soc.h,
>>>> let's export it as GPL symbol.
>>>
>>> I thought we were fixing the soc drivers to not need this.  What
>>> happened to that effort?  I thought I had patches in my tree (or
>>> someone's tree) that did some of this work already, such that this
>>> symbol isn't needed anymore.
>>
>> I do still see this function used in next-20191108 in drivers/soc/.
>>
>> I'll be happy to adjust my RFC driver if someone points me to how!
> 
> Look at c31e73121f4c ("base: soc: Handle custom soc information sysfs
> entries") for how you can just use the default attributes for the soc to
> create the needed sysfs files, instead of having to do it "by hand"
> which is racy and incorrect.

Unrelated.

>> Given the current struct layout, a type cast might work (but ugly).
>> Or if we stay with my current RFC driver design, we could use the
>> platform_device instead of the soc_device (which would clutter the
>> screen more than "soc soc0:") or resort to pr_info() w/o device.
> 
> Ick, no, don't cast blindly.  What do you need the pointer for?  Is this
> for in-tree code?

No, an RFC patchset: https://patchwork.kernel.org/cover/11224261/

As I indicated above, I used it for a dev_info(), which I can easily
avoid by using pr_info() instead:

diff --git a/drivers/soc/realtek/chip.c b/drivers/soc/realtek/chip.c
index e5078c6731fd..f9380e831659 100644
--- a/drivers/soc/realtek/chip.c
+++ b/drivers/soc/realtek/chip.c
@@ -178,8 +178,7 @@ static int rtd_soc_probe(struct platform_device *pdev)

        platform_set_drvdata(pdev, soc_dev);

-       dev_info(soc_device_to_device(soc_dev),
-               "%s %s (0x%08x) rev %s (0x%08x) detected\n",
+       pr_info("%s %s (0x%08x) rev %s (0x%08x) detected\n",
                soc_dev_attr->family, soc_dev_attr->soc_id, chip_id,
                soc_dev_attr->revision, chip_rev);


But as I said, there is still in-tree code using this helper:


arch/arm/mach-ep93xx/core.c:    return soc_device_to_device(soc_dev);

Returned from ep93xx_init_soc(), which is passed through by
ep93xx_init_devices():

arch/arm/mach-ep93xx/adssphere.c:       ep93xx_init_devices();
arch/arm/mach-ep93xx/edb93xx.c: ep93xx_init_devices();
arch/arm/mach-ep93xx/gesbc9312.c:       ep93xx_init_devices();
arch/arm/mach-ep93xx/micro9.c:  ep93xx_init_devices();
arch/arm/mach-ep93xx/simone.c:  ep93xx_init_devices();
arch/arm/mach-ep93xx/snappercl15.c:     ep93xx_init_devices();
arch/arm/mach-ep93xx/ts72xx.c:  ep93xx_init_devices();
arch/arm/mach-ep93xx/ts72xx.c:  ep93xx_init_devices();
arch/arm/mach-ep93xx/vision_ep9307.c:   ep93xx_init_devices();

Return value unused everywhere.


arch/arm/mach-imx/cpu.c:        return soc_device_to_device(soc_dev);

Used as return value of imx_soc_device_init():

arch/arm/mach-imx/mach-imx6q.c: parent = imx_soc_device_init();
arch/arm/mach-imx/mach-imx6sl.c:        parent = imx_soc_device_init();
arch/arm/mach-imx/mach-imx6sx.c:        parent = imx_soc_device_init();
arch/arm/mach-imx/mach-imx6ul.c:        parent = imx_soc_device_init();

These do a NULL check and pass it to of_platform_default_populate().

arch/arm/mach-imx/mach-imx7d.c: parent = imx_soc_device_init();

This one only does a NULL check.

arch/arm/mach-imx/mach-imx7ulp.c:
of_platform_default_populate(NULL, NULL, imx_soc_device_init());

Speaks for itself.


arch/arm/mach-mxs/mach-mxs.c:   parent = soc_device_to_device(soc_dev);

Passed to of_platform_default_populate().


arch/arm/mach-omap2/id.c:       parent = soc_device_to_device(soc_dev);

Used for device_create_file().


arch/arm/mach-zynq/common.c:    parent = soc_device_to_device(soc_dev);

Passed to of_platform_default_populate().


drivers/soc/amlogic/meson-gx-socinfo.c: dev = soc_device_to_device(soc_dev);

Used for dev_info(). CONFIG_MESON_GX_SOCINFO is bool, thus not affected.


drivers/soc/amlogic/meson-mx-socinfo.c:
dev_info(soc_device_to_device(soc_dev), "Amlogic %s %s detected\n",

Speaks for itself. CONFIG_MESON_MX_SOCINFO is bool, thus not affected.


drivers/soc/tegra/fuse/fuse-tegra.c:    return soc_device_to_device(dev);

Returned from tegra_soc_device_register(). For arm64 NULL-checked only,
but also used for arm in arch/arm/mach-tegra/tegra.c:tegra_dt_init(),
which passes it to of_platform_default_populate().


drivers/soc/ux500/ux500-soc-id.c:       parent =
soc_device_to_device(soc_dev);

Used for device_create_file().


drivers/soc/versatile/soc-integrator.c: dev = soc_device_to_device(soc_dev);

Used for device_create_file().

drivers/soc/versatile/soc-realview.c:
device_create_file(soc_device_to_device(soc_dev), &realview_manf_attr);
drivers/soc/versatile/soc-realview.c:
device_create_file(soc_device_to_device(soc_dev), &realview_board_attr);
drivers/soc/versatile/soc-realview.c:
device_create_file(soc_device_to_device(soc_dev), &realview_arch_attr);
drivers/soc/versatile/soc-realview.c:
device_create_file(soc_device_to_device(soc_dev), &realview_build_attr);

Speaks for itself.


So, not counting my unmerged Realtek driver,
* we have two cases of struct device being used for dev_info(), which
could be cleaned up with device-less pr_info(), I could post a patch,
* frequent usage in arm/mach-* for of_platform_default_populate(), this
seems most difficult to replace if we neither want to cast nor expose
the struct,
* some simply unused, which could be refactored to return void, and
* some for device_create_file(), which could probably be avoided with
custom_attr_group.

It also raises the question of whether new arm platforms such as RTD1195
(mach-realtek) should attempt to use of_platform_default_populate() with
the soc_device somehow, or if not, whether those platforms should be
refactored to consistently no longer do so?

I believe in the Broken Window Theory, i.e. fixing what we can before
mistakes get copied and propagate further in code; but here I don't see
a perspective for getting rid of soc_device_to_device() completely to
prevent new usages, nor can I test all of those platforms myself.

Has a cleanup based on custom_attr_group been attempted already and is
waiting on patches to get reviewed and merged through maintainer trees,
or do we need to prepare new cleanup patches here? A search for
"soc_device_to_device" on LAKML Patchwork shows only this patch of mine.

Thanks,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] base: soc: Export soc_device_to_device() helper

[Andreas Färber]

Am 11.11.19 um 21:10 schrieb Andreas Färber:
> Am 11.11.19 um 07:40 schrieb Greg Kroah-Hartman:
>> On Mon, Nov 11, 2019 at 06:42:05AM +0100, Andreas Färber wrote:
>>> Hi Greg,
>>>
>>> Am 11.11.19 um 06:27 schrieb Greg Kroah-Hartman:
>>>> On Mon, Nov 11, 2019 at 05:56:09AM +0100, Andreas Färber wrote:
>>>>> Use of soc_device_to_device() in driver modules causes a build failure.
>>>>> Given that the helper is nicely documented in include/linux/sys_soc.h,
>>>>> let's export it as GPL symbol.
>>>>
>>>> I thought we were fixing the soc drivers to not need this.  What
>>>> happened to that effort?  I thought I had patches in my tree (or
>>>> someone's tree) that did some of this work already, such that this
>>>> symbol isn't needed anymore.
>>>
>>> I do still see this function used in next-20191108 in drivers/soc/.
>>>
>>> I'll be happy to adjust my RFC driver if someone points me to how!
>>
>> Look at c31e73121f4c ("base: soc: Handle custom soc information sysfs
>> entries") for how you can just use the default attributes for the soc to
>> create the needed sysfs files, instead of having to do it "by hand"
>> which is racy and incorrect.
> 
> Unrelated.
> 
>>> Given the current struct layout, a type cast might work (but ugly).
>>> Or if we stay with my current RFC driver design, we could use the
>>> platform_device instead of the soc_device (which would clutter the
>>> screen more than "soc soc0:") or resort to pr_info() w/o device.
>>
>> Ick, no, don't cast blindly.  What do you need the pointer for?  Is this
>> for in-tree code?
> 
> No, an RFC patchset: https://patchwork.kernel.org/cover/11224261/
> 
> As I indicated above, I used it for a dev_info(), which I can easily
> avoid by using pr_info() instead:
> 
> diff --git a/drivers/soc/realtek/chip.c b/drivers/soc/realtek/chip.c
> index e5078c6731fd..f9380e831659 100644
> --- a/drivers/soc/realtek/chip.c
> +++ b/drivers/soc/realtek/chip.c
> @@ -178,8 +178,7 @@ static int rtd_soc_probe(struct platform_device *pdev)
> 
>         platform_set_drvdata(pdev, soc_dev);
> 
> -       dev_info(soc_device_to_device(soc_dev),
> -               "%s %s (0x%08x) rev %s (0x%08x) detected\n",
> +       pr_info("%s %s (0x%08x) rev %s (0x%08x) detected\n",
>                 soc_dev_attr->family, soc_dev_attr->soc_id, chip_id,
>                 soc_dev_attr->revision, chip_rev);
> 

Tested and squashed in my tree.

> 
> But as I said, there is still in-tree code using this helper:
[snip]
> So, not counting my unmerged Realtek driver,
> * we have two cases of struct device being used for dev_info(), which
> could be cleaned up with device-less pr_info(), I could post a patch,

Patch sent: https://patchwork.kernel.org/patch/11237949/ (untested)

> * frequent usage in arm/mach-* for of_platform_default_populate(), this
> seems most difficult to replace if we neither want to cast nor expose
> the struct,

One clever way might be to implement a new of_soc_default_populate() in
drivers/base/soc.c that takes a soc_device instead of device, doing the
conversion inside soc.c and calling of_platform_default_populate() from
there. Then we could convert present users to pass around soc_device
instead of device, with a perspective to make soc_device_to_device()
static inside base/soc.c.

sys_soc.h does not presently #include any OF headers, so the declaration
may need to go into of_platform.h and to consider CONFIG_SOC_BUS.

Will require compile-testing for each platform and ideally some kbuild
bot passes to get right, so not a quick shot.

While at it, an of_soc_device_register() variant could fill
soc_device_attribute::machine in common code instead of each platform
duplicating to read this from the DT root node's model property.

> * some simply unused, which could be refactored to return void, and

Patch sent: https://patchwork.kernel.org/patch/11237971/ (untested)

> * some for device_create_file(), which could probably be avoided with
> custom_attr_group.
> 
> It also raises the question of whether new arm platforms such as RTD1195
> (mach-realtek) should attempt to use of_platform_default_populate() with
> the soc_device somehow, or if not, whether those platforms should be
> refactored to consistently no longer do so?
> 
> I believe in the Broken Window Theory, i.e. fixing what we can before
> mistakes get copied and propagate further in code; but here I don't see
> a perspective for getting rid of soc_device_to_device() completely to
> prevent new usages, nor can I test all of those platforms myself.
> 
> Has a cleanup based on custom_attr_group been attempted already and is
> waiting on patches to get reviewed and merged through maintainer trees,
> or do we need to prepare new cleanup patches here? A search for
> "soc_device_to_device" on LAKML Patchwork shows only this patch of mine.

Actually I don't find a single user of custom_attr_group in linux-next,
which may be because the patch introducing it is from October and people
are waiting on the next -rc1 before they can merge patches using it?

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] base: soc: Export soc_device_to_device() helper

[Greg Kroah-Hartman]

On Mon, Nov 11, 2019 at 09:10:41PM +0100, Andreas Färber wrote:
> Am 11.11.19 um 07:40 schrieb Greg Kroah-Hartman:
> > On Mon, Nov 11, 2019 at 06:42:05AM +0100, Andreas Färber wrote:
> >> Hi Greg,
> >>
> >> Am 11.11.19 um 06:27 schrieb Greg Kroah-Hartman:
> >>> On Mon, Nov 11, 2019 at 05:56:09AM +0100, Andreas Färber wrote:
> >>>> Use of soc_device_to_device() in driver modules causes a build failure.
> >>>> Given that the helper is nicely documented in include/linux/sys_soc.h,
> >>>> let's export it as GPL symbol.
> >>>
> >>> I thought we were fixing the soc drivers to not need this.  What
> >>> happened to that effort?  I thought I had patches in my tree (or
> >>> someone's tree) that did some of this work already, such that this
> >>> symbol isn't needed anymore.
> >>
> >> I do still see this function used in next-20191108 in drivers/soc/.
> >>
> >> I'll be happy to adjust my RFC driver if someone points me to how!
> > 
> > Look at c31e73121f4c ("base: soc: Handle custom soc information sysfs
> > entries") for how you can just use the default attributes for the soc to
> > create the needed sysfs files, instead of having to do it "by hand"
> > which is racy and incorrect.
> 
> Unrelated.
> 
> >> Given the current struct layout, a type cast might work (but ugly).
> >> Or if we stay with my current RFC driver design, we could use the
> >> platform_device instead of the soc_device (which would clutter the
> >> screen more than "soc soc0:") or resort to pr_info() w/o device.
> > 
> > Ick, no, don't cast blindly.  What do you need the pointer for?  Is this
> > for in-tree code?
> 
> No, an RFC patchset: https://patchwork.kernel.org/cover/11224261/
> 
> As I indicated above, I used it for a dev_info(), which I can easily
> avoid by using pr_info() instead:
> 
> diff --git a/drivers/soc/realtek/chip.c b/drivers/soc/realtek/chip.c
> index e5078c6731fd..f9380e831659 100644
> --- a/drivers/soc/realtek/chip.c
> +++ b/drivers/soc/realtek/chip.c
> @@ -178,8 +178,7 @@ static int rtd_soc_probe(struct platform_device *pdev)
> 
>         platform_set_drvdata(pdev, soc_dev);
> 
> -       dev_info(soc_device_to_device(soc_dev),
> -               "%s %s (0x%08x) rev %s (0x%08x) detected\n",
> +       pr_info("%s %s (0x%08x) rev %s (0x%08x) detected\n",
>                 soc_dev_attr->family, soc_dev_attr->soc_id, chip_id,
>                 soc_dev_attr->revision, chip_rev);

First off, the driver should not be spitting out noise for when all goes
well like this :)

Anyway, just use the pdev pointer here, not the soc dev as that is NOT a
pointer you have control over (as is evident by the fact that you need
this call to try to get it.)

I'll look at your other instances later when I've had my coffee...

thanks,

greg k-h

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] base: soc: Export soc_device_to_device() helper

[Uwe Kleine-König]

On Tue, Nov 12, 2019 at 06:23:47AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Nov 11, 2019 at 09:10:41PM +0100, Andreas Färber wrote:
> > Am 11.11.19 um 07:40 schrieb Greg Kroah-Hartman:
> > > On Mon, Nov 11, 2019 at 06:42:05AM +0100, Andreas Färber wrote:
> > >> Hi Greg,
> > >>
> > >> Am 11.11.19 um 06:27 schrieb Greg Kroah-Hartman:
> > >>> On Mon, Nov 11, 2019 at 05:56:09AM +0100, Andreas Färber wrote:
> > >>>> Use of soc_device_to_device() in driver modules causes a build failure.
> > >>>> Given that the helper is nicely documented in include/linux/sys_soc.h,
> > >>>> let's export it as GPL symbol.
> > >>>
> > >>> I thought we were fixing the soc drivers to not need this.  What
> > >>> happened to that effort?  I thought I had patches in my tree (or
> > >>> someone's tree) that did some of this work already, such that this
> > >>> symbol isn't needed anymore.
> > >>
> > >> I do still see this function used in next-20191108 in drivers/soc/.
> > >>
> > >> I'll be happy to adjust my RFC driver if someone points me to how!
> > > 
> > > Look at c31e73121f4c ("base: soc: Handle custom soc information sysfs
> > > entries") for how you can just use the default attributes for the soc to
> > > create the needed sysfs files, instead of having to do it "by hand"
> > > which is racy and incorrect.
> > 
> > Unrelated.
> > 
> > >> Given the current struct layout, a type cast might work (but ugly).
> > >> Or if we stay with my current RFC driver design, we could use the
> > >> platform_device instead of the soc_device (which would clutter the
> > >> screen more than "soc soc0:") or resort to pr_info() w/o device.
> > > 
> > > Ick, no, don't cast blindly.  What do you need the pointer for?  Is this
> > > for in-tree code?
> > 
> > No, an RFC patchset: https://patchwork.kernel.org/cover/11224261/
> > 
> > As I indicated above, I used it for a dev_info(), which I can easily
> > avoid by using pr_info() instead:
> > 
> > diff --git a/drivers/soc/realtek/chip.c b/drivers/soc/realtek/chip.c
> > index e5078c6731fd..f9380e831659 100644
> > --- a/drivers/soc/realtek/chip.c
> > +++ b/drivers/soc/realtek/chip.c
> > @@ -178,8 +178,7 @@ static int rtd_soc_probe(struct platform_device *pdev)
> > 
> >         platform_set_drvdata(pdev, soc_dev);
> > 
> > -       dev_info(soc_device_to_device(soc_dev),
> > -               "%s %s (0x%08x) rev %s (0x%08x) detected\n",
> > +       pr_info("%s %s (0x%08x) rev %s (0x%08x) detected\n",
> >                 soc_dev_attr->family, soc_dev_attr->soc_id, chip_id,
> >                 soc_dev_attr->revision, chip_rev);
> 
> First off, the driver should not be spitting out noise for when all goes
> well like this :)

I didn't follow the discussion closely, but I think I want to object
here a bit. While I agree that each driver emitting some stuff to the
log buffer is hardly helpful, seeing the exact SoC details is indeed
useful at times. With my Debian kernel team member hat on, I'd say
keep this information. This way the SoC details make it into kernel bug
reports without effort on our side.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Sense of soc bus? (was: [PATCH] base: soc: Export soc_device_to_device() helper)

[Andreas Färber]

Am 12.11.19 um 08:29 schrieb Uwe Kleine-König:
> On Tue, Nov 12, 2019 at 06:23:47AM +0100, Greg Kroah-Hartman wrote:
>> On Mon, Nov 11, 2019 at 09:10:41PM +0100, Andreas Färber wrote:
>>> Am 11.11.19 um 07:40 schrieb Greg Kroah-Hartman:
>>>> On Mon, Nov 11, 2019 at 06:42:05AM +0100, Andreas Färber wrote:
>>>>> Am 11.11.19 um 06:27 schrieb Greg Kroah-Hartman:
>>>>>> On Mon, Nov 11, 2019 at 05:56:09AM +0100, Andreas Färber wrote:
>>>>>>> Use of soc_device_to_device() in driver modules causes a build failure.
>>>>>>> Given that the helper is nicely documented in include/linux/sys_soc.h,
>>>>>>> let's export it as GPL symbol.
>>>>>>
>>>>>> I thought we were fixing the soc drivers to not need this.  What
>>>>>> happened to that effort?  I thought I had patches in my tree (or
>>>>>> someone's tree) that did some of this work already, such that this
>>>>>> symbol isn't needed anymore.
>>>>>
>>>>> I do still see this function used in next-20191108 in drivers/soc/.
>>>>>
>>>>> I'll be happy to adjust my RFC driver if someone points me to how!
>>>>
>>>> Look at c31e73121f4c ("base: soc: Handle custom soc information sysfs
>>>> entries") for how you can just use the default attributes for the soc to
>>>> create the needed sysfs files, instead of having to do it "by hand"
>>>> which is racy and incorrect.
>>>
>>> Unrelated.
>>>
>>>>> Given the current struct layout, a type cast might work (but ugly).
>>>>> Or if we stay with my current RFC driver design, we could use the
>>>>> platform_device instead of the soc_device (which would clutter the
>>>>> screen more than "soc soc0:") or resort to pr_info() w/o device.
>>>>
>>>> Ick, no, don't cast blindly.  What do you need the pointer for?  Is this
>>>> for in-tree code?
>>>
>>> No, an RFC patchset: https://patchwork.kernel.org/cover/11224261/
>>>
>>> As I indicated above, I used it for a dev_info(), which I can easily
>>> avoid by using pr_info() instead:
>>>
>>> diff --git a/drivers/soc/realtek/chip.c b/drivers/soc/realtek/chip.c
>>> index e5078c6731fd..f9380e831659 100644
>>> --- a/drivers/soc/realtek/chip.c
>>> +++ b/drivers/soc/realtek/chip.c
>>> @@ -178,8 +178,7 @@ static int rtd_soc_probe(struct platform_device *pdev)
>>>
>>>         platform_set_drvdata(pdev, soc_dev);
>>>
>>> -       dev_info(soc_device_to_device(soc_dev),
>>> -               "%s %s (0x%08x) rev %s (0x%08x) detected\n",
>>> +       pr_info("%s %s (0x%08x) rev %s (0x%08x) detected\n",
>>>                 soc_dev_attr->family, soc_dev_attr->soc_id, chip_id,
>>>                 soc_dev_attr->revision, chip_rev);
>>
>> First off, the driver should not be spitting out noise for when all goes
>> well like this :)
> 
> I didn't follow the discussion closely, but I think I want to object
> here a bit. While I agree that each driver emitting some stuff to the
> log buffer is hardly helpful, seeing the exact SoC details is indeed
> useful at times. With my Debian kernel team member hat on, I'd say
> keep this information. This way the SoC details make it into kernel bug
> reports without effort on our side.

Seconded. For example, RTD1295 will support LSADC only from revision B00
on (and it's not the first time I'm seeing such things in the industry).
So if a user complains, it will be helpful to see that information.

Referencing your Amlogic review, with all due respect for its authors,
the common framework here just lets that information evaporate into the
deeps of sysfs. People who know can check /sys/bus/soc/devices/soc0, but
ordinary users will at most upload dmesg output to a Bugzilla ticket.

And it was precisely info-level boot output from the Amlogic GX driver
that made me aware of this common framework and inspired me to later
contribute such a driver elsewhere myself. That's not a bad effect. ;)

So if anything, we should standardize that output and move it into
soc_device_register(): "<family> <soc_id> [rev <revision>] detected"
with suitable NULL checks? (what I did above for Realtek, minus hex)

The info level seems correct to me - it allows people to use a different
log_level if they only care about warnings or errors on the console;
it's not debug info for that driver, except in my case the accompanying
hex numbers that I'd be happy to drop in favor of standardization.

Another aspect here is that the overall amount of soc_device_register()
users is just an estimated one third above the analysis shared before.
In particular server platforms, be it arm64 or x86_64, that potentially
have more than one SoC integrated in a multi-socket configuration, don't
feed into this soc bus at all! Therefore my comment that
dev_info()-printed "soc soc0:" is kind of useless if there's only one
SoC on those boards. I'm not aware of any tool or a more common file
aggregating this information, certainly not /proc/cpuinfo and I'm not
aware of any special "lssoc" tool either. And if there's no ACPI/DMI
driver feeding support-relevant information into this framework to be
generally useful, I don't expect the big distros to spend time on
improving its usability.

So if we move info output into base/soc.c, we could continue using
dev_info() with "soc"-grep'able prefix in the hopes that someday we'll
have more than one soc device on the bus and will need to distinguish;
otherwise yes, pr_info() would change the output format for Amlogic (and
so would a harmonization of the text), but does anyone really care in
practice? Tools shouldn't be relying on its output format, and humans
will understand equally either way.

Finally, arch/arm seems unique in that it has the machine_desc mechanism
that allows individual SoCs to force creating their soc_device early and
using it as parent for further DT-created platform_devices. With arm64
we've lost that ability, and nios2 is not using it either.
A bad side effect (with SUSE hat on) is that this parent design pattern
does not allow to build such drivers as modules, which means that distro
configs using arm's multi-platform, e.g., CONFIG_ARCH_MULTI_V7 will get
unnecessary code creep as new platforms get added over time (beyond the
basic clk, pinctrl, tty and maybe timer).
Even if it were possible to call into a driver module that early, using
it as parent would seem to imply that all the references by its children
would not allow to unload the module, which I'd consider a flawed design
for such an "optional" read-once driver. If we want the device hierarchy
to have a soc root then most DT based platforms will have a /soc DT node
anyway (although no device_type = "soc") that we probably could have a
device registered for in common code rather than each SoC platform
handling that differently? That might then make soc_register_device()
not the creator of the device (if pre-existent) but the supplier of data
to that core device, which should then allow to unload the data provider
with just the sysfs data disappearing until re-inserted (speeding up the
develop-and-test cycle on say not-so-resource-constrained platforms).

On the other hand, one might argue that such information should just be
parsed by EBBR-conformant bootloaders and be passed to the kernel via
standard UEFI interfaces and DMI tables. But I'm not aware of Barebox
having implemented any of that yet, and even for U-Boot (e.g., Realtek
based consumer devices...) not everyone has the GPL sources or tools to
update their bootloader. So, having the kernel we control gather
information relevant to kernel developers does make some sense to me.

Thoughts?

Regards,
Andreas

P.S. Sorry that a seemingly trivial one-line 0-day fix derailed into
this fundamental use case discussion.

arch/arm/mach-ep93xx/core.c:    soc_dev = soc_device_register(soc_dev_attr);
arch/arm/mach-imx/cpu.c:        soc_dev = soc_device_register(soc_dev_attr);
arch/arm/mach-mvebu/mvebu-soc-id.c:     soc_dev =
soc_device_register(soc_dev_attr);
arch/arm/mach-mxs/mach-mxs.c:   soc_dev = soc_device_register(soc_dev_attr);
arch/arm/mach-omap2/id.c:       soc_dev = soc_device_register(soc_dev_attr);
arch/arm/mach-tegra/tegra.c:    struct device *parent =
tegra_soc_device_register();
arch/arm/mach-zynq/common.c:    soc_dev = soc_device_register(soc_dev_attr);
arch/nios2/platform/platform.c:         soc_dev =
soc_device_register(soc_dev_attr);
drivers/soc/amlogic/meson-gx-socinfo.c: soc_dev =
soc_device_register(soc_dev_attr);
drivers/soc/amlogic/meson-mx-socinfo.c: soc_dev =
soc_device_register(soc_dev_attr);
drivers/soc/atmel/soc.c:        soc_dev = soc_device_register(soc_dev_attr);
drivers/soc/bcm/brcmstb/common.c:       soc_dev =
soc_device_register(soc_dev_attr);
drivers/soc/fsl/guts.c: soc_dev = soc_device_register(&soc_dev_attr);
drivers/soc/imx/soc-imx-scu.c:  soc_dev = soc_device_register(soc_dev_attr);
drivers/soc/imx/soc-imx8.c:     soc_dev = soc_device_register(soc_dev_attr);
drivers/soc/qcom/socinfo.c:     qs->soc_dev =
soc_device_register(&qs->attr);
drivers/soc/realtek/chip.c:     soc_dev = soc_device_register(soc_dev_attr);
drivers/soc/renesas/renesas-soc.c:      soc_dev =
soc_device_register(soc_dev_attr);
drivers/soc/samsung/exynos-chipid.c:    soc_dev =
soc_device_register(soc_dev_attr);
drivers/soc/tegra/fuse/fuse-tegra.c:    dev = soc_device_register(attr);
drivers/soc/ux500/ux500-soc-id.c:       soc_dev =
soc_device_register(soc_dev_attr);
drivers/soc/versatile/soc-integrator.c: soc_dev =
soc_device_register(soc_dev_attr);
drivers/soc/versatile/soc-realview.c:   soc_dev =
soc_device_register(soc_dev_attr);

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Sense of soc bus? (was: [PATCH] base: soc: Export soc_device_to_device() helper)

[Rob Herring]

On Tue, Nov 12, 2019 at 4:47 AM Andreas Färber <afaerber@suse.de> wrote:
>
> Am 12.11.19 um 08:29 schrieb Uwe Kleine-König:
> > On Tue, Nov 12, 2019 at 06:23:47AM +0100, Greg Kroah-Hartman wrote:
> >> On Mon, Nov 11, 2019 at 09:10:41PM +0100, Andreas Färber wrote:
> >>> Am 11.11.19 um 07:40 schrieb Greg Kroah-Hartman:
> >>>> On Mon, Nov 11, 2019 at 06:42:05AM +0100, Andreas Färber wrote:
> >>>>> Am 11.11.19 um 06:27 schrieb Greg Kroah-Hartman:
> >>>>>> On Mon, Nov 11, 2019 at 05:56:09AM +0100, Andreas Färber wrote:
> >>>>>>> Use of soc_device_to_device() in driver modules causes a build failure.
> >>>>>>> Given that the helper is nicely documented in include/linux/sys_soc.h,
> >>>>>>> let's export it as GPL symbol.
> >>>>>>
> >>>>>> I thought we were fixing the soc drivers to not need this.  What
> >>>>>> happened to that effort?  I thought I had patches in my tree (or
> >>>>>> someone's tree) that did some of this work already, such that this
> >>>>>> symbol isn't needed anymore.
> >>>>>
> >>>>> I do still see this function used in next-20191108 in drivers/soc/.
> >>>>>
> >>>>> I'll be happy to adjust my RFC driver if someone points me to how!
> >>>>
> >>>> Look at c31e73121f4c ("base: soc: Handle custom soc information sysfs
> >>>> entries") for how you can just use the default attributes for the soc to
> >>>> create the needed sysfs files, instead of having to do it "by hand"
> >>>> which is racy and incorrect.
> >>>
> >>> Unrelated.
> >>>
> >>>>> Given the current struct layout, a type cast might work (but ugly).
> >>>>> Or if we stay with my current RFC driver design, we could use the
> >>>>> platform_device instead of the soc_device (which would clutter the
> >>>>> screen more than "soc soc0:") or resort to pr_info() w/o device.
> >>>>
> >>>> Ick, no, don't cast blindly.  What do you need the pointer for?  Is this
> >>>> for in-tree code?
> >>>
> >>> No, an RFC patchset: https://patchwork.kernel.org/cover/11224261/
> >>>
> >>> As I indicated above, I used it for a dev_info(), which I can easily
> >>> avoid by using pr_info() instead:
> >>>
> >>> diff --git a/drivers/soc/realtek/chip.c b/drivers/soc/realtek/chip.c
> >>> index e5078c6731fd..f9380e831659 100644
> >>> --- a/drivers/soc/realtek/chip.c
> >>> +++ b/drivers/soc/realtek/chip.c
> >>> @@ -178,8 +178,7 @@ static int rtd_soc_probe(struct platform_device *pdev)
> >>>
> >>>         platform_set_drvdata(pdev, soc_dev);
> >>>
> >>> -       dev_info(soc_device_to_device(soc_dev),
> >>> -               "%s %s (0x%08x) rev %s (0x%08x) detected\n",
> >>> +       pr_info("%s %s (0x%08x) rev %s (0x%08x) detected\n",
> >>>                 soc_dev_attr->family, soc_dev_attr->soc_id, chip_id,
> >>>                 soc_dev_attr->revision, chip_rev);
> >>
> >> First off, the driver should not be spitting out noise for when all goes
> >> well like this :)
> >
> > I didn't follow the discussion closely, but I think I want to object
> > here a bit. While I agree that each driver emitting some stuff to the
> > log buffer is hardly helpful, seeing the exact SoC details is indeed
> > useful at times. With my Debian kernel team member hat on, I'd say
> > keep this information. This way the SoC details make it into kernel bug
> > reports without effort on our side.
>
> Seconded. For example, RTD1295 will support LSADC only from revision B00
> on (and it's not the first time I'm seeing such things in the industry).
> So if a user complains, it will be helpful to see that information.
>
> Referencing your Amlogic review, with all due respect for its authors,
> the common framework here just lets that information evaporate into the
> deeps of sysfs. People who know can check /sys/bus/soc/devices/soc0, but
> ordinary users will at most upload dmesg output to a Bugzilla ticket.
>
> And it was precisely info-level boot output from the Amlogic GX driver
> that made me aware of this common framework and inspired me to later
> contribute such a driver elsewhere myself. That's not a bad effect. ;)
>
> So if anything, we should standardize that output and move it into
> soc_device_register(): "<family> <soc_id> [rev <revision>] detected"
> with suitable NULL checks? (what I did above for Realtek, minus hex)
>
> The info level seems correct to me - it allows people to use a different
> log_level if they only care about warnings or errors on the console;
> it's not debug info for that driver, except in my case the accompanying
> hex numbers that I'd be happy to drop in favor of standardization.
>
> Another aspect here is that the overall amount of soc_device_register()
> users is just an estimated one third above the analysis shared before.
> In particular server platforms, be it arm64 or x86_64, that potentially
> have more than one SoC integrated in a multi-socket configuration, don't
> feed into this soc bus at all! Therefore my comment that
> dev_info()-printed "soc soc0:" is kind of useless if there's only one
> SoC on those boards. I'm not aware of any tool or a more common file
> aggregating this information, certainly not /proc/cpuinfo and I'm not
> aware of any special "lssoc" tool either. And if there's no ACPI/DMI
> driver feeding support-relevant information into this framework to be
> generally useful, I don't expect the big distros to spend time on
> improving its usability.

lshw? That works with info from DT, sysfs, and DMI. It did have some
endian bugs (written for sparc/power) last time I looked at it 5+
years ago.

> So if we move info output into base/soc.c, we could continue using
> dev_info() with "soc"-grep'able prefix in the hopes that someday we'll
> have more than one soc device on the bus and will need to distinguish;
> otherwise yes, pr_info() would change the output format for Amlogic (and
> so would a harmonization of the text), but does anyone really care in
> practice? Tools shouldn't be relying on its output format, and humans
> will understand equally either way.
>
> Finally, arch/arm seems unique in that it has the machine_desc mechanism
> that allows individual SoCs to force creating their soc_device early and
> using it as parent for further DT-created platform_devices. With arm64
> we've lost that ability, and nios2 is not using it either.
> A bad side effect (with SUSE hat on) is that this parent design pattern
> does not allow to build such drivers as modules, which means that distro
> configs using arm's multi-platform, e.g., CONFIG_ARCH_MULTI_V7 will get
> unnecessary code creep as new platforms get added over time (beyond the
> basic clk, pinctrl, tty and maybe timer).
> Even if it were possible to call into a driver module that early, using
> it as parent would seem to imply that all the references by its children
> would not allow to unload the module, which I'd consider a flawed design
> for such an "optional" read-once driver. If we want the device hierarchy
> to have a soc root then most DT based platforms will have a /soc DT node
> anyway (although no device_type = "soc") that we probably could have a
> device registered for in common code rather than each SoC platform
> handling that differently? That might then make soc_register_device()
> not the creator of the device (if pre-existent) but the supplier of data
> to that core device, which should then allow to unload the data provider
> with just the sysfs data disappearing until re-inserted (speeding up the
> develop-and-test cycle on say not-so-resource-constrained platforms).

I for one would like to for this to be consistent. Either we always
have an SoC device parent or never. I wouldn't decide based on having
a DT node or not either. Generally, we should always have MMIO devices
under a bus node and perhaps more than one, but that doesn't always
happen. I think building the drivers as modules is a good reason to do
away with the parent device.

It would also allow getting rid of remaining
of_platform_default_populate calls in arm platforms except for auxdata
cases. Pretty much that's the ones you list below in arch/arm/.

> On the other hand, one might argue that such information should just be
> parsed by EBBR-conformant bootloaders and be passed to the kernel via
> standard UEFI interfaces and DMI tables. But I'm not aware of Barebox
> having implemented any of that yet, and even for U-Boot (e.g., Realtek
> based consumer devices...) not everyone has the GPL sources or tools to
> update their bootloader. So, having the kernel we control gather
> information relevant to kernel developers does make some sense to me.

UEFI and DMI are orthogonal, right. You can't expect DMI on a UEFI+DT system.

Rob

>
> Thoughts?
>
> Regards,
> Andreas
>
> P.S. Sorry that a seemingly trivial one-line 0-day fix derailed into
> this fundamental use case discussion.
>
> arch/arm/mach-ep93xx/core.c:    soc_dev = soc_device_register(soc_dev_attr);
> arch/arm/mach-imx/cpu.c:        soc_dev = soc_device_register(soc_dev_attr);
> arch/arm/mach-mvebu/mvebu-soc-id.c:     soc_dev =
> soc_device_register(soc_dev_attr);
> arch/arm/mach-mxs/mach-mxs.c:   soc_dev = soc_device_register(soc_dev_attr);
> arch/arm/mach-omap2/id.c:       soc_dev = soc_device_register(soc_dev_attr);
> arch/arm/mach-tegra/tegra.c:    struct device *parent =
> tegra_soc_device_register();
> arch/arm/mach-zynq/common.c:    soc_dev = soc_device_register(soc_dev_attr);
> arch/nios2/platform/platform.c:         soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/amlogic/meson-gx-socinfo.c: soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/amlogic/meson-mx-socinfo.c: soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/atmel/soc.c:        soc_dev = soc_device_register(soc_dev_attr);
> drivers/soc/bcm/brcmstb/common.c:       soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/fsl/guts.c: soc_dev = soc_device_register(&soc_dev_attr);
> drivers/soc/imx/soc-imx-scu.c:  soc_dev = soc_device_register(soc_dev_attr);
> drivers/soc/imx/soc-imx8.c:     soc_dev = soc_device_register(soc_dev_attr);
> drivers/soc/qcom/socinfo.c:     qs->soc_dev =
> soc_device_register(&qs->attr);
> drivers/soc/realtek/chip.c:     soc_dev = soc_device_register(soc_dev_attr);
> drivers/soc/renesas/renesas-soc.c:      soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/samsung/exynos-chipid.c:    soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/tegra/fuse/fuse-tegra.c:    dev = soc_device_register(attr);
> drivers/soc/ux500/ux500-soc-id.c:       soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/versatile/soc-integrator.c: soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/versatile/soc-realview.c:   soc_dev =
> soc_device_register(soc_dev_attr);
>
> --
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer
> HRB 36809 (AG Nürnberg)

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Sense of soc bus? (was: [PATCH] base: soc: Export soc_device_to_device() helper)

[Andreas Färber]

Am 14.11.19 um 23:09 schrieb Rob Herring:
> On Tue, Nov 12, 2019 at 4:47 AM Andreas Färber <afaerber@suse.de> wrote:
>> On the other hand, one might argue that such information should just be
>> parsed by EBBR-conformant bootloaders and be passed to the kernel via
>> standard UEFI interfaces and DMI tables. But I'm not aware of Barebox
>> having implemented any of that yet, and even for U-Boot (e.g., Realtek
>> based consumer devices...) not everyone has the GPL sources or tools to
>> update their bootloader. So, having the kernel we control gather
>> information relevant to kernel developers does make some sense to me.
> 
> UEFI and DMI are orthogonal, right. You can't expect DMI on a UEFI+DT system.

Not sure, that's why I CC'ed the EBBR folks for input. If it's not
mandatory today, the next revision of EBBR could just require it - if
that's the unified way between SBBR and EBBR. Little point in doing it
only for EBBR.

On my UEFI+DT based Raspberry Pi 3 Model B I do see it, note the
non-filled Processor Information delivered by U-Boot:

raspi3:~ # dmidecode
# dmidecode 3.2
Getting SMBIOS data from sysfs.
SMBIOS 3.0 present.
7 structures occupying 253 bytes.
Table at 0x3CB3E020.

Handle 0x0000, DMI type 0, 24 bytes
BIOS Information
	Vendor: U-Boot
	Version: 2019.10
	Release Date: 10/26/2019
	ROM Size: 64 kB
	Characteristics:
		PCI is supported
		BIOS is upgradeable
		Selectable boot is supported
		I2O boot is supported
		Targeted content distribution is supported

Handle 0x0001, DMI type 1, 27 bytes
System Information
	Manufacturer: raspberrypi
	Product Name: rpi
	Version: Not Specified
	Serial Number: 00000000********
	UUID: 30303030-3030-3030-6437-623461336666
	Wake-up Type: Reserved
	SKU Number: Not Specified
	Family: Not Specified

Handle 0x0002, DMI type 2, 14 bytes
Base Board Information
	Manufacturer: raspberrypi
	Product Name: rpi
	Version: Not Specified
	Serial Number: Not Specified
	Asset Tag: Not Specified
	Features:
		Board is a hosting board
	Location In Chassis: Not Specified
	Chassis Handle: 0x0000
	Type: Motherboard

Handle 0x0003, DMI type 3, 21 bytes
Chassis Information
	Manufacturer: raspberrypi
	Type: Desktop
	Lock: Not Present
	Version: Not Specified
	Serial Number: Not Specified
	Asset Tag: Not Specified
	Boot-up State: Safe
	Power Supply State: Safe
	Thermal State: Safe
	Security Status: None
	OEM Information: 0x00000000
	Height: Unspecified
	Number Of Power Cords: Unspecified
	Contained Elements: 0

Handle 0x0004, DMI type 4, 48 bytes
Processor Information
	Socket Designation: Not Specified
	Type: Central Processor
	Family: Unknown
	Manufacturer: Unknown
	ID: 00 00 00 00 00 00 00 00
	Version: Unknown
	Voltage: Unknown
	External Clock: Unknown
	Max Speed: Unknown
	Current Speed: Unknown
	Status: Unpopulated
	Upgrade: None
	L1 Cache Handle: Not Provided
	L2 Cache Handle: Not Provided
	L3 Cache Handle: Not Provided
	Serial Number: Not Specified
	Asset Tag: Not Specified
	Part Number: Not Specified
	Characteristics: None

Handle 0x0005, DMI type 32, 11 bytes
System Boot Information
	Status: No errors detected

Handle 0x0006, DMI type 127, 4 bytes
End Of Table


Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Sense of soc bus? (was: [PATCH] base: soc: Export soc_device_to_device() helper)

[Andreas Färber]

Am 14.11.19 um 23:09 schrieb Rob Herring:
> On Tue, Nov 12, 2019 at 4:47 AM Andreas Färber <afaerber@suse.de> wrote:
>> Finally, arch/arm seems unique in that it has the machine_desc mechanism
>> that allows individual SoCs to force creating their soc_device early and
>> using it as parent for further DT-created platform_devices. With arm64
>> we've lost that ability, and nios2 is not using it either.
>> A bad side effect (with SUSE hat on) is that this parent design pattern
>> does not allow to build such drivers as modules, which means that distro
>> configs using arm's multi-platform, e.g., CONFIG_ARCH_MULTI_V7 will get
>> unnecessary code creep as new platforms get added over time (beyond the
>> basic clk, pinctrl, tty and maybe timer).
>> Even if it were possible to call into a driver module that early, using
>> it as parent would seem to imply that all the references by its children
>> would not allow to unload the module, which I'd consider a flawed design
>> for such an "optional" read-once driver. If we want the device hierarchy
>> to have a soc root then most DT based platforms will have a /soc DT node
>> anyway (although no device_type = "soc") that we probably could have a
>> device registered for in common code rather than each SoC platform
>> handling that differently? That might then make soc_register_device()
>> not the creator of the device (if pre-existent) but the supplier of data
>> to that core device, which should then allow to unload the data provider
>> with just the sysfs data disappearing until re-inserted (speeding up the
>> develop-and-test cycle on say not-so-resource-constrained platforms).
> 
> I for one would like to for this to be consistent. Either we always
> have an SoC device parent or never. I wouldn't decide based on having
> a DT node or not either.

Sure, if we can always be consistent, that might be best.

Where I was coming from here is that, if we're not supposed to use
soc_device_to_device(), then we have no way to associate an of_node with
the soc_device from the outside (and nobody was doing it today, as per
my analysis). We'd either need a new helper of_soc_device_register()
with additional argument for the node, or it would need to be done
entirely in the infrastructure as I suggested, be it by looking for one
hardcoded /soc node or for nodes with device_type = "soc".

Rob, in light of this discussion, should we start adding the latter
property for new DTs such as my new Realtek SoCs, or was there a reason
this has not been used consistently outside of powerpc and nios2?
Intel/Altera appear to be the only two in arm64, with only three more in
arm, none in mips.

(BTW my assumption was that we don't follow the booting-without-of.txt
documented schema of soc<SOCname> so that we can share .dtsi across
differently named SoCs, is that correct?)

> Generally, we should always have MMIO devices
> under a bus node and perhaps more than one, but that doesn't always
> happen. I think building the drivers as modules is a good reason to do
> away with the parent device.
> 
> It would also allow getting rid of remaining
> of_platform_default_populate calls in arm platforms except for auxdata
> cases. Pretty much that's the ones you list below in arch/arm/.

The majority was indeed passing in NULL, so yeah, we might clean that
up, if someone could come up with a plan of where/how to implement it.

Cheers,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Sense of soc bus? (was: [PATCH] base: soc: Export soc_device_to_device() helper)

[Neil Armstrong]

On 12/11/2019 11:47, Andreas Färber wrote:
> Am 12.11.19 um 08:29 schrieb Uwe Kleine-König:
>> On Tue, Nov 12, 2019 at 06:23:47AM +0100, Greg Kroah-Hartman wrote:
>>> On Mon, Nov 11, 2019 at 09:10:41PM +0100, Andreas Färber wrote:
>>>> Am 11.11.19 um 07:40 schrieb Greg Kroah-Hartman:
>>>>> On Mon, Nov 11, 2019 at 06:42:05AM +0100, Andreas Färber wrote:
>>>>>> Am 11.11.19 um 06:27 schrieb Greg Kroah-Hartman:
>>>>>>> On Mon, Nov 11, 2019 at 05:56:09AM +0100, Andreas Färber wrote:
>>>>>>>> Use of soc_device_to_device() in driver modules causes a build failure.
>>>>>>>> Given that the helper is nicely documented in include/linux/sys_soc.h,
>>>>>>>> let's export it as GPL symbol.
>>>>>>>
>>>>>>> I thought we were fixing the soc drivers to not need this.  What
>>>>>>> happened to that effort?  I thought I had patches in my tree (or
>>>>>>> someone's tree) that did some of this work already, such that this
>>>>>>> symbol isn't needed anymore.
>>>>>>
>>>>>> I do still see this function used in next-20191108 in drivers/soc/.
>>>>>>
>>>>>> I'll be happy to adjust my RFC driver if someone points me to how!
>>>>>
>>>>> Look at c31e73121f4c ("base: soc: Handle custom soc information sysfs
>>>>> entries") for how you can just use the default attributes for the soc to
>>>>> create the needed sysfs files, instead of having to do it "by hand"
>>>>> which is racy and incorrect.
>>>>
>>>> Unrelated.
>>>>
>>>>>> Given the current struct layout, a type cast might work (but ugly).
>>>>>> Or if we stay with my current RFC driver design, we could use the
>>>>>> platform_device instead of the soc_device (which would clutter the
>>>>>> screen more than "soc soc0:") or resort to pr_info() w/o device.
>>>>>
>>>>> Ick, no, don't cast blindly.  What do you need the pointer for?  Is this
>>>>> for in-tree code?
>>>>
>>>> No, an RFC patchset: https://patchwork.kernel.org/cover/11224261/
>>>>
>>>> As I indicated above, I used it for a dev_info(), which I can easily
>>>> avoid by using pr_info() instead:
>>>>
>>>> diff --git a/drivers/soc/realtek/chip.c b/drivers/soc/realtek/chip.c
>>>> index e5078c6731fd..f9380e831659 100644
>>>> --- a/drivers/soc/realtek/chip.c
>>>> +++ b/drivers/soc/realtek/chip.c
>>>> @@ -178,8 +178,7 @@ static int rtd_soc_probe(struct platform_device *pdev)
>>>>
>>>>         platform_set_drvdata(pdev, soc_dev);
>>>>
>>>> -       dev_info(soc_device_to_device(soc_dev),
>>>> -               "%s %s (0x%08x) rev %s (0x%08x) detected\n",
>>>> +       pr_info("%s %s (0x%08x) rev %s (0x%08x) detected\n",
>>>>                 soc_dev_attr->family, soc_dev_attr->soc_id, chip_id,
>>>>                 soc_dev_attr->revision, chip_rev);
>>>
>>> First off, the driver should not be spitting out noise for when all goes
>>> well like this :)
>>
>> I didn't follow the discussion closely, but I think I want to object
>> here a bit. While I agree that each driver emitting some stuff to the
>> log buffer is hardly helpful, seeing the exact SoC details is indeed
>> useful at times. With my Debian kernel team member hat on, I'd say
>> keep this information. This way the SoC details make it into kernel bug
>> reports without effort on our side.
> 
> Seconded. For example, RTD1295 will support LSADC only from revision B00
> on (and it's not the first time I'm seeing such things in the industry).
> So if a user complains, it will be helpful to see that information.
> 
> Referencing your Amlogic review, with all due respect for its authors,
> the common framework here just lets that information evaporate into the
> deeps of sysfs. 

Hopefully we never had the case where needed to use the soc info in drivers,
but now we have one and having such infrastructure already in-place will help.

Renesas platforms makes a extensive usage of the soc info infrastructure to
figure out plenty of HW parameters at runtime and lower their DT changes.

Neil

> People who know can check /sys/bus/soc/devices/soc0, but
> ordinary users will at most upload dmesg output to a Bugzilla ticket.
> 
> And it was precisely info-level boot output from the Amlogic GX driver
> that made me aware of this common framework and inspired me to later
> contribute such a driver elsewhere myself. That's not a bad effect. ;)
> 
> So if anything, we should standardize that output and move it into
> soc_device_register(): "<family> <soc_id> [rev <revision>] detected"
> with suitable NULL checks? (what I did above for Realtek, minus hex)
> 
> The info level seems correct to me - it allows people to use a different
> log_level if they only care about warnings or errors on the console;
> it's not debug info for that driver, except in my case the accompanying
> hex numbers that I'd be happy to drop in favor of standardization.
> 
> Another aspect here is that the overall amount of soc_device_register()
> users is just an estimated one third above the analysis shared before.
> In particular server platforms, be it arm64 or x86_64, that potentially
> have more than one SoC integrated in a multi-socket configuration, don't
> feed into this soc bus at all! Therefore my comment that
> dev_info()-printed "soc soc0:" is kind of useless if there's only one
> SoC on those boards. I'm not aware of any tool or a more common file
> aggregating this information, certainly not /proc/cpuinfo and I'm not
> aware of any special "lssoc" tool either. And if there's no ACPI/DMI
> driver feeding support-relevant information into this framework to be
> generally useful, I don't expect the big distros to spend time on
> improving its usability.
> 
> So if we move info output into base/soc.c, we could continue using
> dev_info() with "soc"-grep'able prefix in the hopes that someday we'll
> have more than one soc device on the bus and will need to distinguish;
> otherwise yes, pr_info() would change the output format for Amlogic (and
> so would a harmonization of the text), but does anyone really care in
> practice? Tools shouldn't be relying on its output format, and humans
> will understand equally either way.
> 
> Finally, arch/arm seems unique in that it has the machine_desc mechanism
> that allows individual SoCs to force creating their soc_device early and
> using it as parent for further DT-created platform_devices. With arm64
> we've lost that ability, and nios2 is not using it either.
> A bad side effect (with SUSE hat on) is that this parent design pattern
> does not allow to build such drivers as modules, which means that distro
> configs using arm's multi-platform, e.g., CONFIG_ARCH_MULTI_V7 will get
> unnecessary code creep as new platforms get added over time (beyond the
> basic clk, pinctrl, tty and maybe timer).
> Even if it were possible to call into a driver module that early, using
> it as parent would seem to imply that all the references by its children
> would not allow to unload the module, which I'd consider a flawed design
> for such an "optional" read-once driver. If we want the device hierarchy
> to have a soc root then most DT based platforms will have a /soc DT node
> anyway (although no device_type = "soc") that we probably could have a
> device registered for in common code rather than each SoC platform
> handling that differently? That might then make soc_register_device()
> not the creator of the device (if pre-existent) but the supplier of data
> to that core device, which should then allow to unload the data provider
> with just the sysfs data disappearing until re-inserted (speeding up the
> develop-and-test cycle on say not-so-resource-constrained platforms).
> 
> On the other hand, one might argue that such information should just be
> parsed by EBBR-conformant bootloaders and be passed to the kernel via
> standard UEFI interfaces and DMI tables. But I'm not aware of Barebox
> having implemented any of that yet, and even for U-Boot (e.g., Realtek
> based consumer devices...) not everyone has the GPL sources or tools to
> update their bootloader. So, having the kernel we control gather
> information relevant to kernel developers does make some sense to me.
> 
> Thoughts?
> 
> Regards,
> Andreas
> 
> P.S. Sorry that a seemingly trivial one-line 0-day fix derailed into
> this fundamental use case discussion.
> 
> arch/arm/mach-ep93xx/core.c:    soc_dev = soc_device_register(soc_dev_attr);
> arch/arm/mach-imx/cpu.c:        soc_dev = soc_device_register(soc_dev_attr);
> arch/arm/mach-mvebu/mvebu-soc-id.c:     soc_dev =
> soc_device_register(soc_dev_attr);
> arch/arm/mach-mxs/mach-mxs.c:   soc_dev = soc_device_register(soc_dev_attr);
> arch/arm/mach-omap2/id.c:       soc_dev = soc_device_register(soc_dev_attr);
> arch/arm/mach-tegra/tegra.c:    struct device *parent =
> tegra_soc_device_register();
> arch/arm/mach-zynq/common.c:    soc_dev = soc_device_register(soc_dev_attr);
> arch/nios2/platform/platform.c:         soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/amlogic/meson-gx-socinfo.c: soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/amlogic/meson-mx-socinfo.c: soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/atmel/soc.c:        soc_dev = soc_device_register(soc_dev_attr);
> drivers/soc/bcm/brcmstb/common.c:       soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/fsl/guts.c: soc_dev = soc_device_register(&soc_dev_attr);
> drivers/soc/imx/soc-imx-scu.c:  soc_dev = soc_device_register(soc_dev_attr);
> drivers/soc/imx/soc-imx8.c:     soc_dev = soc_device_register(soc_dev_attr);
> drivers/soc/qcom/socinfo.c:     qs->soc_dev =
> soc_device_register(&qs->attr);
> drivers/soc/realtek/chip.c:     soc_dev = soc_device_register(soc_dev_attr);
> drivers/soc/renesas/renesas-soc.c:      soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/samsung/exynos-chipid.c:    soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/tegra/fuse/fuse-tegra.c:    dev = soc_device_register(attr);
> drivers/soc/ux500/ux500-soc-id.c:       soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/versatile/soc-integrator.c: soc_dev =
> soc_device_register(soc_dev_attr);
> drivers/soc/versatile/soc-realview.c:   soc_dev =
> soc_device_register(soc_dev_attr);
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Sense of soc bus? (was: [PATCH] base: soc: Export soc_device_to_device() helper)

[Geert Uytterhoeven]

Hi Neil,

On Fri, Nov 15, 2019 at 9:52 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
> On 12/11/2019 11:47, Andreas Färber wrote:
> > Am 12.11.19 um 08:29 schrieb Uwe Kleine-König:
> >> On Tue, Nov 12, 2019 at 06:23:47AM +0100, Greg Kroah-Hartman wrote:
> >>> On Mon, Nov 11, 2019 at 09:10:41PM +0100, Andreas Färber wrote:
> >>>> Am 11.11.19 um 07:40 schrieb Greg Kroah-Hartman:
> >>>>> On Mon, Nov 11, 2019 at 06:42:05AM +0100, Andreas Färber wrote:
> >>>>>> Am 11.11.19 um 06:27 schrieb Greg Kroah-Hartman:
> >>>>>>> On Mon, Nov 11, 2019 at 05:56:09AM +0100, Andreas Färber wrote:
> >>>>>>>> Use of soc_device_to_device() in driver modules causes a build failure.
> >>>>>>>> Given that the helper is nicely documented in include/linux/sys_soc.h,
> >>>>>>>> let's export it as GPL symbol.
> >>>>>>>
> >>>>>>> I thought we were fixing the soc drivers to not need this.  What
> >>>>>>> happened to that effort?  I thought I had patches in my tree (or
> >>>>>>> someone's tree) that did some of this work already, such that this
> >>>>>>> symbol isn't needed anymore.
> >>>>>>
> >>>>>> I do still see this function used in next-20191108 in drivers/soc/.
> >>>>>>
> >>>>>> I'll be happy to adjust my RFC driver if someone points me to how!
> >>>>>
> >>>>> Look at c31e73121f4c ("base: soc: Handle custom soc information sysfs
> >>>>> entries") for how you can just use the default attributes for the soc to
> >>>>> create the needed sysfs files, instead of having to do it "by hand"
> >>>>> which is racy and incorrect.
> >>>>
> >>>> Unrelated.
> >>>>
> >>>>>> Given the current struct layout, a type cast might work (but ugly).
> >>>>>> Or if we stay with my current RFC driver design, we could use the
> >>>>>> platform_device instead of the soc_device (which would clutter the
> >>>>>> screen more than "soc soc0:") or resort to pr_info() w/o device.
> >>>>>
> >>>>> Ick, no, don't cast blindly.  What do you need the pointer for?  Is this
> >>>>> for in-tree code?
> >>>>
> >>>> No, an RFC patchset: https://patchwork.kernel.org/cover/11224261/
> >>>>
> >>>> As I indicated above, I used it for a dev_info(), which I can easily
> >>>> avoid by using pr_info() instead:
> >>>>
> >>>> diff --git a/drivers/soc/realtek/chip.c b/drivers/soc/realtek/chip.c
> >>>> index e5078c6731fd..f9380e831659 100644
> >>>> --- a/drivers/soc/realtek/chip.c
> >>>> +++ b/drivers/soc/realtek/chip.c
> >>>> @@ -178,8 +178,7 @@ static int rtd_soc_probe(struct platform_device *pdev)
> >>>>
> >>>>         platform_set_drvdata(pdev, soc_dev);
> >>>>
> >>>> -       dev_info(soc_device_to_device(soc_dev),
> >>>> -               "%s %s (0x%08x) rev %s (0x%08x) detected\n",
> >>>> +       pr_info("%s %s (0x%08x) rev %s (0x%08x) detected\n",
> >>>>                 soc_dev_attr->family, soc_dev_attr->soc_id, chip_id,
> >>>>                 soc_dev_attr->revision, chip_rev);
> >>>
> >>> First off, the driver should not be spitting out noise for when all goes
> >>> well like this :)
> >>
> >> I didn't follow the discussion closely, but I think I want to object
> >> here a bit. While I agree that each driver emitting some stuff to the
> >> log buffer is hardly helpful, seeing the exact SoC details is indeed
> >> useful at times. With my Debian kernel team member hat on, I'd say
> >> keep this information. This way the SoC details make it into kernel bug
> >> reports without effort on our side.
> >
> > Seconded. For example, RTD1295 will support LSADC only from revision B00
> > on (and it's not the first time I'm seeing such things in the industry).
> > So if a user complains, it will be helpful to see that information.
> >
> > Referencing your Amlogic review, with all due respect for its authors,
> > the common framework here just lets that information evaporate into the
> > deeps of sysfs.
>
> Hopefully we never had the case where needed to use the soc info in drivers,
> but now we have one and having such infrastructure already in-place will help.
>
> Renesas platforms makes a extensive usage of the soc info infrastructure to
> figure out plenty of HW parameters at runtime and lower their DT changes.

We do our best to use it solely for detecting quirks in early SoC revisions.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Sense of soc bus? (was: [PATCH] base: soc: Export soc_device_to_device() helper)

[Andreas Färber]

Hi Geert,

Am 15.11.19 um 09:58 schrieb Geert Uytterhoeven:
> On Fri, Nov 15, 2019 at 9:52 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>> On 12/11/2019 11:47, Andreas Färber wrote:
>>> For example, RTD1295 will support LSADC only from revision B00
>>> on (and it's not the first time I'm seeing such things in the industry).
>>> So if a user complains, it will be helpful to see that information.
>>>
>>> Referencing your Amlogic review, with all due respect for its authors,
>>> the common framework here just lets that information evaporate into the
>>> deeps of sysfs.
>>
>> Hopefully we never had the case where needed to use the soc info in drivers,
>> but now we have one and having such infrastructure already in-place will help.
>>
>> Renesas platforms makes a extensive usage of the soc info infrastructure to
>> figure out plenty of HW parameters at runtime and lower their DT changes.
> 
> We do our best to use it solely for detecting quirks in early SoC revisions.

Got a pointer? I fail to immediately understand how sysfs would help
drivers (as opposed to userspace) detect quirks: Parsing strings back
doesn't sound efficient, and I don't see you exporting any custom APIs
in drivers/soc/renesas/renesas-soc.c?

Regards,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Sense of soc bus? (was: [PATCH] base: soc: Export soc_device_to_device() helper)

[Geert Uytterhoeven]

Hi Andreas,

On Fri, Nov 15, 2019 at 1:01 PM Andreas Färber <afaerber@suse.de> wrote:
> Am 15.11.19 um 09:58 schrieb Geert Uytterhoeven:
> > On Fri, Nov 15, 2019 at 9:52 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >> On 12/11/2019 11:47, Andreas Färber wrote:
> >>> For example, RTD1295 will support LSADC only from revision B00
> >>> on (and it's not the first time I'm seeing such things in the industry).
> >>> So if a user complains, it will be helpful to see that information.
> >>>
> >>> Referencing your Amlogic review, with all due respect for its authors,
> >>> the common framework here just lets that information evaporate into the
> >>> deeps of sysfs.
> >>
> >> Hopefully we never had the case where needed to use the soc info in drivers,
> >> but now we have one and having such infrastructure already in-place will help.
> >>
> >> Renesas platforms makes a extensive usage of the soc info infrastructure to
> >> figure out plenty of HW parameters at runtime and lower their DT changes.
> >
> > We do our best to use it solely for detecting quirks in early SoC revisions.
>
> Got a pointer? I fail to immediately understand how sysfs would help
> drivers (as opposed to userspace) detect quirks: Parsing strings back
> doesn't sound efficient, and I don't see you exporting any custom APIs
> in drivers/soc/renesas/renesas-soc.c?

We use soc_device_match(), inside kernel drivers.
Exposure through sysfs is a side-effect of using soc_device_register(),
and welcomed, as it allows the user to find out quickly which SoC and
revision is being used.

FTR, lshw (Ubuntu 18.04 has v2.18, which does seem to be the latest
upstream version) does not parse /sys/devices/soc0/.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Sense of soc bus? (was: [PATCH] base: soc: Export soc_device_to_device() helper)

[Tony Lindgren]

* Geert Uytterhoeven <geert@linux-m68k.org> [191115 15:51]:
> On Fri, Nov 15, 2019 at 1:01 PM Andreas Färber <afaerber@suse.de> wrote:
> > Am 15.11.19 um 09:58 schrieb Geert Uytterhoeven:
> > > We do our best to use it solely for detecting quirks in early SoC revisions.
> >
> > Got a pointer? I fail to immediately understand how sysfs would help
> > drivers (as opposed to userspace) detect quirks: Parsing strings back
> > doesn't sound efficient, and I don't see you exporting any custom APIs
> > in drivers/soc/renesas/renesas-soc.c?
> 
> We use soc_device_match(), inside kernel drivers.
> Exposure through sysfs is a side-effect of using soc_device_register(),
> and welcomed, as it allows the user to find out quickly which SoC and
> revision is being used.

For the omap variants too, we've so far gotten away with early SoC
detection for platform code, and then use soc_device_match() in few
cases for drivers at probe time if needed.

Regards,

Tony

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH] base: soc: Export soc_device_to_device() helper

[Lee Jones]

On Tue, 12 Nov 2019, Uwe Kleine-König wrote:

> On Tue, Nov 12, 2019 at 06:23:47AM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Nov 11, 2019 at 09:10:41PM +0100, Andreas Färber wrote:
> > > Am 11.11.19 um 07:40 schrieb Greg Kroah-Hartman:
> > > > On Mon, Nov 11, 2019 at 06:42:05AM +0100, Andreas Färber wrote:
> > > >> Hi Greg,
> > > >>
> > > >> Am 11.11.19 um 06:27 schrieb Greg Kroah-Hartman:
> > > >>> On Mon, Nov 11, 2019 at 05:56:09AM +0100, Andreas Färber wrote:
> > > >>>> Use of soc_device_to_device() in driver modules causes a build failure.
> > > >>>> Given that the helper is nicely documented in include/linux/sys_soc.h,
> > > >>>> let's export it as GPL symbol.
> > > >>>
> > > >>> I thought we were fixing the soc drivers to not need this.  What
> > > >>> happened to that effort?  I thought I had patches in my tree (or
> > > >>> someone's tree) that did some of this work already, such that this
> > > >>> symbol isn't needed anymore.
> > > >>
> > > >> I do still see this function used in next-20191108 in drivers/soc/.
> > > >>
> > > >> I'll be happy to adjust my RFC driver if someone points me to how!
> > > > 
> > > > Look at c31e73121f4c ("base: soc: Handle custom soc information sysfs
> > > > entries") for how you can just use the default attributes for the soc to
> > > > create the needed sysfs files, instead of having to do it "by hand"
> > > > which is racy and incorrect.
> > > 
> > > Unrelated.
> > > 
> > > >> Given the current struct layout, a type cast might work (but ugly).
> > > >> Or if we stay with my current RFC driver design, we could use the
> > > >> platform_device instead of the soc_device (which would clutter the
> > > >> screen more than "soc soc0:") or resort to pr_info() w/o device.
> > > > 
> > > > Ick, no, don't cast blindly.  What do you need the pointer for?  Is this
> > > > for in-tree code?
> > > 
> > > No, an RFC patchset: https://patchwork.kernel.org/cover/11224261/
> > > 
> > > As I indicated above, I used it for a dev_info(), which I can easily
> > > avoid by using pr_info() instead:
> > > 
> > > diff --git a/drivers/soc/realtek/chip.c b/drivers/soc/realtek/chip.c
> > > index e5078c6731fd..f9380e831659 100644
> > > --- a/drivers/soc/realtek/chip.c
> > > +++ b/drivers/soc/realtek/chip.c
> > > @@ -178,8 +178,7 @@ static int rtd_soc_probe(struct platform_device *pdev)
> > > 
> > >         platform_set_drvdata(pdev, soc_dev);
> > > 
> > > -       dev_info(soc_device_to_device(soc_dev),
> > > -               "%s %s (0x%08x) rev %s (0x%08x) detected\n",
> > > +       pr_info("%s %s (0x%08x) rev %s (0x%08x) detected\n",
> > >                 soc_dev_attr->family, soc_dev_attr->soc_id, chip_id,
> > >                 soc_dev_attr->revision, chip_rev);
> > 
> > First off, the driver should not be spitting out noise for when all goes
> > well like this :)
> 
> I didn't follow the discussion closely, but I think I want to object
> here a bit. While I agree that each driver emitting some stuff to the
> log buffer is hardly helpful, seeing the exact SoC details is indeed
> useful at times. With my Debian kernel team member hat on, I'd say
> keep this information. This way the SoC details make it into kernel bug
> reports without effort on our side.

Right. From my angle we are starting to be way too aggressive with the
point about not printing information to the kernel log. In only a
small set of cases does this actually cause an issue i.e. with
platforms containing so many devices that printing information from
each of them does significantly increase boot times. In my world of
small electronics I've been greatly hindered by the lack of
information, such that it has cost days of engineering trying to track
down fictitious bugs and the like.

For platforms where printing useful information culminates in negative
effects, perhaps simply lower their log level, rather than suffocate
all platforms.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic