From: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> To: "Rafał Miłecki" <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>, "Florian Fainelli" <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "Rafał Miłecki" <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Cc: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>, Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>, Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: Re: [PATCH] ARM: dts: BCM5301X: Specify MDIO bus in the DT Date: Wed, 19 Apr 2017 09:43:06 -0700 [thread overview] Message-ID: <94250925-7d5d-ac31-58ad-918406d904f1@gmail.com> (raw) In-Reply-To: <c3a0455a-821f-d8a8-bf8a-7ad06ad0466e-g1n6cQUeyibVItvQsEIGlw@public.gmane.org> On 04/02/2017 02:25 PM, Rafał Miłecki wrote: > On 04/02/2017 11:14 PM, Florian Fainelli wrote: >> Le 04/02/17 à 14:08, Rafał Miłecki a écrit : >>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org> >>> >>> Northstar devices have MDIO bus that may contain various PHYs attached. >>> A common example is USB 3.0 PHY (that doesn't have an MDIO driver yet). >>> >>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org> >>> --- >>> arch/arm/boot/dts/bcm5301x.dtsi | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi >>> b/arch/arm/boot/dts/bcm5301x.dtsi >>> index acee36a61004..6a2afe7880ae 100644 >>> --- a/arch/arm/boot/dts/bcm5301x.dtsi >>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi >>> @@ -320,6 +320,13 @@ >>> }; >>> }; >>> >>> + mdio@18003000 { >>> + compatible = "brcm,iproc-mdio"; >>> + reg = <0x18003000 0x8>; >>> + #size-cells = <1>; >>> + #address-cells = <0>; >>> + }; >> >> This looks fine, but usually the block should be enabled on a per-board >> basis, such that there should be a status = "disabled" property here by >> default. > > I think we have few blocks in bcm5301x.dtsi enabled by default. I guess > it's > for stuff that is always present on every SoC family board: rng, nand, > spi to > name few. > > It makes some sense, consider e.g. spi. Every Northstar board has SPI > controller so it's enabled by default. Not every board has SPI flash, so > it's > disabled by default. > > It's there and it make sense to me. Is that OK or not? Even though there are devices that are always enabled on a given SoC, because the board designs are always consistent does not necessarily make them good candidates to be enabled at the .dtsi level. This is particularly true when there are external connections to blocks (SPI, NAND, USB, Ethernet, MDIO to name a few), having them disabled by default is safer as a starting point to begin with. > > I find MDIO situation quite simiar. It seems every Northstar board has > MDIO bus > just devices may differ and should not be enabled by default. In which case, the only difference, for you would be to do to, at the board-level DTS: &mdio { status = "okay"; phy@0 { reg = <0>; ... }; }; versus: &mdio { phy@0 { reg = <0>; ... }; }; I think we can afford putting the mdio node's status property in each board-level DTS and make it clear that way that it is enabled because there are child nodes enabled? NB: with a CONFIG_OF system, there is no automatic probing of MDIO child devices because it relies on child nodes being declared, but you would still get the driver to be probed and enabled, which is a waste of resources at best. Thanks -- Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: f.fainelli@gmail.com (Florian Fainelli) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] ARM: dts: BCM5301X: Specify MDIO bus in the DT Date: Wed, 19 Apr 2017 09:43:06 -0700 [thread overview] Message-ID: <94250925-7d5d-ac31-58ad-918406d904f1@gmail.com> (raw) In-Reply-To: <c3a0455a-821f-d8a8-bf8a-7ad06ad0466e@milecki.pl> On 04/02/2017 02:25 PM, Rafa? Mi?ecki wrote: > On 04/02/2017 11:14 PM, Florian Fainelli wrote: >> Le 04/02/17 ? 14:08, Rafa? Mi?ecki a ?crit : >>> From: Rafa? Mi?ecki <rafal@milecki.pl> >>> >>> Northstar devices have MDIO bus that may contain various PHYs attached. >>> A common example is USB 3.0 PHY (that doesn't have an MDIO driver yet). >>> >>> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl> >>> --- >>> arch/arm/boot/dts/bcm5301x.dtsi | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi >>> b/arch/arm/boot/dts/bcm5301x.dtsi >>> index acee36a61004..6a2afe7880ae 100644 >>> --- a/arch/arm/boot/dts/bcm5301x.dtsi >>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi >>> @@ -320,6 +320,13 @@ >>> }; >>> }; >>> >>> + mdio at 18003000 { >>> + compatible = "brcm,iproc-mdio"; >>> + reg = <0x18003000 0x8>; >>> + #size-cells = <1>; >>> + #address-cells = <0>; >>> + }; >> >> This looks fine, but usually the block should be enabled on a per-board >> basis, such that there should be a status = "disabled" property here by >> default. > > I think we have few blocks in bcm5301x.dtsi enabled by default. I guess > it's > for stuff that is always present on every SoC family board: rng, nand, > spi to > name few. > > It makes some sense, consider e.g. spi. Every Northstar board has SPI > controller so it's enabled by default. Not every board has SPI flash, so > it's > disabled by default. > > It's there and it make sense to me. Is that OK or not? Even though there are devices that are always enabled on a given SoC, because the board designs are always consistent does not necessarily make them good candidates to be enabled at the .dtsi level. This is particularly true when there are external connections to blocks (SPI, NAND, USB, Ethernet, MDIO to name a few), having them disabled by default is safer as a starting point to begin with. > > I find MDIO situation quite simiar. It seems every Northstar board has > MDIO bus > just devices may differ and should not be enabled by default. In which case, the only difference, for you would be to do to, at the board-level DTS: &mdio { status = "okay"; phy at 0 { reg = <0>; ... }; }; versus: &mdio { phy at 0 { reg = <0>; ... }; }; I think we can afford putting the mdio node's status property in each board-level DTS and make it clear that way that it is enabled because there are child nodes enabled? NB: with a CONFIG_OF system, there is no automatic probing of MDIO child devices because it relies on child nodes being declared, but you would still get the driver to be probed and enabled, which is a waste of resources at best. Thanks -- Florian
next prev parent reply other threads:[~2017-04-19 16:43 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-04-02 21:08 [PATCH] ARM: dts: BCM5301X: Specify MDIO bus in the DT Rafał Miłecki 2017-04-02 21:08 ` Rafał Miłecki [not found] ` <20170402210840.11429-1-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-04-02 21:14 ` Florian Fainelli 2017-04-02 21:14 ` Florian Fainelli [not found] ` <5a6ecda9-2cf6-2171-7210-6b3a1d4c6012-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-04-02 21:25 ` Rafał Miłecki 2017-04-02 21:25 ` Rafał Miłecki [not found] ` <c3a0455a-821f-d8a8-bf8a-7ad06ad0466e-g1n6cQUeyibVItvQsEIGlw@public.gmane.org> 2017-04-19 16:43 ` Florian Fainelli [this message] 2017-04-19 16:43 ` Florian Fainelli [not found] ` <94250925-7d5d-ac31-58ad-918406d904f1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-04-19 17:35 ` Rafał Miłecki 2017-04-19 17:35 ` Rafał Miłecki [not found] ` <cd324440-ad88-0981-1577-822d39ee289d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-04-19 17:52 ` Florian Fainelli 2017-04-19 17:52 ` Florian Fainelli 2017-04-19 18:10 ` Rafał Miłecki 2017-04-19 18:10 ` Rafał Miłecki [not found] ` <ab696d24-3544-a554-82d1-18839b29caa0-g1n6cQUeyibVItvQsEIGlw@public.gmane.org> 2017-04-19 18:13 ` Florian Fainelli 2017-04-19 18:13 ` Florian Fainelli 2017-04-19 21:54 ` [PATCH V2] " Rafał Miłecki 2017-04-19 21:54 ` Rafał Miłecki
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=94250925-7d5d-ac31-58ad-918406d904f1@gmail.com \ --to=f.fainelli-re5jqeeqqe8avxtiumwx3w@public.gmane.org \ --cc=bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org \ --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org \ --cc=linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org \ --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \ --cc=rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org \ --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \ --cc=zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.