Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 0/8] block: implement NVMEM provider
From: Rob Herring @ 2024-03-25 15:12 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Krzysztof Kozlowski, Conor Dooley, Ulf Hansson, Jens Axboe,
	Dave Chinner, Jan Kara, Thomas Weißschuh, Damien Le Moal,
	Li Lingfeng, Christian Brauner, Christian Heusel, Min Li,
	Adrian Hunter, Avri Altman, Hannes Reinecke, Christian Loehle,
	Bean Huo, Yeqi Fu, Victor Shih, Christophe JAILLET,
	Dominique Martinet, Ricardo B. Marliere, devicetree, linux-kernel,
	linux-mmc, linux-block
In-Reply-To: <cover.1711048433.git.daniel@makrotopia.org>

On Thu, Mar 21, 2024 at 07:31:48PM +0000, Daniel Golle wrote:
> On embedded devices using an eMMC it is common that one or more (hw/sw)
> partitions on the eMMC are used to store MAC addresses and Wi-Fi
> calibration EEPROM data.
> 
> Implement an NVMEM provider backed by a block device as typically the
> NVMEM framework is used to have kernel drivers read and use binary data
> from EEPROMs, efuses, flash memory (MTD), ...
> 
> In order to be able to reference hardware partitions on an eMMC, add code
> to bind each hardware partition to a specific firmware subnode.
> 
> Overall, this enables uniform handling across practially all flash
> storage types used for this purpose (MTD, UBI, and now also MMC).
> 
> As part of this series it was necessary to define a device tree schema
> for block devices and partitions on them, which (similar to how it now
> works also for UBI volumes) can be matched by one or more properties.
> 
> ---
> This series has previously been submitted as RFC on July 19th 2023[1]
> and most of the basic idea did not change since. Another round of RFC
> was submitted on March 5th 2024[2] which has received overall positive
> feedback and only minor corrections have been done since (see
> changelog below).

Also, please version your patches. 'RFC' is a tag, not a version. v1 was
July. v2 was March 5th. This is v3.

Rob

^ permalink raw reply

* [PATCH v2] arm64: dts: imx8mp: Add DT nodes for the two ISPs
From: Laurent Pinchart @ 2024-03-25 15:13 UTC (permalink / raw)
  To: imx, linux-arm-kernel
  Cc: Paul Elder, Adam Ford, Alexander Stein, Conor Dooley,
	Fabio Estevam, Krzysztof Kozlowski, Marek Vasut,
	Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo,
	devicetree, linux-media

From: Paul Elder <paul.elder@ideasonboard.com>

The ISP supports both CSI and parallel interfaces, where port 0
corresponds to the former and port 1 corresponds to the latter. Since
the i.MX8MP's ISPs are connected by the parallel interface to the CSI
receiver, set them both to port 1.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:

- Fix clock ordering
- Add #address-cells and #size-cells to ports nodes
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 50 +++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index bfc5c81a5bd4..1d2670b91b53 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1616,6 +1616,56 @@ isi_in_1: endpoint {
 				};
 			};
 
