From: "Andreas Färber" <afaerber@suse.de> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Geert Uytterhoeven <geert+renesas@glider.be>, linux-realtek-soc@lists.infradead.org, Tony Lindgren <tony@atomide.com>, Linus Walleij <linus.walleij@linaro.org>, Bjorn Andersson <bjorn.andersson@linaro.org>, Thierry Reding <thierry.reding@gmail.com>, Fabio Estevam <festevam@gmail.com>, Kevin Hilman <khilman@baylibre.com>, "Rafael J. Wysocki" <rafael@kernel.org>, Michal Simek <michal.simek@xilinx.com>, Jonathan Hunter <jonathanh@nvidia.com>, NXP Linux Team <linux-imx@nxp.com>, Sascha Hauer <s.hauer@pengutronix.de>, "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>, linux-amlogic@lists.infradead.org, Lee Jones <lee.jones@linaro.org>, linux-omap@vger.kernel.org, Alexander Sverdlin <alexander.sverdlin@gmail.com>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Hartley Sweeten <hsweeten@visionengravers.com>, Pengutronix Kernel Team <kernel@pengutronix.de>, Shawn Guo <shawnguo@kernel.org> Subject: Re: [PATCH] base: soc: Export soc_device_to_device() helper Date: Tue, 12 Nov 2019 01:29:07 +0100 Message-ID: <94db77d0-f7a3-2a16-6a5d-cd28f68fe5b2@suse.de> (raw) In-Reply-To: <a88442df-dc6b-07e5-8dee-9e308bdda450@suse.de> 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)
next prev parent reply index Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-03 1:36 [RFC 00/11] ARM: Realtek RTD1195/RTD1295 SoC info Andreas Färber 2019-11-03 1:36 ` [RFC 01/11] dt-bindings: soc: Add Realtek RTD1195 chip info binding Andreas Färber 2019-11-06 4:41 ` Rob Herring 2019-11-03 1:36 ` [RFC 02/11] soc: Add Realtek chip info driver for RTD1195 and RTD1295 Andreas Färber 2019-11-03 1:45 ` Andreas Färber 2019-11-11 4:56 ` [PATCH] base: soc: Export soc_device_to_device() helper Andreas Färber 2019-11-11 5:27 ` Greg Kroah-Hartman 2019-11-11 5:42 ` Andreas Färber 2019-11-11 6:40 ` Greg Kroah-Hartman 2019-11-11 20:10 ` Andreas Färber 2019-11-12 0:29 ` Andreas Färber [this message] 2019-11-12 5:23 ` Greg Kroah-Hartman 2019-11-12 7:29 ` Uwe Kleine-König 2019-11-12 10:47 ` Sense of soc bus? (was: [PATCH] base: soc: Export soc_device_to_device() helper) Andreas Färber 2019-11-14 22:09 ` Rob Herring 2019-11-15 11:15 ` Andreas Färber 2019-11-15 11:49 ` Andreas Färber 2019-11-15 8:52 ` Neil Armstrong 2019-11-15 8:58 ` Geert Uytterhoeven 2019-11-15 12:00 ` Andreas Färber 2019-11-15 12:34 ` Geert Uytterhoeven 2019-11-18 15:55 ` Tony Lindgren 2019-11-12 10:48 ` [PATCH] base: soc: Export soc_device_to_device() helper Lee Jones 2020-01-02 14:29 ` [RFC 02/11] soc: Add Realtek chip info driver for RTD1195 and RTD1295 James Tai 2020-01-02 14:39 ` Andreas Färber 2020-01-02 15:02 ` James Tai 2020-01-03 5:07 ` Stanley Chang[昌育德] 2019-11-03 1:36 ` [RFC 03/11] arm64: dts: realtek: rtd129x: Add chip info node Andreas Färber 2020-01-02 14:32 ` James Tai 2020-01-03 5:07 ` Stanley Chang[昌育德] 2020-01-02 14:33 ` James Tai 2020-01-02 14:34 ` James Tai 2019-11-03 1:36 ` [RFC 04/11] ARM: dts: rtd1195: " Andreas Färber 2019-11-03 1:36 ` [RFC 05/11] dt-bindings: soc: realtek: rtd1195-chip: Extend reg property Andreas Färber 2019-11-06 4:46 ` Rob Herring 2019-11-06 8:42 ` Andreas Färber 2019-11-03 1:36 ` [RFC 06/11] soc: realtek: chip: Detect RTD1296 Andreas Färber 2020-01-02 14:35 ` James Tai 2019-11-03 1:36 ` [RFC 07/11] arm64: dts: realtek: rtd129x: Extend chip-info reg with CHIP_INFO1 Andreas Färber 2019-11-03 1:36 ` [RFC 08/11] soc: realtek: chip: Detect RTD1293 Andreas Färber 2019-11-03 1:36 ` [RFC 09/11] dt-bindings: soc: realtek: rtd1195-chip: Extend reg node again Andreas Färber 2019-11-03 1:36 ` [RFC 10/11] soc: realtek: chip: Detect RTD1294 Andreas Färber 2019-11-03 1:36 ` [RFC 11/11] arm64: dts: realtek: rtd129x: Extend chip-info reg with efuse Andreas Färber 2019-11-07 7:16 ` [RFC 00/11] ARM: Realtek RTD1195/RTD1295 SoC info Andreas Färber
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=94db77d0-f7a3-2a16-6a5d-cd28f68fe5b2@suse.de \ --to=afaerber@suse.de \ --cc=alexander.sverdlin@gmail.com \ --cc=bjorn.andersson@linaro.org \ --cc=festevam@gmail.com \ --cc=geert+renesas@glider.be \ --cc=gregkh@linuxfoundation.org \ --cc=hsweeten@visionengravers.com \ --cc=jonathanh@nvidia.com \ --cc=kernel@pengutronix.de \ --cc=khilman@baylibre.com \ --cc=lee.jones@linaro.org \ --cc=linus.walleij@linaro.org \ --cc=linux-amlogic@lists.infradead.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-imx@nxp.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-omap@vger.kernel.org \ --cc=linux-realtek-soc@lists.infradead.org \ --cc=linux-tegra@vger.kernel.org \ --cc=michal.simek@xilinx.com \ --cc=rafael@kernel.org \ --cc=s.hauer@pengutronix.de \ --cc=shawnguo@kernel.org \ --cc=thierry.reding@gmail.com \ --cc=tony@atomide.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
LKML Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \ linux-kernel@vger.kernel.org public-inbox-index lkml Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git