| Message ID | 1394713963-18300-1-git-send-email-ulf.hansson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 13, 2014 at 01:32:43PM +0100, Ulf Hansson wrote: > As the first step in preparing the mmci driver for converting to the > mmc_of_parse API, let's align to the common names of DT bindings for > the mmc/sd highspeed modes. NAK. These bindings have been documented as being there since March 14th 2012, and therefore need to be supported for ever by the driver. You can _augment_ the bindings with the generic ones, and change the DT files, but you can't remove the parsing of the old property names.
On 13 March 2014 18:47, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Thu, Mar 13, 2014 at 01:32:43PM +0100, Ulf Hansson wrote: >> As the first step in preparing the mmci driver for converting to the >> mmc_of_parse API, let's align to the common names of DT bindings for >> the mmc/sd highspeed modes. > > NAK. These bindings have been documented as being there since March > 14th 2012, and therefore need to be supported for ever by the driver. > You can _augment_ the bindings with the generic ones, and change the > DT files, but you can't remove the parsing of the old property names. I was kind of expecting this response. :-) So, since we made a mistake about adding these DT bindings we are now unable to remove them, is there really no way back? In this particular case, I am confident that it should be safe to remove them, but I guess this is more matter of principle, right? Kind regards Ulf Hansson > > -- > FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly > improving, and getting towards what was expected from it.
On Fri, Mar 14, 2014 at 8:27 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 13 March 2014 18:47, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >> On Thu, Mar 13, 2014 at 01:32:43PM +0100, Ulf Hansson wrote: >>> As the first step in preparing the mmci driver for converting to the >>> mmc_of_parse API, let's align to the common names of DT bindings for >>> the mmc/sd highspeed modes. >> >> NAK. These bindings have been documented as being there since March >> 14th 2012, and therefore need to be supported for ever by the driver. >> You can _augment_ the bindings with the generic ones, and change the >> DT files, but you can't remove the parsing of the old property names. > > I was kind of expecting this response. :-) > > So, since we made a mistake about adding these DT bindings we are now > unable to remove them, is there really no way back? > > In this particular case, I am confident that it should be safe to > remove them, but I guess this is more matter of principle, right? I can guarantee that there are no deployed U300, Nomadik or Ux500 systems out there with DTBs deployed in them, and for certain no products. At one point that was the criterion... All setups actually use the appended device tree. In worst case I guess mmc-cap-sd-highspeed and mmc-cap-mmc-highspeed could be put as aliases into the generic parser? Yours, Linus Walleij
On 14 March 2014 11:58, Linus Walleij <linus.walleij@linaro.org> wrote: > On Fri, Mar 14, 2014 at 8:27 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> On 13 March 2014 18:47, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >>> On Thu, Mar 13, 2014 at 01:32:43PM +0100, Ulf Hansson wrote: >>>> As the first step in preparing the mmci driver for converting to the >>>> mmc_of_parse API, let's align to the common names of DT bindings for >>>> the mmc/sd highspeed modes. >>> >>> NAK. These bindings have been documented as being there since March >>> 14th 2012, and therefore need to be supported for ever by the driver. >>> You can _augment_ the bindings with the generic ones, and change the >>> DT files, but you can't remove the parsing of the old property names. >> >> I was kind of expecting this response. :-) >> >> So, since we made a mistake about adding these DT bindings we are now >> unable to remove them, is there really no way back? >> >> In this particular case, I am confident that it should be safe to >> remove them, but I guess this is more matter of principle, right? > > I can guarantee that there are no deployed U300, Nomadik or > Ux500 systems out there with DTBs deployed in them, and > for certain no products. At one point that was the criterion... > All setups actually use the appended device tree. It sure seems like it's safe to remove them. If you decide to accept the patch, could we queue it through ARM SOC with your help Linus? Maybe even for 3.15 as the sooner the better!? > > In worst case I guess mmc-cap-sd-highspeed and > mmc-cap-mmc-highspeed could be put as aliases into the > generic parser? That's an option - and maybe mark them as deprecated. Still I would prefer if we could remove them instead. Kind regards Ulf Hansson > > Yours, > Linus Walleij
On Mon, Mar 17, 2014 at 11:01 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 14 March 2014 11:58, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Fri, Mar 14, 2014 at 8:27 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> On 13 March 2014 18:47, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >>>> NAK. These bindings have been documented as being there since March >>>> 14th 2012, and therefore need to be supported for ever by the driver. >>>> You can _augment_ the bindings with the generic ones, and change the >>>> DT files, but you can't remove the parsing of the old property names. >>> >>> I was kind of expecting this response. :-) >>> >>> So, since we made a mistake about adding these DT bindings we are now >>> unable to remove them, is there really no way back? >>> >>> In this particular case, I am confident that it should be safe to >>> remove them, but I guess this is more matter of principle, right? >> >> I can guarantee that there are no deployed U300, Nomadik or >> Ux500 systems out there with DTBs deployed in them, and >> for certain no products. At one point that was the criterion... >> All setups actually use the appended device tree. > > It sure seems like it's safe to remove them. > > If you decide to accept the patch, could we queue it through ARM SOC > with your help Linus? Maybe even for 3.15 as the sooner the better!? Actually I think I would want some indication from the device tree maintainers on this. After all they are safeguarding the DT process. I am however worried about the lack of response from that camp at times, this is supposed to be a simple yes/no question :-/ DT folks?? Yours, Linus Walleij
On Mon, Mar 17, 2014 at 02:00:58PM +0000, Linus Walleij wrote: > On Mon, Mar 17, 2014 at 11:01 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On 14 March 2014 11:58, Linus Walleij <linus.walleij@linaro.org> wrote: > >> On Fri, Mar 14, 2014 at 8:27 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > >>> On 13 March 2014 18:47, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > >>>> NAK. These bindings have been documented as being there since March > >>>> 14th 2012, and therefore need to be supported for ever by the driver. > >>>> You can _augment_ the bindings with the generic ones, and change the > >>>> DT files, but you can't remove the parsing of the old property names. > >>> > >>> I was kind of expecting this response. :-) > >>> > >>> So, since we made a mistake about adding these DT bindings we are now > >>> unable to remove them, is there really no way back? > >>> > >>> In this particular case, I am confident that it should be safe to > >>> remove them, but I guess this is more matter of principle, right? Basically, yes. Our default position is to not break existing DTBs. In general that's beneficial, ensures stability, and keeps people in the right mindset. Sticking to the position even where we could be a little softer is a way of keeping people honest -- practically everyone has a binding (or seventy) which they hate and would love to change, sticking to the rules (even when painful) saves us from endless churn for little benefit. In this case the binding isn't broken; it provides the information we need, but just not with our preferred names. As Russell has pointed out, we can support the more standard names in addition to the current ones (which we can deprecate for new DTs). Forcibly breaking existing DTBs is not necessary, and I'm not sure if it's worthwhile for the sake of a few bytes. There are bindings out there which are fundamentally broken, which are always reliant on platform data or just don't currently work. Those are what we need to focus on fixing to ensure long-term support. We also need a strategy for binding deprecation, which we do not have currently. There was talk of unstable bindings at the last mini-summit to allow people to tinker without committing to long-term support of bindings, but nothing seems to have happened on that front. It would be nice to have a plan for dealing with all these vestigal bits long term. Thanks, Mark.
On 17 March 2014 17:15, Mark Rutland <mark.rutland@arm.com> wrote: > On Mon, Mar 17, 2014 at 02:00:58PM +0000, Linus Walleij wrote: >> On Mon, Mar 17, 2014 at 11:01 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> > On 14 March 2014 11:58, Linus Walleij <linus.walleij@linaro.org> wrote: >> >> On Fri, Mar 14, 2014 at 8:27 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> >>> On 13 March 2014 18:47, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >> >> >>>> NAK. These bindings have been documented as being there since March >> >>>> 14th 2012, and therefore need to be supported for ever by the driver. >> >>>> You can _augment_ the bindings with the generic ones, and change the >> >>>> DT files, but you can't remove the parsing of the old property names. >> >>> >> >>> I was kind of expecting this response. :-) >> >>> >> >>> So, since we made a mistake about adding these DT bindings we are now >> >>> unable to remove them, is there really no way back? >> >>> >> >>> In this particular case, I am confident that it should be safe to >> >>> remove them, but I guess this is more matter of principle, right? > > Basically, yes. > > Our default position is to not break existing DTBs. In general that's > beneficial, ensures stability, and keeps people in the right mindset. > Sticking to the position even where we could be a little softer is a way > of keeping people honest -- practically everyone has a binding (or > seventy) which they hate and would love to change, sticking to the rules > (even when painful) saves us from endless churn for little benefit. > > In this case the binding isn't broken; it provides the information we > need, but just not with our preferred names. As Russell has pointed out, > we can support the more standard names in addition to the current ones > (which we can deprecate for new DTs). Forcibly breaking existing DTBs is > not necessary, and I'm not sure if it's worthwhile for the sake of a few > bytes. I get the idea, let's keep the bindings then. > > There are bindings out there which are fundamentally broken, which are > always reliant on platform data or just don't currently work. Those are > what we need to focus on fixing to ensure long-term support. > > We also need a strategy for binding deprecation, which we do not have > currently. There was talk of unstable bindings at the last mini-summit > to allow people to tinker without committing to long-term support of > bindings, but nothing seems to have happened on that front. It would be > nice to have a plan for dealing with all these vestigal bits long term. I suppose adding a comment about a binding being deprecated in the documentation - is the best we can do for now? Thanks for your response Mark! Kind regards Ulf Hansson > > Thanks, > Mark.
diff --git a/Documentation/devicetree/bindings/mmc/mmci.txt b/Documentation/devicetree/bindings/mmc/mmci.txt index 2b584ca..0c463e0 100644 --- a/Documentation/devicetree/bindings/mmc/mmci.txt +++ b/Documentation/devicetree/bindings/mmc/mmci.txt @@ -9,7 +9,3 @@ by mmc.txt and the properties used by the mmci driver. Required properties: - compatible : contains "arm,pl18x", "arm,primecell". - arm,primecell-periphid : contains the PrimeCell Peripheral ID. - -Optional properties: -- mmc-cap-mmc-highspeed : indicates whether MMC is high speed capable -- mmc-cap-sd-highspeed : indicates whether SD is high speed capable diff --git a/arch/arm/boot/dts/ste-ccu9540.dts b/arch/arm/boot/dts/ste-ccu9540.dts index 2295087..651c56d 100644 --- a/arch/arm/boot/dts/ste-ccu9540.dts +++ b/arch/arm/boot/dts/ste-ccu9540.dts @@ -38,8 +38,8 @@ arm,primecell-periphid = <0x10480180>; max-frequency = <100000000>; bus-width = <4>; - mmc-cap-sd-highspeed; - mmc-cap-mmc-highspeed; + cap-sd-highspeed; + cap-mmc-highspeed; vmmc-supply = <&ab8500_ldo_aux3_reg>; cd-gpios = <&gpio7 6 0x4>; // 230 @@ -63,7 +63,7 @@ arm,primecell-periphid = <0x10480180>; max-frequency = <100000000>; bus-width = <8>; - mmc-cap-mmc-highspeed; + cap-mmc-highspeed; vmmc-supply = <&ab8500_ldo_aux2_reg>; status = "okay"; diff --git a/arch/arm/boot/dts/ste-href.dtsi b/arch/arm/boot/dts/ste-href.dtsi index 6cb9b68..a5db487 100644 --- a/arch/arm/boot/dts/ste-href.dtsi +++ b/arch/arm/boot/dts/ste-href.dtsi @@ -116,8 +116,8 @@ arm,primecell-periphid = <0x10480180>; max-frequency = <100000000>; bus-width = <4>; - mmc-cap-sd-highspeed; - mmc-cap-mmc-highspeed; + cap-sd-highspeed; + cap-mmc-highspeed; vmmc-supply = <&ab8500_ldo_aux3_reg>; vqmmc-supply = <&vmmci>; pinctrl-names = "default", "sleep"; @@ -144,7 +144,7 @@ arm,primecell-periphid = <0x10480180>; max-frequency = <100000000>; bus-width = <8>; - mmc-cap-mmc-highspeed; + cap-mmc-highspeed; pinctrl-names = "default", "sleep"; pinctrl-0 = <&sdi2_default_mode>; pinctrl-1 = <&sdi2_sleep_mode>; @@ -157,7 +157,7 @@ arm,primecell-periphid = <0x10480180>; max-frequency = <100000000>; bus-width = <8>; - mmc-cap-mmc-highspeed; + cap-mmc-highspeed; vmmc-supply = <&ab8500_ldo_aux2_reg>; pinctrl-names = "default", "sleep"; pinctrl-0 = <&sdi4_default_mode>; diff --git a/arch/arm/boot/dts/ste-nomadik-stn8815.dtsi b/arch/arm/boot/dts/ste-nomadik-stn8815.dtsi index 5acc044..d316c95 100644 --- a/arch/arm/boot/dts/ste-nomadik-stn8815.dtsi +++ b/arch/arm/boot/dts/ste-nomadik-stn8815.dtsi @@ -840,8 +840,8 @@ interrupts = <22>; max-frequency = <48000000>; bus-width = <4>; - mmc-cap-mmc-highspeed; - mmc-cap-sd-highspeed; + cap-mmc-highspeed; + cap-sd-highspeed; cd-gpios = <&gpio3 15 0x1>; cd-inverted; pinctrl-names = "default"; diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts index 97d5d21..4add51f 100644 --- a/arch/arm/boot/dts/ste-snowball.dts +++ b/arch/arm/boot/dts/ste-snowball.dts @@ -155,7 +155,7 @@ arm,primecell-periphid = <0x10480180>; max-frequency = <100000000>; bus-width = <4>; - mmc-cap-mmc-highspeed; + cap-mmc-highspeed; vmmc-supply = <&ab8500_ldo_aux3_reg>; vqmmc-supply = <&vmmci>; pinctrl-names = "default", "sleep"; @@ -194,7 +194,7 @@ arm,primecell-periphid = <0x10480180>; max-frequency = <100000000>; bus-width = <8>; - mmc-cap-mmc-highspeed; + cap-mmc-highspeed; vmmc-supply = <&ab8500_ldo_aux2_reg>; pinctrl-names = "default", "sleep"; pinctrl-0 = <&sdi4_default_mode>; diff --git a/arch/arm/boot/dts/ste-u300.dts b/arch/arm/boot/dts/ste-u300.dts index a9da480..0a839da 100644 --- a/arch/arm/boot/dts/ste-u300.dts +++ b/arch/arm/boot/dts/ste-u300.dts @@ -442,8 +442,8 @@ clock-names = "apb_pclk", "mclk"; max-frequency = <24000000>; bus-width = <4>; // SD-card slot - mmc-cap-mmc-highspeed; - mmc-cap-sd-highspeed; + cap-mmc-highspeed; + cap-sd-highspeed; cd-gpios = <&gpio 12 0x4>; cd-inverted; vmmc-supply = <&ab3100_ldo_g_reg>; diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index b931226..c321bb6 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1395,9 +1395,9 @@ static void mmci_dt_populate_generic_pdata(struct device_node *np, if (!pdata->f_max) pr_warn("%s has no 'max-frequency' property\n", np->full_name); - if (of_get_property(np, "mmc-cap-mmc-highspeed", NULL)) + if (of_get_property(np, "cap-mmc-highspeed", NULL)) pdata->capabilities |= MMC_CAP_MMC_HIGHSPEED; - if (of_get_property(np, "mmc-cap-sd-highspeed", NULL)) + if (of_get_property(np, "cap-sd-highspeed", NULL)) pdata->capabilities |= MMC_CAP_SD_HIGHSPEED; of_property_read_u32(np, "bus-width", &bus_width);
As the first step in preparing the mmci driver for converting to the mmc_of_parse API, let's align to the common names of DT bindings for the mmc/sd highspeed modes. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- Documentation/devicetree/bindings/mmc/mmci.txt | 4 ---- arch/arm/boot/dts/ste-ccu9540.dts | 6 +++--- arch/arm/boot/dts/ste-href.dtsi | 8 ++++---- arch/arm/boot/dts/ste-nomadik-stn8815.dtsi | 4 ++-- arch/arm/boot/dts/ste-snowball.dts | 4 ++-- arch/arm/boot/dts/ste-u300.dts | 4 ++-- drivers/mmc/host/mmci.c | 4 ++-- 7 files changed, 15 insertions(+), 19 deletions(-)