+			isp_0: isp@32e10000 {
+				compatible = "fsl,imx8mp-isp";
+				reg = <0x32e10000 0x10000>;
+				interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MP_CLK_MEDIA_ISP_ROOT>,
+					 <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>,
+					 <&clk IMX8MP_CLK_MEDIA_APB_ROOT>;
+				clock-names = "isp", "aclk", "hclk";
+				assigned-clocks = <&clk IMX8MP_CLK_MEDIA_ISP>;
+				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
+				assigned-clock-rates = <500000000>;
+				power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_ISP>;
+				fsl,blk-ctrl = <&media_blk_ctrl 0>;
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@1 {
+						reg = <1>;
+					};
+				};
+			};
+
+			isp_1: isp@32e20000 {
+				compatible = "fsl,imx8mp-isp";
+				reg = <0x32e20000 0x10000>;
+				interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MP_CLK_MEDIA_ISP_ROOT>,
+					 <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>,
+					 <&clk IMX8MP_CLK_MEDIA_APB_ROOT>;
+				clock-names = "isp", "aclk", "hclk";
+				assigned-clocks = <&clk IMX8MP_CLK_MEDIA_ISP>;
+				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
+				assigned-clock-rates = <500000000>;
+				power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_ISP>;
+				fsl,blk-ctrl = <&media_blk_ctrl 1>;
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@1 {
+						reg = <1>;
+					};
+				};
+			};
+
 			dewarp: dwe@32e30000 {
 				compatible = "nxp,imx8mp-dw100";
 				reg = <0x32e30000 0x10000>;

base-commit: 4cece764965020c22cff7665b18a012006359095
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related

* Re: [PATCH 1/2] arm64: dts: imx8mp: Add DT nodes for the two ISPs
From: Laurent Pinchart @ 2024-03-25 15:14 UTC (permalink / raw)
  To: Adam Ford
  Cc: Paul Elder, linux-media, devicetree, kieran.bingham,
	tomi.valkeinen, umang.jain, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, Marek Vasut, Alexander Stein,
	Lucas Stach, Frank Li,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list
In-Reply-To: <CAHCN7xLOu5qfxNihEYuSXxuxxH=S_+7nDkX1H=ziDW3QvaErQA@mail.gmail.com>

Hi Adam,

On Wed, Mar 20, 2024 at 07:35:46AM -0500, Adam Ford wrote:
> On Wed, Nov 29, 2023 at 3:31 AM Paul Elder wrote:
> >
> > The ISP supports both CSI and parallel interfaces, where port 0
> > corresponds to the former and port 1 corresponds to the latter. Since
> > the i.MX8MP's ISPs are connected by the parallel interface to the CSI
> > receiver, set them both to port 1.
> >
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> Paul, are you able to resend this now that the driver part has been
> merged into the main branch?
> 
> If you can't, I can resend it on your behalf.

I've just sent a v2, you're on CC.

> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 50 +++++++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index c9a610ba4836..25579d4c58f2 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -1604,6 +1604,56 @@ isi_in_1: endpoint {
> >                                 };
> >                         };
> >
> > +                       isp_0: isp@32e10000 {
> > +                               compatible = "fsl,imx8mp-isp";
> > +                               reg = <0x32e10000 0x10000>;
> > +                               interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
> > +                               clocks = <&clk IMX8MP_CLK_MEDIA_ISP_ROOT>,
> > +                                        <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>,
> > +                                        <&clk IMX8MP_CLK_MEDIA_APB_ROOT>;
> > +                               clock-names = "isp", "aclk", "hclk";
> > +                               assigned-clocks = <&clk IMX8MP_CLK_MEDIA_ISP>;
> > +                               assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > +                               assigned-clock-rates = <500000000>;
> > +                               power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_ISP>;
> > +                               fsl,blk-ctrl = <&media_blk_ctrl 0>;
> > +                               status = "disabled";
> > +
> > +                               ports {
> > +                                       #address-cells = <1>;
> > +                                       #size-cells = <0>;
> > +
> > +                                       port@1 {
> > +                                               reg = <1>;
> > +                                       };
> > +                               };
> > +                       };
> > +
> > +                       isp_1: isp@32e20000 {
> > +                               compatible = "fsl,imx8mp-isp";
> > +                               reg = <0x32e20000 0x10000>;
> > +                               interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
> > +                               clocks = <&clk IMX8MP_CLK_MEDIA_ISP_ROOT>,
> > +                                        <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>,
> > +                                        <&clk IMX8MP_CLK_MEDIA_APB_ROOT>;
> > +                               clock-names = "isp", "aclk", "hclk";
> > +                               assigned-clocks = <&clk IMX8MP_CLK_MEDIA_ISP>;
> > +                               assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > +                               assigned-clock-rates = <500000000>;
> > +                               power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_ISP>;
> > +                               fsl,blk-ctrl = <&media_blk_ctrl 1>;
> > +                               status = "disabled";
> > +
> > +                               ports {
> > +                                       #address-cells = <1>;
> > +                                       #size-cells = <0>;
> > +
> > +                                       port@1 {
> > +                                               reg = <1>;
> > +                                       };
> > +                               };
> > +                       };
> > +
> >                         dewarp: dwe@32e30000 {
> >                                 compatible = "nxp,imx8mp-dw100";
> >                                 reg = <0x32e30000 0x10000>;

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [PATCH v2] arm64: dts: imx8mp: Add DT nodes for the two ISPs
From: Adam Ford @ 2024-03-25 15:16 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: imx, linux-arm-kernel, Paul Elder, Alexander Stein, Conor Dooley,
	Fabio Estevam, Krzysztof Kozlowski, Marek Vasut,
	Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo,
	devicetree, linux-media
In-Reply-To: <20240325151339.19041-1-laurent.pinchart@ideasonboard.com>

On Mon, Mar 25, 2024 at 10:13 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> From: Paul Elder <paul.elder@ideasonboard.com>
>
> The ISP supports both CSI and parallel interfaces, where port 0
> corresponds to the former and port 1 corresponds to the latter. Since
> the i.MX8MP's ISPs are connected by the parallel interface to the CSI
> receiver, set them both to port 1.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Tested-by:  Adam Ford <aford173@gmail.com> # imx8mp-beacon

> ---
> Changes since v1:
>
> - Fix clock ordering
> - Add #address-cells and #size-cells to ports nodes
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 50 +++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index bfc5c81a5bd4..1d2670b91b53 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -1616,6 +1616,56 @@ isi_in_1: endpoint {
>                                 };
>                         };
>
> +                       isp_0: isp@32e10000 {
> +                               compatible = "fsl,imx8mp-isp";
> +                               reg = <0x32e10000 0x10000>;
> +                               interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
> +                               clocks = <&clk IMX8MP_CLK_MEDIA_ISP_ROOT>,
> +                                        <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>,
> +                                        <&clk IMX8MP_CLK_MEDIA_APB_ROOT>;
> +                               clock-names = "isp", "aclk", "hclk";
> +                               assigned-clocks = <&clk IMX8MP_CLK_MEDIA_ISP>;
> +                               assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> +                               assigned-clock-rates = <500000000>;
> +                               power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_ISP>;
> +                               fsl,blk-ctrl = <&media_blk_ctrl 0>;
> +                               status = "disabled";
> +
> +                               ports {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +
> +                                       port@1 {
> +                                               reg = <1>;
> +                                       };
> +                               };
> +                       };
> +
> +                       isp_1: isp@32e20000 {
> +                               compatible = "fsl,imx8mp-isp";
> +                               reg = <0x32e20000 0x10000>;
> +                               interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
> +                               clocks = <&clk IMX8MP_CLK_MEDIA_ISP_ROOT>,
> +                                        <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>,
> +                                        <&clk IMX8MP_CLK_MEDIA_APB_ROOT>;
> +                               clock-names = "isp", "aclk", "hclk";
> +                               assigned-clocks = <&clk IMX8MP_CLK_MEDIA_ISP>;
> +                               assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> +                               assigned-clock-rates = <500000000>;
> +                               power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_ISP>;
> +                               fsl,blk-ctrl = <&media_blk_ctrl 1>;
> +                               status = "disabled";
> +
> +                               ports {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +
> +                                       port@1 {
> +                                               reg = <1>;
> +                                       };
> +                               };
> +                       };
> +
>                         dewarp: dwe@32e30000 {
>                                 compatible = "nxp,imx8mp-dw100";
>                                 reg = <0x32e30000 0x10000>;
>
> base-commit: 4cece764965020c22cff7665b18a012006359095
> --
> Regards,
>
> Laurent Pinchart
>

^ permalink raw reply

* Re: [PATCH 1/2] arm64: dts: imx8mp: Add DT nodes for the two ISPs
From: Adam Ford @ 2024-03-25 15:17 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Paul Elder, linux-media, devicetree, kieran.bingham,
	tomi.valkeinen, umang.jain, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, Marek Vasut, Alexander Stein,
	Lucas Stach, Frank Li,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list
In-Reply-To: <20240325151402.GW18799@pendragon.ideasonboard.com>

On Mon, Mar 25, 2024 at 10:14 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Adam,
>
> On Wed, Mar 20, 2024 at 07:35:46AM -0500, Adam Ford wrote:
> > On Wed, Nov 29, 2023 at 3:31 AM Paul Elder wrote:
> > >
> > > The ISP supports both CSI and parallel interfaces, where port 0
> > > corresponds to the former and port 1 corresponds to the latter. Since
> > > the i.MX8MP's ISPs are connected by the parallel interface to the CSI
> > > receiver, set them both to port 1.
> > >
> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> >
> > Paul, are you able to resend this now that the driver part has been
> > merged into the main branch?
> >
> > If you can't, I can resend it on your behalf.
>
> I've just sent a v2, you're on CC.

Thanks!

adam
>
> > > ---
> > >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 50 +++++++++++++++++++++++
> > >  1 file changed, 50 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > index c9a610ba4836..25579d4c58f2 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > @@ -1604,6 +1604,56 @@ isi_in_1: endpoint {
> > >                                 };
> > >                         };
> > >
> > > +                       isp_0: isp@32e10000 {
> > > +                               compatible = "fsl,imx8mp-isp";
> > > +                               reg = <0x32e10000 0x10000>;
> > > +                               interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
> > > +                               clocks = <&clk IMX8MP_CLK_MEDIA_ISP_ROOT>,
> > > +                                        <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>,
> > > +                                        <&clk IMX8MP_CLK_MEDIA_APB_ROOT>;
> > > +                               clock-names = "isp", "aclk", "hclk";
> > > +                               assigned-clocks = <&clk IMX8MP_CLK_MEDIA_ISP>;
> > > +                               assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > > +                               assigned-clock-rates = <500000000>;
> > > +                               power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_ISP>;
> > > +                               fsl,blk-ctrl = <&media_blk_ctrl 0>;
> > > +                               status = "disabled";
> > > +
> > > +                               ports {
> > > +                                       #address-cells = <1>;
> > > +                                       #size-cells = <0>;
> > > +
> > > +                                       port@1 {
> > > +                                               reg = <1>;
> > > +                                       };
> > > +                               };
> > > +                       };
> > > +
> > > +                       isp_1: isp@32e20000 {
> > > +                               compatible = "fsl,imx8mp-isp";
> > > +                               reg = <0x32e20000 0x10000>;
> > > +                               interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
> > > +                               clocks = <&clk IMX8MP_CLK_MEDIA_ISP_ROOT>,
> > > +                                        <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>,
> > > +                                        <&clk IMX8MP_CLK_MEDIA_APB_ROOT>;
> > > +                               clock-names = "isp", "aclk", "hclk";
> > > +                               assigned-clocks = <&clk IMX8MP_CLK_MEDIA_ISP>;
> > > +                               assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > > +                               assigned-clock-rates = <500000000>;
> > > +                               power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_ISP>;
> > > +                               fsl,blk-ctrl = <&media_blk_ctrl 1>;
> > > +                               status = "disabled";
> > > +
> > > +                               ports {
> > > +                                       #address-cells = <1>;
> > > +                                       #size-cells = <0>;
> > > +
> > > +                                       port@1 {
> > > +                                               reg = <1>;
> > > +                                       };
> > > +                               };
> > > +                       };
> > > +
> > >                         dewarp: dwe@32e30000 {
> > >                                 compatible = "nxp,imx8mp-dw100";
> > >                                 reg = <0x32e30000 0x10000>;
>
> --
> Regards,
>
> Laurent Pinchart

^ permalink raw reply

* [PATCH] arm64: dts: ti: k3-am62-lp-sk: Remove tps65219 power-button
From: Markus Schneider-Pargmann @ 2024-03-25 15:20 UTC (permalink / raw)
  To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Anand Gadiyar
  Cc: linux-arm-kernel, devicetree, linux-kernel,
	Markus Schneider-Pargmann

On am62-lp-sk the PMIC is not wired up to a power button. Remove this
property. This fixes issues observed when entering a very deep sleep
state that is not yet available upstream.

Fixes: e6a51ffabfc1 ("arm64: ti: dts: Add support for AM62x LP SK")
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts b/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
index c4149059a4c5..9a17bd3e59c9 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62-lp-sk.dts
@@ -166,7 +166,6 @@ tps65219: pmic@30 {
 
 		interrupt-parent = <&gic500>;
 		interrupts = <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
-		ti,power-button;
 
 		regulators {
 			buck1_reg: buck1 {
-- 
2.43.0


^ permalink raw reply related

* [PATCH v6 0/2] Synchronize DT overlay removal with devlink removals
From: Herve Codina @ 2024-03-25 15:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Rob Herring, Frank Rowand,
	Saravana Kannan
  Cc: Lizhi Hou, Max Zhen, Sonal Santan, Stefano Stabellini,
	Jonathan Cameron, linux-kernel, devicetree, Allan Nielsen,
	Horatiu Vultur, Steen Hegelund, Luca Ceresoli, Nuno Sa,
	Thomas Petazzoni, Herve Codina

Hi,

In the following sequence:
  of_platform_depopulate(); /* Remove devices from a DT overlay node */
  of_overlay_remove(); /* Remove the DT overlay node itself */

Some warnings are raised by __of_changeset_entry_destroy() which  was
called from of_overlay_remove():
  ERROR: memory leak, expected refcount 1 instead of 2 ...

The issue is that, during the device devlink removals triggered from the
of_platform_depopulate(), jobs are put in a workqueue.
These jobs drop the reference to the devices. When a device is no more
referenced (refcount == 0), it is released and the reference to its
of_node is dropped by a call to of_node_put().
These operations are fully correct except that, because of the
workqueue, they are done asynchronously with respect to function calls.

In the sequence provided, the jobs are run too late, after the call to
__of_changeset_entry_destroy() and so a missing of_node_put() call is
detected by __of_changeset_entry_destroy().

This series fixes this issue introducing device_link_wait_removal() in
order to wait for the end of jobs execution (patch 1) and using this
function to synchronize the overlay removal with the end of jobs
execution (patch 2).

Compared to the previous iteration:
  https://lore.kernel.org/linux-kernel/20240307111036.225007-1-herve.codina@bootlin.com/
this v6 series:
- Add Saravana's 'Reviewed-by' tag

This series handles cases reported by Luca [1] and Nuno [2].
  [1]: https://lore.kernel.org/all/20231220181627.341e8789@booty/
  [2]: https://lore.kernel.org/all/20240205-fix-device-links-overlays-v2-2-5344f8c79d57@analog.com/

Best regards,
Hervé

Changes v5 -> v6
  - Patch 1
    Add 'Reviewed-by: Saravana Kannan <saravanak@google.com>'

  - Patch 2
    No changes

Changes v4 -> v5
  - Patch 1
    Remove the 'Fixes' tag
    Add 'Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>'
    Add 'Reviewed-by: Nuno Sa <nuno.sa@analog.com>'

  - Patch 2
    Update comment as suggested
    Add 'Reviewed-by: Saravana Kannan <saravanak@google.com>'
    Add 'Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>'
    Add 'Reviewed-by: Nuno Sa <nuno.sa@analog.com>'

Changes v3 -> v4
  - Patch 1
    Uses flush_workqueue() instead of drain_workqueue().

  - Patch 2
    Remove unlock/re-lock when calling device_link_wait_removal()
    Move device_link_wait_removal() call to of_changeset_destroy()
    Update commit log

Changes v2 -> v3
  - Patch 1
    No changes

  - Patch 2
    Add missing device.h

Changes v1 -> v2
  - Patch 1
    Rename the workqueue to 'device_link_wq'
    Add 'Fixes' tag and Cc stable

  - Patch 2
    Add device.h inclusion.
    Call device_link_wait_removal() later in the overlay removal
    sequence (i.e. in free_overlay_changeset() function).
    Drop of_mutex lock while calling device_link_wait_removal().
    Add	'Fixes'	tag and Cc stable

Herve Codina (2):
  driver core: Introduce device_link_wait_removal()
  of: dynamic: Synchronize of_changeset_destroy() with the devlink
    removals

 drivers/base/core.c    | 26 +++++++++++++++++++++++---
 drivers/of/dynamic.c   | 12 ++++++++++++
 include/linux/device.h |  1 +
 3 files changed, 36 insertions(+), 3 deletions(-)

-- 
2.44.0


^ permalink raw reply

* [PATCH v6 1/2] driver core: Introduce device_link_wait_removal()
From: Herve Codina @ 2024-03-25 15:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Rob Herring, Frank Rowand,
	Saravana Kannan
  Cc: Lizhi Hou, Max Zhen, Sonal Santan, Stefano Stabellini,
	Jonathan Cameron, linux-kernel, devicetree, Allan Nielsen,
	Horatiu Vultur, Steen Hegelund, Luca Ceresoli, Nuno Sa,
	Thomas Petazzoni, Herve Codina, stable
In-Reply-To: <20240325152140.198219-1-herve.codina@bootlin.com>

The commit 80dd33cf72d1 ("drivers: base: Fix device link removal")
introduces a workqueue to release the consumer and supplier devices used
in the devlink.
In the job queued, devices are release and in turn, when all the
references to these devices are dropped, the release function of the
device itself is called.

Nothing is present to provide some synchronisation with this workqueue
in order to ensure that all ongoing releasing operations are done and
so, some other operations can be started safely.

For instance, in the following sequence:
  1) of_platform_depopulate()
  2) of_overlay_remove()

During the step 1, devices are released and related devlinks are removed
(jobs pushed in the workqueue).
During the step 2, OF nodes are destroyed but, without any
synchronisation with devlink removal jobs, of_overlay_remove() can raise
warnings related to missing of_node_put():
  ERROR: memory leak, expected refcount 1 instead of 2

Indeed, the missing of_node_put() call is going to be done, too late,
from the workqueue job execution.

Introduce device_link_wait_removal() to offer a way to synchronize
operations waiting for the end of devlink removals (i.e. end of
workqueue jobs).
Also, as a flushing operation is done on the workqueue, the workqueue
used is moved from a system-wide workqueue to a local one.

Cc: stable@vger.kernel.org
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
Reviewed-by: Saravana Kannan <saravanak@google.com>
---
 drivers/base/core.c    | 26 +++++++++++++++++++++++---
 include/linux/device.h |  1 +
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 7e3af0ad770a..f2242aadffb0 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -44,6 +44,7 @@ static bool fw_devlink_is_permissive(void);
 static void __fw_devlink_link_to_consumers(struct device *dev);
 static bool fw_devlink_drv_reg_done;
 static bool fw_devlink_best_effort;
+static struct workqueue_struct *device_link_wq;
 
 /**
  * __fwnode_link_add - Create a link between two fwnode_handles.
@@ -533,12 +534,26 @@ static void devlink_dev_release(struct device *dev)
 	/*
 	 * It may take a while to complete this work because of the SRCU
 	 * synchronization in device_link_release_fn() and if the consumer or
-	 * supplier devices get deleted when it runs, so put it into the "long"
-	 * workqueue.
+	 * supplier devices get deleted when it runs, so put it into the
+	 * dedicated workqueue.
 	 */
-	queue_work(system_long_wq, &link->rm_work);
+	queue_work(device_link_wq, &link->rm_work);
 }
 
+/**
+ * device_link_wait_removal - Wait for ongoing devlink removal jobs to terminate
+ */
+void device_link_wait_removal(void)
+{
+	/*
+	 * devlink removal jobs are queued in the dedicated work queue.
+	 * To be sure that all removal jobs are terminated, ensure that any
+	 * scheduled work has run to completion.
+	 */
+	flush_workqueue(device_link_wq);
+}
+EXPORT_SYMBOL_GPL(device_link_wait_removal);
+
 static struct class devlink_class = {
 	.name = "devlink",
 	.dev_groups = devlink_groups,
@@ -4165,9 +4180,14 @@ int __init devices_init(void)
 	sysfs_dev_char_kobj = kobject_create_and_add("char", dev_kobj);
 	if (!sysfs_dev_char_kobj)
 		goto char_kobj_err;
+	device_link_wq = alloc_workqueue("device_link_wq", 0, 0);
+	if (!device_link_wq)
+		goto wq_err;
 
 	return 0;
 
+ wq_err:
+	kobject_put(sysfs_dev_char_kobj);
  char_kobj_err:
 	kobject_put(sysfs_dev_block_kobj);
  block_kobj_err:
diff --git a/include/linux/device.h b/include/linux/device.h
index 1795121dee9a..d7d8305a72e8 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1249,6 +1249,7 @@ void device_link_del(struct device_link *link);
 void device_link_remove(void *consumer, struct device *supplier);
 void device_links_supplier_sync_state_pause(void);
 void device_links_supplier_sync_state_resume(void);
+void device_link_wait_removal(void);
 
 /* Create alias, so I can be autoloaded. */
 #define MODULE_ALIAS_CHARDEV(major,minor) \
-- 
2.44.0


^ permalink raw reply related

* [PATCH v6 2/2] of: dynamic: Synchronize of_changeset_destroy() with the devlink removals
From: Herve Codina @ 2024-03-25 15:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Rob Herring, Frank Rowand,
	Saravana Kannan
  Cc: Lizhi Hou, Max Zhen, Sonal Santan, Stefano Stabellini,
	Jonathan Cameron, linux-kernel, devicetree, Allan Nielsen,
	Horatiu Vultur, Steen Hegelund, Luca Ceresoli, Nuno Sa,
	Thomas Petazzoni, Herve Codina, stable
In-Reply-To: <20240325152140.198219-1-herve.codina@bootlin.com>

In the following sequence:
  1) of_platform_depopulate()
  2) of_overlay_remove()

