From: "Andreas Färber" <afaerber@suse.de> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: linux-realtek-soc@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Lee Jones <lee.jones@linaro.org>, Bjorn Andersson <bjorn.andersson@linaro.org>, Geert Uytterhoeven <geert+renesas@glider.be>, "Rafael J. Wysocki" <rafael@kernel.org>, Shawn Guo <shawnguo@kernel.org>, Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix Kernel Team <kernel@pengutronix.de>, Fabio Estevam <festevam@gmail.com>, NXP Linux Team <linux-imx@nxp.com>, Hartley Sweeten <hsweeten@visionengravers.com>, Alexander Sverdlin <alexander.sverdlin@gmail.com>, Tony Lindgren <tony@atomide.com>, linux-omap@vger.kernel.org, Michal Simek <michal.simek@xilinx.com>, Kevin Hilman <khilman@baylibre.com>, linux-amlogic@lists.infradead.org, Thierry Reding <thierry.reding@gmail.com>, Jonathan Hunter <jonathanh@nvidia.com>, "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>, Linus Walleij <linus.walleij@linaro.org> Subject: Re: [PATCH] base: soc: Export soc_device_to_device() helper Date: Mon, 11 Nov 2019 21:10:41 +0100 Message-ID: <a88442df-dc6b-07e5-8dee-9e308bdda450@suse.de> (raw) In-Reply-To: <20191111064040.GA3502217@kroah.com> 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)
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 [this message] 2019-11-12 0:29 ` Andreas Färber 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=a88442df-dc6b-07e5-8dee-9e308bdda450@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