[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMuHMdWOWWQoJh5=07VMRhtrFR_Gc_qNhjTV4tCsvwvMn0kYfA@mail.gmail.com>
Date: Fri, 15 Nov 2019 09:58:18 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Neil Armstrong <narmstrong@...libre.com>
Cc: Andreas Färber <afaerber@...e.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Geert Uytterhoeven <geert+renesas@...der.be>,
linux-realtek-soc@...ts.infradead.org,
Tony Lindgren <tony@...mide.com>,
Linus Walleij <linus.walleij@...aro.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Thierry Reding <thierry.reding@...il.com>,
Lee Jones <lee.jones@...aro.org>,
Rob Herring <robh@...nel.org>,
Kevin Hilman <khilman@...libre.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Michal Simek <michal.simek@...inx.com>,
Jonathan Hunter <jonathanh@...dia.com>,
NXP Linux Team <linux-imx@....com>,
Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>,
boot-architecture@...ts.linaro.org,
Sascha Hauer <s.hauer@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
"open list:ARM/Amlogic Meson..." <linux-amlogic@...ts.infradead.org>,
"open list:TI ETHERNET SWITCH DRIVER (CPSW)"
<linux-omap@...r.kernel.org>,
Alexander Sverdlin <alexander.sverdlin@...il.com>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Hartley Sweeten <hsweeten@...ionengravers.com>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Shawn Guo <shawnguo@...nel.org>
Subject: Re: Sense of soc bus? (was: [PATCH] base: soc: Export
soc_device_to_device() helper)
Hi Neil,
On Fri, Nov 15, 2019 at 9:52 AM Neil Armstrong <narmstrong@...libre.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@...ux-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
Powered by blists - more mailing lists