During the step 1, devices are destroyed and devlinks are removed.
During the step 2, OF nodes are destroyed but
__of_changeset_entry_destroy() can raise warnings related to missing
of_node_put():
  ERROR: memory leak, expected refcount 1 instead of 2 ...

Indeed, during the devlink removals performed at step 1, the removal
itself releasing the device (and the attached of_node) is done by a job
queued in a workqueue and so, it is done asynchronously with respect to
function calls.
When the warning is present, of_node_put() will be called but wrongly
too late from the workqueue job.

In order to be sure that any ongoing devlink removals are done before
the of_node destruction, synchronize the of_changeset_destroy() with the
devlink removals.

Fixes: 80dd33cf72d1 ("drivers: base: Fix device link removal")
Cc: stable@vger.kernel.org
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Saravana Kannan <saravanak@google.com>
Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Reviewed-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/of/dynamic.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 3bf27052832f..4d57a4e34105 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -9,6 +9,7 @@
 
 #define pr_fmt(fmt)	"OF: " fmt
 
+#include <linux/device.h>
 #include <linux/of.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
@@ -667,6 +668,17 @@ void of_changeset_destroy(struct of_changeset *ocs)
 {
 	struct of_changeset_entry *ce, *cen;
 
+	/*
+	 * When a device is deleted, the device links to/from it are also queued
+	 * for deletion. Until these device links are freed, the devices
+	 * themselves aren't freed. If the device being deleted is due to an
+	 * overlay change, this device might be holding a reference to a device
+	 * node that will be freed. So, wait until all already pending device
+	 * links are deleted before freeing a device node. This ensures we don't
+	 * free any device node that has a non-zero reference count.
+	 */
+	device_link_wait_removal();
+
 	list_for_each_entry_safe_reverse(ce, cen, &ocs->entries, node)
 		__of_changeset_entry_destroy(ce);
 }
-- 
2.44.0


^ permalink raw reply related

* [PATCH 0/2] Input: add ft5426
From: Andreas Kemnade @ 2024-03-24 18:44 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	andreas, o.rempel, dario.binacchi, u.kleine-koenig, hdegoede,
	p.puschmann, linux-input, devicetree, linux-kernel,
	caleb.connolly

Add ft5426 touchscreen controller and the corresponding compatible.

Andreas Kemnade (2):
  dt-bindings: input: touchscreen: edt-ft5x06: Add ft5426
  Input: edt-ft5x06 - add ft5426

 .../devicetree/bindings/input/touchscreen/edt-ft5x06.yaml        | 1 +
 drivers/input/touchscreen/edt-ft5x06.c                           | 1 +
 2 files changed, 2 insertions(+)

-- 
2.39.2


^ permalink raw reply

* Re: [RFC PATCH 2/2] mfd: rohm-bd71828: Add power off functionality
From: Andreas Kemnade @ 2024-03-25 12:16 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: lee, robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
	linux-kernel, imx
In-Reply-To: <472c6eaf-6cbc-484c-bc94-571d115176aa@gmail.com>

On Mon, 25 Mar 2024 13:31:15 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 3/24/24 22:12, Andreas Kemnade wrote:
> > Since the chip can power off the system, add the corresponding
> > functionality.
> > Based on https://github.com/kobolabs/Kobo-Reader/raw/master/hw/imx6sll-clara2e/kernel.tar.bz2
> > No information source about the magic numbers found.  
> 
> Oh, interesting repository :) Thanks for linking to it! I didn't know 
> someone had reworked this driver...
> 
which btw: contains this interesting snippet (output from fdtdump)
  bd71828-i2c@4b {
                    reg = <0x0000004b>;
                    compatible = "rohm,bd71828";
                    gpio_int = <0x00000008 0x00000013 0x00000001>;
                    gpio_wdogb = <0x00000039 0x00000018 0x00000001>;
                    #address-cells = <0x00000001>;
                    #size-cells = <0x00000000>;
                    pmic@4b {
                        compatible = "rohm,bd71828";
             	        regulators {
                        	BUCK1 {
                            		regulator-name = "buck1";


and to make it work since basically no regulators are registered 
instead just some regmap_write()s are done to configure something
in probe(). It is a pitfall to think that the information below pmic@4b
is used, especially since it is not that obvious in the source.

> I have access to the data-sheets so I also have some pieces of 
> information. I hope I can clarify part of the puzzle. Unfortunately I 
> have no information about the magic delays. I guess I could try asking 
> though.
> 
> Oh, it seems to me this handler is only working on BD71828, not on 
> BD71815. So, it should be tied to the ROHM_CHIP_TYPE_BD71828.
> 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> >   drivers/mfd/rohm-bd71828.c | 31 ++++++++++++++++++++++++++++++-
> >   1 file changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
> > index 594718f7e8e1..5a55aa3620d0 100644
> > --- a/drivers/mfd/rohm-bd71828.c
> > +++ b/drivers/mfd/rohm-bd71828.c
> > @@ -464,6 +464,24 @@ static int set_clk_mode(struct device *dev, struct regmap *regmap,
> >   				  OUT32K_MODE_CMOS);
> >   }
> >   
> > +static struct i2c_client *bd71828_dev;  
> 
> I'm not sure why to store pointer to the device and not a pointer to the 
> regmap?
> 
> > +static void bd71828_power_off(void)
> > +{
> > +	i2c_smbus_write_byte_data(bd71828_dev, 0x03, 0xff);  
> 
> 0x03 is a "reset reason" - register. Spec I have states that the 
> register should clear when a reset occurs - but it also says the bits 
> are "write '1' to clear". So, for some reason(?), this clears the 
> previous reset reason.

well, so just check in bootloader what the reset reason is and check if
there is anything odd.

> I am unsure why i2c_smbus_write_byte_data() and 
> not regmap()?
> 
regmap involves mutex_lock() and we are not allowed to sleep here.

> > +	mdelay(500);
> > +	i2c_smbus_write_byte_data(bd71828_dev, BD71828_REG_INT_DCIN2, 0x02);  
> 
> This clears the DCIN monitoring status bit from the IRQ status register. 
> I don't understand the purpose though.
> 
so maybe something to prevent power on by just plugging a usb cable? Will
experiment a bit with it.

> > +	mdelay(500);
> > +	while (true) {
> > +		i2c_smbus_write_byte_data(bd71828_dev, BD71828_REG_PS_CTRL_1, 0x02);  
> 
> This write to PS_CTRL_1 initiates a state transition. 0x2 equals to HBNT 
> state. Eg, in usual cases this should be a start of the power-off sequence.
> 
> > +		mdelay(500);
> > +	}
> > +}  
> 
> If you have the hardware to test this on, then it'd be great to see if 
> clearing the reset reason and IRQ status could be dropped. I can't 
> immediately think of a reason for those.
> 
I will to so. That will also remove the need for all those delays.

> > +static void bd71828_remove_poweroff(void *data)
> > +{
> > +	bd71828_dev = NULL;  
> 
> This does not smell correct to me. Should we remove the 
> bd71828_power_off() from the pm_power_off instead?
> 
oh, yes, that is not correct.

Regards,
Andreas

^ permalink raw reply

* Re: [RFC PATCH 2/2] mfd: rohm-bd71828: Add power off functionality
From: Andreas Kemnade @ 2024-03-25 13:28 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: lee, robh+dt, krzysztof.kozlowski+dt, conor+dt, devicetree,
	linux-kernel, imx
In-Reply-To: <81bf6604-9160-4bae-8da6-7034f8aa3f92@gmail.com>

Hi Matti,

On Mon, 25 Mar 2024 14:44:43 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Hi Andreas,
> 
> On 3/25/24 14:16, Andreas Kemnade wrote:
> > On Mon, 25 Mar 2024 13:31:15 +0200
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> >   
> >> On 3/24/24 22:12, Andreas Kemnade wrote:  
> >>> Since the chip can power off the system, add the corresponding
> >>> functionality.
> >>> Based on https://github.com/kobolabs/Kobo-Reader/raw/master/hw/imx6sll-clara2e/kernel.tar.bz2
> >>> No information source about the magic numbers found.  
> >>
> >> Oh, interesting repository :) Thanks for linking to it! I didn't know
> >> someone had reworked this driver...
> >>  
> > which btw: contains this interesting snippet (output from fdtdump)
> >    bd71828-i2c@4b {
> >                      reg = <0x0000004b>;
> >                      compatible = "rohm,bd71828";
> >                      gpio_int = <0x00000008 0x00000013 0x00000001>;
> >                      gpio_wdogb = <0x00000039 0x00000018 0x00000001>;
> >                      #address-cells = <0x00000001>;
> >                      #size-cells = <0x00000000>;
> >                      pmic@4b {
> >                          compatible = "rohm,bd71828";
> >               	        regulators {
> >                          	BUCK1 {
> >                              		regulator-name = "buck1";
> > 
> > 
> > and to make it work since basically no regulators are registered
> > instead just some regmap_write()s are done to configure something
> > in probe(). It is a pitfall to think that the information below pmic@4b
> > is used, especially since it is not that obvious in the source.  
> 
> Just to ensure there will be no misunderstanding - I have not authored 
> the modifications seen in "Kobo-Reader" repository. Upstream driver does 
> register the regulators - and it does not use the oddly named gpio_int 
> or the gpio_wdogb:

yes, I know it is not from you and I know the general quality in those repos.
But I feel not well pointing to those repositories without issuing the warning
that regulator information in dtb is not an authoritative source for required
regulator settings. Not that someone enables the smoke-generator
functionality...

But lets not go too much offtopic.

Regards,
Andreas


^ permalink raw reply

* [RFC PATCH 1/2] dt-bindings: mfd: Add ROHM BD71828 system-power-controller property
From: Andreas Kemnade @ 2024-03-24 20:12 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, conor+dt, mazziesaccount,
	devicetree, linux-kernel, imx
  Cc: Andreas Kemnade
In-Reply-To: <20240324201210.232301-1-andreas@kemnade.info>

As the PMIC can power off the system, add the corresponding property.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
index 11089aa89ec6..0b62f854bf6b 100644
--- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
@@ -73,6 +73,8 @@ properties:
       used to mark the pins which should not be configured for GPIO. Please see
       the ../gpio/gpio.txt for more information.
 
+  system-power-controller: true
+
 required:
   - compatible
   - reg
-- 
2.39.2


^ permalink raw reply related

* [RFC PATCH 2/2] mfd: rohm-bd71828: Add power off functionality
From: Andreas Kemnade @ 2024-03-24 20:12 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, conor+dt, mazziesaccount,
	devicetree, linux-kernel, imx
  Cc: Andreas Kemnade
In-Reply-To: <20240324201210.232301-1-andreas@kemnade.info>

Since the chip can power off the system, add the corresponding
functionality.
Based on https://github.com/kobolabs/Kobo-Reader/raw/master/hw/imx6sll-clara2e/kernel.tar.bz2
No information source about the magic numbers found.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/mfd/rohm-bd71828.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
index 594718f7e8e1..5a55aa3620d0 100644
--- a/drivers/mfd/rohm-bd71828.c
+++ b/drivers/mfd/rohm-bd71828.c
@@ -464,6 +464,24 @@ static int set_clk_mode(struct device *dev, struct regmap *regmap,
 				  OUT32K_MODE_CMOS);
 }
 
+static struct i2c_client *bd71828_dev;
+static void bd71828_power_off(void)
+{
+	i2c_smbus_write_byte_data(bd71828_dev, 0x03, 0xff);
+	mdelay(500);
+	i2c_smbus_write_byte_data(bd71828_dev, BD71828_REG_INT_DCIN2, 0x02);
+	mdelay(500);
+	while (true) {
+		i2c_smbus_write_byte_data(bd71828_dev, BD71828_REG_PS_CTRL_1, 0x02);
+		mdelay(500);
+	}
+}
+
+static void bd71828_remove_poweroff(void *data)
+{
+	bd71828_dev = NULL;
+}
+
 static int bd71828_i2c_probe(struct i2c_client *i2c)
 {
 	struct regmap_irq_chip_data *irq_data;
@@ -542,7 +560,18 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
 	ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells,
 				   NULL, 0, regmap_irq_get_domain(irq_data));
 	if (ret)
-		dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
+		return	dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
+
+	if (of_device_is_system_power_controller(i2c->dev.of_node)) {
+		if (!pm_power_off) {
+			bd71828_dev = i2c;
+			pm_power_off = bd71828_power_off;
+			ret = devm_add_action_or_reset(&i2c->dev,
+						       bd71828_remove_poweroff,
+						       NULL);
+		} else
+			dev_warn(&i2c->dev, "Poweroff callback already assigned\n");
+	}
 
 	return ret;
 }
-- 
2.39.2


^ permalink raw reply related

* [PATCH v4 0/7] iio: accel: adxl345: Add spi-3wire feature
From: Lothar Rubusch @ 2024-03-25 15:33 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch

Pass a function setup() as pointer from SPI/I2C specific modules
to the core module. Implement setup() to pass the spi-3wire bus
option, if declared in the device-tree.

In the core module, then update data_format register
configuration bits instead of overwriting it. The changes allow
to remove a data_range field.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
V1 -> V2: split into spi-3wire and refactoring
V2 -> V3: split further, focus on needed changesets
V3 -> V4: drop "Remove single info instances";
          split "Group bus configuration" into separat
          comment patch; reorder patch set

Lothar Rubusch (7):
  iio: accel: adxl345: Make data_range obsolete
  iio: accel: adxl345: Group bus configuration
  iio: accel: adxl345: Move defines to header
  dt-bindings: iio: accel: adxl345: Add spi-3wire
  iio: accel: adxl345: Pass function pointer to core
  iio: accel: adxl345: Add comment to probe
  iio: accel: adxl345: Add spi-3wire option

 .../bindings/iio/accel/adi,adxl345.yaml       |  2 +
 drivers/iio/accel/adxl345.h                   | 35 ++++++++++-
 drivers/iio/accel/adxl345_core.c              | 63 ++++++++-----------
 drivers/iio/accel/adxl345_i2c.c               |  2 +-
 drivers/iio/accel/adxl345_spi.c               | 12 +++-
 5 files changed, 74 insertions(+), 40 deletions(-)

-- 
2.25.1


^ permalink raw reply

* [PATCH v4 1/7] iio: accel: adxl345: Make data_range obsolete
From: Lothar Rubusch @ 2024-03-25 15:33 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240325153356.46112-1-l.rubusch@gmail.com>

Replace write() data_format by regmap_update_bits(), because
bus specific pre-configuration may have happened before on
the same register. Changes then need to be masked.

Remove the data_range field from the struct adxl345_data,
because it is not used anymore.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 8bd30a23e..be6758015 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -42,13 +42,13 @@
 #define ADXL345_DATA_FORMAT_4G		1
 #define ADXL345_DATA_FORMAT_8G		2
 #define ADXL345_DATA_FORMAT_16G		3
+#define ADXL345_DATA_FORMAT_MSK		~((u8) BIT(6)) /* ignore spi-3wire */
 
 #define ADXL345_DEVID			0xE5
 
 struct adxl345_data {
 	const struct adxl345_chip_info *info;
 	struct regmap *regmap;
-	u8 data_range;
 };
 
 #define ADXL345_CHANNEL(index, axis) {					\
@@ -219,14 +219,13 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap)
 	data = iio_priv(indio_dev);
 	data->regmap = regmap;
 	/* Enable full-resolution mode */
-	data->data_range = ADXL345_DATA_FORMAT_FULL_RES;
 	data->info = device_get_match_data(dev);
 	if (!data->info)
 		return -ENODEV;
 
-	ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT,
-			   data->data_range);
-	if (ret < 0)
+	ret = regmap_update_bits(regmap, ADXL345_REG_DATA_FORMAT,
+				 ADXL345_DATA_FORMAT_MSK, ADXL345_DATA_FORMAT_FULL_RES);
+	if (ret)
 		return dev_err_probe(dev, ret, "Failed to set data range\n");
 
 	indio_dev->name = data->info->name;
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 2/7] iio: accel: adxl345: Group bus configuration
From: Lothar Rubusch @ 2024-03-25 15:33 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240325153356.46112-1-l.rubusch@gmail.com>

In the probe function group bus configuration and the
indio_dev initialization to improve readability.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index be6758015..469015e9c 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -218,22 +218,23 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap)
 
 	data = iio_priv(indio_dev);
 	data->regmap = regmap;
-	/* Enable full-resolution mode */
+
 	data->info = device_get_match_data(dev);
 	if (!data->info)
 		return -ENODEV;
 
-	ret = regmap_update_bits(regmap, ADXL345_REG_DATA_FORMAT,
-				 ADXL345_DATA_FORMAT_MSK, ADXL345_DATA_FORMAT_FULL_RES);
-	if (ret)
-		return dev_err_probe(dev, ret, "Failed to set data range\n");
-
 	indio_dev->name = data->info->name;
 	indio_dev->info = &adxl345_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = adxl345_channels;
 	indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
 
+	/* Enable full-resolution mode */
+	ret = regmap_update_bits(regmap, ADXL345_REG_DATA_FORMAT,
+				 ADXL345_DATA_FORMAT_MSK, ADXL345_DATA_FORMAT_FULL_RES);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to set data range\n");
+
 	/* Enable measurement mode */
 	ret = adxl345_powerup(data->regmap);
 	if (ret < 0)
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 3/7] iio: accel: adxl345: Move defines to header
From: Lothar Rubusch @ 2024-03-25 15:33 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240325153356.46112-1-l.rubusch@gmail.com>

Move defines to the header file. This keeps the defines block
together in one location, when extended by new shared defines
which then have to be in the header file.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345.h      | 31 +++++++++++++++++++++++++++++++
 drivers/iio/accel/adxl345_core.c | 29 -----------------------------
 2 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index 284bd387c..ee169fed4 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -8,6 +8,37 @@
 #ifndef _ADXL345_H_
 #define _ADXL345_H_
 
+#include <linux/iio/iio.h>
+
+#define ADXL345_REG_DEVID		0x00
+#define ADXL345_REG_OFSX		0x1E
+#define ADXL345_REG_OFSY		0x1F
+#define ADXL345_REG_OFSZ		0x20
+#define ADXL345_REG_OFS_AXIS(index)	(ADXL345_REG_OFSX + (index))
+#define ADXL345_REG_BW_RATE		0x2C
+#define ADXL345_REG_POWER_CTL		0x2D
+#define ADXL345_REG_DATA_FORMAT		0x31
+#define ADXL345_REG_DATAX0		0x32
+#define ADXL345_REG_DATAY0		0x34
+#define ADXL345_REG_DATAZ0		0x36
+#define ADXL345_REG_DATA_AXIS(index)	\
+	(ADXL345_REG_DATAX0 + (index) * sizeof(__le16))
+
+#define ADXL345_BW_RATE			GENMASK(3, 0)
+#define ADXL345_BASE_RATE_NANO_HZ	97656250LL
+
+#define ADXL345_POWER_CTL_MEASURE	BIT(3)
+#define ADXL345_POWER_CTL_STANDBY	0x00
+
+#define ADXL345_DATA_FORMAT_FULL_RES	BIT(3) /* Up to 13-bits resolution */
+#define ADXL345_DATA_FORMAT_2G		0
+#define ADXL345_DATA_FORMAT_4G		1
+#define ADXL345_DATA_FORMAT_8G		2
+#define ADXL345_DATA_FORMAT_16G		3
+#define ADXL345_DATA_FORMAT_MSK		~((u8) BIT(6)) /* ignore spi-3wire */
+
+#define ADXL345_DEVID			0xE5
+
 /*
  * In full-resolution mode, scale factor is maintained at ~4 mg/LSB
  * in all g ranges.
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 469015e9c..eba9c048a 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -17,35 +17,6 @@
 
 #include "adxl345.h"
 
-#define ADXL345_REG_DEVID		0x00
-#define ADXL345_REG_OFSX		0x1e
-#define ADXL345_REG_OFSY		0x1f
-#define ADXL345_REG_OFSZ		0x20
-#define ADXL345_REG_OFS_AXIS(index)	(ADXL345_REG_OFSX + (index))
-#define ADXL345_REG_BW_RATE		0x2C
-#define ADXL345_REG_POWER_CTL		0x2D
-#define ADXL345_REG_DATA_FORMAT		0x31
-#define ADXL345_REG_DATAX0		0x32
-#define ADXL345_REG_DATAY0		0x34
-#define ADXL345_REG_DATAZ0		0x36
-#define ADXL345_REG_DATA_AXIS(index)	\
-	(ADXL345_REG_DATAX0 + (index) * sizeof(__le16))
-
-#define ADXL345_BW_RATE			GENMASK(3, 0)
-#define ADXL345_BASE_RATE_NANO_HZ	97656250LL
-
-#define ADXL345_POWER_CTL_MEASURE	BIT(3)
-#define ADXL345_POWER_CTL_STANDBY	0x00
-
-#define ADXL345_DATA_FORMAT_FULL_RES	BIT(3) /* Up to 13-bits resolution */
-#define ADXL345_DATA_FORMAT_2G		0
-#define ADXL345_DATA_FORMAT_4G		1
-#define ADXL345_DATA_FORMAT_8G		2
-#define ADXL345_DATA_FORMAT_16G		3
-#define ADXL345_DATA_FORMAT_MSK		~((u8) BIT(6)) /* ignore spi-3wire */
-
-#define ADXL345_DEVID			0xE5
-
 struct adxl345_data {
 	const struct adxl345_chip_info *info;
 	struct regmap *regmap;
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 4/7] dt-bindings: iio: accel: adxl345: Add spi-3wire
From: Lothar Rubusch @ 2024-03-25 15:33 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240325153356.46112-1-l.rubusch@gmail.com>

Add spi-3wire because the driver optionally supports spi-3wire.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml
index 07cacc3f6..280ed479e 100644
--- a/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl345.yaml
@@ -32,6 +32,8 @@ properties:
 
   spi-cpol: true
 
+  spi-3wire: true
+
   interrupts:
     maxItems: 1
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 5/7] iio: accel: adxl345: Pass function pointer to core
From: Lothar Rubusch @ 2024-03-25 15:33 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240325153356.46112-1-l.rubusch@gmail.com>

Add a function pointer argument to the probe function in
the core module to provide a way to pre-configure the bus.

The passed setup function can be prepared in the bus
specific spi or the i2c module, or NULL otherwise. It shall
then be executed in the bus independent core module.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345.h      |  3 ++-
 drivers/iio/accel/adxl345_core.c | 10 +++++++++-
 drivers/iio/accel/adxl345_i2c.c  |  2 +-
 drivers/iio/accel/adxl345_spi.c  |  2 +-
 4 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index ee169fed4..620a2e0f0 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -59,6 +59,7 @@ struct adxl345_chip_info {
 	int uscale;
 };
 
-int adxl345_core_probe(struct device *dev, struct regmap *regmap);
+int adxl345_core_probe(struct device *dev, struct regmap *regmap,
+		       int (*setup)(struct device*, struct regmap*));
 
 #endif /* _ADXL345_H_ */
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index eba9c048a..476d729bc 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -168,13 +168,21 @@ static void adxl345_powerdown(void *regmap)
 	regmap_write(regmap, ADXL345_REG_POWER_CTL, ADXL345_POWER_CTL_STANDBY);
 }
 
-int adxl345_core_probe(struct device *dev, struct regmap *regmap)
+int adxl345_core_probe(struct device *dev, struct regmap *regmap,
+	int (*setup)(struct device*, struct regmap*))
 {
 	struct adxl345_data *data;
 	struct iio_dev *indio_dev;
 	u32 regval;
 	int ret;
 
+	/* Perform optional initial bus specific configuration */
+	if (setup) {
+		ret = setup(dev, regmap);
+		if (ret)
+			return ret;
+	}
+
 	ret = regmap_read(regmap, ADXL345_REG_DEVID, &regval);
 	if (ret < 0)
 		return dev_err_probe(dev, ret, "Error reading device ID\n");
diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c
index a3084b0a8..4065b8f7c 100644
--- a/drivers/iio/accel/adxl345_i2c.c
+++ b/drivers/iio/accel/adxl345_i2c.c
@@ -27,7 +27,7 @@ static int adxl345_i2c_probe(struct i2c_client *client)
 	if (IS_ERR(regmap))
 		return dev_err_probe(&client->dev, PTR_ERR(regmap), "Error initializing regmap\n");
 
-	return adxl345_core_probe(&client->dev, regmap);
+	return adxl345_core_probe(&client->dev, regmap, NULL);
 }
 
 static const struct adxl345_chip_info adxl345_i2c_info = {
diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
index 93ca349f1..1c0513bd3 100644
--- a/drivers/iio/accel/adxl345_spi.c
+++ b/drivers/iio/accel/adxl345_spi.c
@@ -33,7 +33,7 @@ static int adxl345_spi_probe(struct spi_device *spi)
 	if (IS_ERR(regmap))
 		return dev_err_probe(&spi->dev, PTR_ERR(regmap), "Error initializing regmap\n");
 
-	return adxl345_core_probe(&spi->dev, regmap);
+	return adxl345_core_probe(&spi->dev, regmap, NULL);
 }
 
 static const struct adxl345_chip_info adxl345_spi_info = {
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 6/7] iio: accel: adxl345: Add comment to probe
From: Lothar Rubusch @ 2024-03-25 15:33 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240325153356.46112-1-l.rubusch@gmail.com>

Add a comment to the probe() function to describe the
passed function pointer setup in particular.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 476d729bc..cc3da4ece 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -168,6 +168,16 @@ static void adxl345_powerdown(void *regmap)
 	regmap_write(regmap, ADXL345_REG_POWER_CTL, ADXL345_POWER_CTL_STANDBY);
 }
 
+/**
+ * adxl345_core_probe() - probe and setup for the adxl345 accelerometer,
+ *                        also covers the adlx375 accelerometer
+ * @dev:	Driver model representation of the device
+ * @regmap:	Regmap instance for the device
+ * @setup:	Setup routine to be executed right before the standard device
+ *		setup
+ *
+ * Return: 0 on success, negative errno on error
+ */
 int adxl345_core_probe(struct device *dev, struct regmap *regmap,
 	int (*setup)(struct device*, struct regmap*))
 {
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 7/7] iio: accel: adxl345: Add spi-3wire option
From: Lothar Rubusch @ 2024-03-25 15:33 UTC (permalink / raw)
  To: lars, Michael.Hennerich, jic23, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-iio, devicetree, linux-kernel, eraretuya, l.rubusch
In-Reply-To: <20240325153356.46112-1-l.rubusch@gmail.com>

Add a setup function implementation to the spi module to enable
spi-3wire as option when specified in the device-tree.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345.h     |  1 +
 drivers/iio/accel/adxl345_spi.c | 12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index 620a2e0f0..55a72ca38 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -36,6 +36,7 @@
 #define ADXL345_DATA_FORMAT_8G		2
 #define ADXL345_DATA_FORMAT_16G		3
 #define ADXL345_DATA_FORMAT_MSK		~((u8) BIT(6)) /* ignore spi-3wire */
+#define ADXL345_DATA_FORMAT_SPI_3WIRE	BIT(6)
 
 #define ADXL345_DEVID			0xE5
 
diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
index 1c0513bd3..f145d5c1d 100644
--- a/drivers/iio/accel/adxl345_spi.c
+++ b/drivers/iio/accel/adxl345_spi.c
@@ -20,6 +20,16 @@ static const struct regmap_config adxl345_spi_regmap_config = {
 	.read_flag_mask = BIT(7) | BIT(6),
 };
 
+static int adxl345_spi_setup(struct device *dev, struct regmap *regmap)
+{
+	struct spi_device *spi = container_of(dev, struct spi_device, dev);
+
+	if (spi->mode & SPI_3WIRE)
+		return regmap_write(regmap, ADXL345_REG_DATA_FORMAT,
+				    ADXL345_DATA_FORMAT_SPI_3WIRE);
+	return 0;
+}
+
 static int adxl345_spi_probe(struct spi_device *spi)
 {
 	struct regmap *regmap;
@@ -33,7 +43,7 @@ static int adxl345_spi_probe(struct spi_device *spi)
 	if (IS_ERR(regmap))
 		return dev_err_probe(&spi->dev, PTR_ERR(regmap), "Error initializing regmap\n");
 
-	return adxl345_core_probe(&spi->dev, regmap, NULL);
+	return adxl345_core_probe(&spi->dev, regmap, adxl345_spi_setup);
 }
 
 static const struct adxl345_chip_info adxl345_spi_info = {
-- 
2.25.1


^ permalink raw reply related

* [net-next,v4 0/2] ravb: Support describing the MDIO bus
From: Niklas Söderlund @ 2024-03-25 15:34 UTC (permalink / raw)
  To: Sergey Shtylyov, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Claudiu Beznea, Yoshihiro Shimoda, Biju Das,
	netdev, devicetree
  Cc: linux-renesas-soc, Niklas Söderlund

Hello,

This series adds support to the binding and driver of the Renesas
Ethernet AVB to described the MDIO bus. Currently the driver uses the OF
node of the device itself when registering the MDIO bus. This forces any
MDIO bus properties the MDIO core should react on to be set on the
device OF node. This is confusing and non of the MDIO bus properties are
described in the Ethernet AVB bindings.

Patch 1/2 extends the bindings with an optional mdio child-node to the
device that can be used to contain the MDIO bus settings. While patch
2/2 changes the driver to use this node (if present) when registering
the MDIO bus.

If the new optional mdio child-node is not present the driver fallback
to the old behavior and uses the device OF node like before. This change
is fully backward compatible with existing usage of the bindings.

For changelog see individual patches.

Niklas Söderlund (2):
  dt-bindings: net: renesas,etheravb: Add optional MDIO bus node
  ravb: Add support for an optional MDIO mode

 .../devicetree/bindings/net/renesas,etheravb.yaml    | 12 ++++++++++--
 drivers/net/ethernet/renesas/ravb_main.c             |  9 ++++++++-
 2 files changed, 18 insertions(+), 3 deletions(-)

-- 
2.44.0


^ permalink raw reply

* [net-next,v4 1/2] dt-bindings: net: renesas,etheravb: Add optional MDIO bus node
From: Niklas Söderlund @ 2024-03-25 15:34 UTC (permalink / raw)
  To: Sergey Shtylyov, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Claudiu Beznea, Yoshihiro Shimoda, Biju Das,
	netdev, devicetree
  Cc: linux-renesas-soc, Niklas Söderlund, Rob Herring
In-Reply-To: <20240325153451.2366083-1-niklas.soderlund+renesas@ragnatech.se>

The Renesas Ethernet AVB bindings do not allow the MDIO bus to be
described. This has not been needed as only a single PHY is
supported and no MDIO bus properties have been needed.

Add an optional mdio node to the binding which allows the MDIO bus to be
described and allow bus properties to be set.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
---
* Changes since v3
- Marked the '#address-cells' and '#size-cells' properties as depricated
  as well as making them optional.

* Changes since v2
- Mark the PHY pattern as deprecated and add a comment that new bindings
  should describe the PHY inside the MDIO node.
---
 .../devicetree/bindings/net/renesas,etheravb.yaml    | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/renesas,etheravb.yaml b/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
index de7ba7f345a9..21a92f179093 100644
--- a/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
+++ b/Documentation/devicetree/bindings/net/renesas,etheravb.yaml
@@ -88,10 +88,16 @@ properties:
   '#address-cells':
     description: Number of address cells for the MDIO bus.
     const: 1
+    deprecated: true
 
   '#size-cells':
     description: Number of size cells on the MDIO bus.
     const: 0
+    deprecated: true
+
+  mdio:
+    $ref: /schemas/net/mdio.yaml#
+    unevaluatedProperties: false
 
   renesas,no-ether-link:
     type: boolean
@@ -110,9 +116,13 @@ properties:
   tx-internal-delay-ps:
     enum: [0, 2000]
 
+# In older bindings there where no mdio child-node to describe the MDIO bus
+# and the PHY. To not fail older bindings accept any node with an address. New
+# users should describe the PHY inside the mdio child-node.
 patternProperties:
   "@[0-9a-f]$":
     type: object
+    deprecated: true
 
 required:
   - compatible
@@ -123,8 +133,6 @@ required:
   - resets
   - phy-mode
   - phy-handle
-  - '#address-cells'
-  - '#size-cells'
 
 allOf:
   - $ref: ethernet-controller.yaml#
-- 
2.44.0


^ permalink raw reply related

* [net-next,v4 2/2] ravb: Add support for an optional MDIO mode
From: Niklas Söderlund @ 2024-03-25 15:34 UTC (permalink / raw)
  To: Sergey Shtylyov, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Claudiu Beznea, Yoshihiro Shimoda, Biju Das,
	netdev, devicetree
  Cc: linux-renesas-soc, Niklas Söderlund, Andrew Lunn
In-Reply-To: <20240325153451.2366083-1-niklas.soderlund+renesas@ragnatech.se>

The driver used the DT node of the device itself when registering the
MDIO bus. While this works, it creates a problem: it forces any MDIO bus
properties to also be set on the devices DT node. This mixes the
properties of two distinctly different things and is confusing.

This change adds support for an optional mdio node to be defined as a
child to the device DT node. The child node can then be used to describe
MDIO bus properties that the MDIO core can act on when registering the
bus.

If no mdio child node is found the driver fallback to the old behavior
and register the MDIO bus using the device DT node. This change is
backward compatible with old bindings in use.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
* Changes since v3
- None

* Changes since v2
- Use DT node instead of OF ode in commit message.
- Update punctuation in commit message.
- Simplify the node selection in the driver.
---
 drivers/net/ethernet/renesas/ravb_main.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index d1be030c8848..9a397f9b4c0c 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2564,6 +2564,7 @@ static int ravb_mdio_init(struct ravb_private *priv)
 {
 	struct platform_device *pdev = priv->pdev;
 	struct device *dev = &pdev->dev;
+	struct device_node *mdio_node;
 	struct phy_device *phydev;
 	struct device_node *pn;
 	int error;
@@ -2583,7 +2584,13 @@ static int ravb_mdio_init(struct ravb_private *priv)
 		 pdev->name, pdev->id);
 
 	/* Register MDIO bus */
-	error = of_mdiobus_register(priv->mii_bus, dev->of_node);
+	mdio_node = of_get_child_by_name(dev->of_node, "mdio");
+	if (!mdio_node) {
+		/* backwards compatibility for DT lacking mdio subnode */
+		mdio_node = of_node_get(dev->of_node);
+	}
+	error = of_mdiobus_register(priv->mii_bus, mdio_node);
+	of_node_put(mdio_node);
 	if (error)
 		goto out_free_bus;
 
-- 
2.44.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox