diff mbox

[v3,3/5] dt-bindings: sdhci-omap: Add bindings for the sdhci-omap controller

Message ID 20170823054246.23927-1-kishon@ti.com
State Changes Requested, archived
Headers show

Commit Message

Kishon Vijay Abraham I Aug. 23, 2017, 5:42 a.m. UTC
Add binding for the TI's sdhci-omap controller. This now includes only
a subset of properties documented in ti-omap-hsmmc.txt but will eventually
include all the properties.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
Changes from v2:
*) Fixed example to use the updated compatible

Changes from v1:
*) Create a new sdhci-omap.txt document for TI's sdhci-omap controller instead
   of using the ti-omap-hsmmc.txt as suggested by Tony
 .../devicetree/bindings/mmc/sdhci-omap.txt         | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-omap.txt

Comments

Ulf Hansson Aug. 23, 2017, 1:07 p.m. UTC | #1
On 23 August 2017 at 07:42, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Add binding for the TI's sdhci-omap controller. This now includes only
> a subset of properties documented in ti-omap-hsmmc.txt but will eventually
> include all the properties.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> Changes from v2:
> *) Fixed example to use the updated compatible
>
> Changes from v1:
> *) Create a new sdhci-omap.txt document for TI's sdhci-omap controller instead
>    of using the ti-omap-hsmmc.txt as suggested by Tony
>  .../devicetree/bindings/mmc/sdhci-omap.txt         | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
> new file mode 100644
> index 000000000000..139695ad2d58
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
> @@ -0,0 +1,22 @@
> +* TI OMAP SDHCI Controller
> +
> +Refer to mmc.txt for standard MMC bindings.
> +
> +Required properties:
> +- compatible: Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
> +- ti,hwmods: Must be "mmc<n>", <n> is controller instance starting 1
> +
> +Optional properties:
> +- ti,dual-volt: boolean, supports dual voltage cards
> +- ti,non-removable: non-removable slot (like eMMC)
> +
> +Example:
> +       mmc1: mmc@0x4809c000 {
> +               compatible = "ti,dra7-sdhci";
> +               reg = <0x4809c000 0x400>;
> +               ti,hwmods = "mmc1";
> +               ti,dual-volt;
> +               bus-width = <4>;
> +               vmmc-supply = <&vmmc>; /* phandle to regulator node */
> +               ti,non-removable;
> +       };
> --
> 2.11.0
>

I am wondering a bit on the long term plan here.

Ideally at some point in future, we would like to remove the old
omap_hsmmc driver, but from compatible string point of view, that
means we first needs to deprecate the old ones for a while. Right?

That said, what is then the reason to why we should bring over the
existing omap_hsmmc bindings to the sdhci-omap bindings?

For example, "ti,dual-volt" can likely be replaced with something
better that already exists (either a common mmc binding or an sdhci
binding). For "ti,non-removable", we already have a common mmc binding
"non-removable" for this.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kishon Vijay Abraham I Aug. 23, 2017, 1:56 p.m. UTC | #2
Hi Uffe,

On Wednesday 23 August 2017 06:37 PM, Ulf Hansson wrote:
> On 23 August 2017 at 07:42, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> Add binding for the TI's sdhci-omap controller. This now includes only
>> a subset of properties documented in ti-omap-hsmmc.txt but will eventually
>> include all the properties.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> Changes from v2:
>> *) Fixed example to use the updated compatible
>>
>> Changes from v1:
>> *) Create a new sdhci-omap.txt document for TI's sdhci-omap controller instead
>>    of using the ti-omap-hsmmc.txt as suggested by Tony
>>  .../devicetree/bindings/mmc/sdhci-omap.txt         | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>> new file mode 100644
>> index 000000000000..139695ad2d58
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>> @@ -0,0 +1,22 @@
>> +* TI OMAP SDHCI Controller
>> +
>> +Refer to mmc.txt for standard MMC bindings.
>> +
>> +Required properties:
>> +- compatible: Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
>> +- ti,hwmods: Must be "mmc<n>", <n> is controller instance starting 1
>> +
>> +Optional properties:
>> +- ti,dual-volt: boolean, supports dual voltage cards
>> +- ti,non-removable: non-removable slot (like eMMC)
>> +
>> +Example:
>> +       mmc1: mmc@0x4809c000 {
>> +               compatible = "ti,dra7-sdhci";
>> +               reg = <0x4809c000 0x400>;
>> +               ti,hwmods = "mmc1";
>> +               ti,dual-volt;
>> +               bus-width = <4>;
>> +               vmmc-supply = <&vmmc>; /* phandle to regulator node */
>> +               ti,non-removable;
>> +       };
>> --
>> 2.11.0
>>
> 
> I am wondering a bit on the long term plan here.
> 
> Ideally at some point in future, we would like to remove the old
> omap_hsmmc driver, but from compatible string point of view, that
> means we first needs to deprecate the old ones for a while. Right?

right but sdhci-omap is still lacking features that was present in omap_hsmmc
like context save/restore, SDIO support etc. I think we should deprecate
omap_hsmmc compatible once we add all the features in sdhci-omap?
> 
> That said, what is then the reason to why we should bring over the
> existing omap_hsmmc bindings to the sdhci-omap bindings?

This is mainly for old dt compatibility. Even after removing the omap_hsmmc
driver, users should still be able to use newer kernel with their existing dtbs.

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Aug. 24, 2017, 11:29 a.m. UTC | #3
On 23 August 2017 at 15:56, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Hi Uffe,
>
> On Wednesday 23 August 2017 06:37 PM, Ulf Hansson wrote:
>> On 23 August 2017 at 07:42, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>> Add binding for the TI's sdhci-omap controller. This now includes only
>>> a subset of properties documented in ti-omap-hsmmc.txt but will eventually
>>> include all the properties.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>> ---
>>> Changes from v2:
>>> *) Fixed example to use the updated compatible
>>>
>>> Changes from v1:
>>> *) Create a new sdhci-omap.txt document for TI's sdhci-omap controller instead
>>>    of using the ti-omap-hsmmc.txt as suggested by Tony
>>>  .../devicetree/bindings/mmc/sdhci-omap.txt         | 22 ++++++++++++++++++++++
>>>  1 file changed, 22 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>> new file mode 100644
>>> index 000000000000..139695ad2d58
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>> @@ -0,0 +1,22 @@
>>> +* TI OMAP SDHCI Controller
>>> +
>>> +Refer to mmc.txt for standard MMC bindings.
>>> +
>>> +Required properties:
>>> +- compatible: Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
>>> +- ti,hwmods: Must be "mmc<n>", <n> is controller instance starting 1
>>> +
>>> +Optional properties:
>>> +- ti,dual-volt: boolean, supports dual voltage cards
>>> +- ti,non-removable: non-removable slot (like eMMC)
>>> +
>>> +Example:
>>> +       mmc1: mmc@0x4809c000 {
>>> +               compatible = "ti,dra7-sdhci";
>>> +               reg = <0x4809c000 0x400>;
>>> +               ti,hwmods = "mmc1";
>>> +               ti,dual-volt;
>>> +               bus-width = <4>;
>>> +               vmmc-supply = <&vmmc>; /* phandle to regulator node */
>>> +               ti,non-removable;
>>> +       };
>>> --
>>> 2.11.0
>>>
>>
>> I am wondering a bit on the long term plan here.
>>
>> Ideally at some point in future, we would like to remove the old
>> omap_hsmmc driver, but from compatible string point of view, that
>> means we first needs to deprecate the old ones for a while. Right?
>
> right but sdhci-omap is still lacking features that was present in omap_hsmmc
> like context save/restore, SDIO support etc. I think we should deprecate
> omap_hsmmc compatible once we add all the features in sdhci-omap?
>>
>> That said, what is then the reason to why we should bring over the
>> existing omap_hsmmc bindings to the sdhci-omap bindings?
>
> This is mainly for old dt compatibility. Even after removing the omap_hsmmc
> driver, users should still be able to use newer kernel with their existing dtbs.

I guess we have two options.

1) Allow us to invent and use new bindings - and a new compatible.
When everything is implemented in sdhci-omap, we can deprecate the old
omap_hsmmc driver and its corresponding compatible/bindings. At some
point later we can remove the legacy driver/bindings altogether - of
course that might take a while. This option allows us to re-think some
of the old bindings and really clean up some if its related code. For
example, I think "ti,dual-volt" is a bad binding. Instead it would be
better to use the existing mmc bindings about which speed mode the
controller/board supports (as the voltage level comes with it).

2) Invent only a new compatible, but stick to use the old omap hsmmc
bindings and thus also deploy the similar code dealing with them. When
everything is implemented move the old omap_hsmmc compatibles into the
new sdhci-omap driver and them remove the old omap_hsmmc driver. At
that point we could also deprecate the old omap hsmmc compatibles, but
to me that is rather pointless.

The two options has different advantages, feel free to pick any of them!

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kishon Vijay Abraham I Aug. 29, 2017, 11:20 a.m. UTC | #4
Hi Uffe,

On Thursday 24 August 2017 04:59 PM, Ulf Hansson wrote:
> On 23 August 2017 at 15:56, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> Hi Uffe,
>>
>> On Wednesday 23 August 2017 06:37 PM, Ulf Hansson wrote:
>>> On 23 August 2017 at 07:42, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>>> Add binding for the TI's sdhci-omap controller. This now includes only
>>>> a subset of properties documented in ti-omap-hsmmc.txt but will eventually
>>>> include all the properties.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> ---
>>>> Changes from v2:
>>>> *) Fixed example to use the updated compatible
>>>>
>>>> Changes from v1:
>>>> *) Create a new sdhci-omap.txt document for TI's sdhci-omap controller instead
>>>>    of using the ti-omap-hsmmc.txt as suggested by Tony
>>>>  .../devicetree/bindings/mmc/sdhci-omap.txt         | 22 ++++++++++++++++++++++
>>>>  1 file changed, 22 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>>> new file mode 100644
>>>> index 000000000000..139695ad2d58
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>>> @@ -0,0 +1,22 @@
>>>> +* TI OMAP SDHCI Controller
>>>> +
>>>> +Refer to mmc.txt for standard MMC bindings.
>>>> +
>>>> +Required properties:
>>>> +- compatible: Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
>>>> +- ti,hwmods: Must be "mmc<n>", <n> is controller instance starting 1
>>>> +
>>>> +Optional properties:
>>>> +- ti,dual-volt: boolean, supports dual voltage cards
>>>> +- ti,non-removable: non-removable slot (like eMMC)
>>>> +
>>>> +Example:
>>>> +       mmc1: mmc@0x4809c000 {
>>>> +               compatible = "ti,dra7-sdhci";
>>>> +               reg = <0x4809c000 0x400>;
>>>> +               ti,hwmods = "mmc1";
>>>> +               ti,dual-volt;
>>>> +               bus-width = <4>;
>>>> +               vmmc-supply = <&vmmc>; /* phandle to regulator node */
>>>> +               ti,non-removable;
>>>> +       };
>>>> --
>>>> 2.11.0
>>>>
>>>
>>> I am wondering a bit on the long term plan here.
>>>
>>> Ideally at some point in future, we would like to remove the old
>>> omap_hsmmc driver, but from compatible string point of view, that
>>> means we first needs to deprecate the old ones for a while. Right?
>>
>> right but sdhci-omap is still lacking features that was present in omap_hsmmc
>> like context save/restore, SDIO support etc. I think we should deprecate
>> omap_hsmmc compatible once we add all the features in sdhci-omap?
>>>
>>> That said, what is then the reason to why we should bring over the
>>> existing omap_hsmmc bindings to the sdhci-omap bindings?
>>
>> This is mainly for old dt compatibility. Even after removing the omap_hsmmc
>> driver, users should still be able to use newer kernel with their existing dtbs.
> 
> I guess we have two options.
> 
> 1) Allow us to invent and use new bindings - and a new compatible.
> When everything is implemented in sdhci-omap, we can deprecate the old
> omap_hsmmc driver and its corresponding compatible/bindings. At some
> point later we can remove the legacy driver/bindings altogether - of
> course that might take a while. This option allows us to re-think some
> of the old bindings and really clean up some if its related code. For
> example, I think "ti,dual-volt" is a bad binding. Instead it would be
> better to use the existing mmc bindings about which speed mode the
> controller/board supports (as the voltage level comes with it).
> 
> 2) Invent only a new compatible, but stick to use the old omap hsmmc
> bindings and thus also deploy the similar code dealing with them. When
> everything is implemented move the old omap_hsmmc compatibles into the
> new sdhci-omap driver and them remove the old omap_hsmmc driver. At
> that point we could also deprecate the old omap hsmmc compatibles, but
> to me that is rather pointless.
> 
> The two options has different advantages, feel free to pick any of them!

Okay. I'll send a new version with option '1' (new compatible/new bindings).

And when we deprecate the omap_hsmmc driver (some time later), we'll add
support for the legacy bindings in sdhci-omap driver (so that old dtbs continue
to work). Tony, are you okay with this?

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sebastian Reichel Aug. 29, 2017, 11:50 a.m. UTC | #5
Hi,

On Tue, Aug 29, 2017 at 04:50:22PM +0530, Kishon Vijay Abraham I wrote:
> Hi Uffe,
> 
> On Thursday 24 August 2017 04:59 PM, Ulf Hansson wrote:
> > On 23 August 2017 at 15:56, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> >> Hi Uffe,
> >>
> >> On Wednesday 23 August 2017 06:37 PM, Ulf Hansson wrote:
> >>> On 23 August 2017 at 07:42, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> >>>> Add binding for the TI's sdhci-omap controller. This now includes only
> >>>> a subset of properties documented in ti-omap-hsmmc.txt but will eventually
> >>>> include all the properties.
> >>>>
> >>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >>>> ---
> >>>> Changes from v2:
> >>>> *) Fixed example to use the updated compatible
> >>>>
> >>>> Changes from v1:
> >>>> *) Create a new sdhci-omap.txt document for TI's sdhci-omap controller instead
> >>>>    of using the ti-omap-hsmmc.txt as suggested by Tony
> >>>>  .../devicetree/bindings/mmc/sdhci-omap.txt         | 22 ++++++++++++++++++++++
> >>>>  1 file changed, 22 insertions(+)
> >>>>  create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-omap.txt
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
> >>>> new file mode 100644
> >>>> index 000000000000..139695ad2d58
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
> >>>> @@ -0,0 +1,22 @@
> >>>> +* TI OMAP SDHCI Controller
> >>>> +
> >>>> +Refer to mmc.txt for standard MMC bindings.
> >>>> +
> >>>> +Required properties:
> >>>> +- compatible: Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
> >>>> +- ti,hwmods: Must be "mmc<n>", <n> is controller instance starting 1
> >>>> +
> >>>> +Optional properties:
> >>>> +- ti,dual-volt: boolean, supports dual voltage cards
> >>>> +- ti,non-removable: non-removable slot (like eMMC)
> >>>> +
> >>>> +Example:
> >>>> +       mmc1: mmc@0x4809c000 {
> >>>> +               compatible = "ti,dra7-sdhci";
> >>>> +               reg = <0x4809c000 0x400>;
> >>>> +               ti,hwmods = "mmc1";
> >>>> +               ti,dual-volt;
> >>>> +               bus-width = <4>;
> >>>> +               vmmc-supply = <&vmmc>; /* phandle to regulator node */
> >>>> +               ti,non-removable;
> >>>> +       };
> >>>> --
> >>>> 2.11.0
> >>>>
> >>>
> >>> I am wondering a bit on the long term plan here.
> >>>
> >>> Ideally at some point in future, we would like to remove the old
> >>> omap_hsmmc driver, but from compatible string point of view, that
> >>> means we first needs to deprecate the old ones for a while. Right?
> >>
> >> right but sdhci-omap is still lacking features that was present in omap_hsmmc
> >> like context save/restore, SDIO support etc. I think we should deprecate
> >> omap_hsmmc compatible once we add all the features in sdhci-omap?
> >>>
> >>> That said, what is then the reason to why we should bring over the
> >>> existing omap_hsmmc bindings to the sdhci-omap bindings?
> >>
> >> This is mainly for old dt compatibility. Even after removing the omap_hsmmc
> >> driver, users should still be able to use newer kernel with their existing dtbs.
> > 
> > I guess we have two options.
> > 
> > 1) Allow us to invent and use new bindings - and a new compatible.
> > When everything is implemented in sdhci-omap, we can deprecate the old
> > omap_hsmmc driver and its corresponding compatible/bindings. At some
> > point later we can remove the legacy driver/bindings altogether - of
> > course that might take a while. This option allows us to re-think some
> > of the old bindings and really clean up some if its related code. For
> > example, I think "ti,dual-volt" is a bad binding. Instead it would be
> > better to use the existing mmc bindings about which speed mode the
> > controller/board supports (as the voltage level comes with it).
> > 
> > 2) Invent only a new compatible, but stick to use the old omap hsmmc
> > bindings and thus also deploy the similar code dealing with them. When
> > everything is implemented move the old omap_hsmmc compatibles into the
> > new sdhci-omap driver and them remove the old omap_hsmmc driver. At
> > that point we could also deprecate the old omap hsmmc compatibles, but
> > to me that is rather pointless.
> > 
> > The two options has different advantages, feel free to pick any of them!
> 
> Okay. I'll send a new version with option '1' (new compatible/new bindings).
> 
> And when we deprecate the omap_hsmmc driver (some time later), we'll add
> support for the legacy bindings in sdhci-omap driver (so that old dtbs continue
> to work). Tony, are you okay with this?

I think you should Cc Rob Herring and Mark Rutland (DT binding
maintainers). This sounds like "I use DT to describe driver
behaviour" instead of "I use DT to describe hardware".

I would expect the conversion to look like the one done for UART,
see CONFIG_SERIAL_OMAP vs CONFIG_SERIAL_8250_OMAP. Both use the
same compatible value and you can choose using kernel configuration.

-- Sebastian
Tony Lindgren Aug. 29, 2017, 1:58 p.m. UTC | #6
* Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170829 04:51]:
> On Tue, Aug 29, 2017 at 04:50:22PM +0530, Kishon Vijay Abraham I wrote:
> > On Thursday 24 August 2017 04:59 PM, Ulf Hansson wrote:
> > > I guess we have two options.
> > > 
> > > 1) Allow us to invent and use new bindings - and a new compatible.
> > > When everything is implemented in sdhci-omap, we can deprecate the old
> > > omap_hsmmc driver and its corresponding compatible/bindings. At some
> > > point later we can remove the legacy driver/bindings altogether - of
> > > course that might take a while. This option allows us to re-think some
> > > of the old bindings and really clean up some if its related code. For
> > > example, I think "ti,dual-volt" is a bad binding. Instead it would be
> > > better to use the existing mmc bindings about which speed mode the
> > > controller/board supports (as the voltage level comes with it).
> > > 
> > > 2) Invent only a new compatible, but stick to use the old omap hsmmc
> > > bindings and thus also deploy the similar code dealing with them. When
> > > everything is implemented move the old omap_hsmmc compatibles into the
> > > new sdhci-omap driver and them remove the old omap_hsmmc driver. At
> > > that point we could also deprecate the old omap hsmmc compatibles, but
> > > to me that is rather pointless.
> > > 
> > > The two options has different advantages, feel free to pick any of them!
> > 
> > Okay. I'll send a new version with option '1' (new compatible/new bindings).

Agreed.

> > And when we deprecate the omap_hsmmc driver (some time later), we'll add
> > support for the legacy bindings in sdhci-omap driver (so that old dtbs continue
> > to work). Tony, are you okay with this?
> 
> I think you should Cc Rob Herring and Mark Rutland (DT binding
> maintainers). This sounds like "I use DT to describe driver
> behaviour" instead of "I use DT to describe hardware".

Yes please.

> I would expect the conversion to look like the one done for UART,
> see CONFIG_SERIAL_OMAP vs CONFIG_SERIAL_8250_OMAP. Both use the
> same compatible value and you can choose using kernel configuration.

That does not work unfortunately :( We are now stuck in a situation
where two drivers are attempting to probe with the same compatible
and we can't enable 8250_OMAP because of the user space breakage
with the device names. And I'm actuallly thinking we should add a
new compatible for 8250-omap to be able to start enabling it one
board at a time.

It's best to enable devices to use the new compatible as things are
tested rather than hope for some magic "flag day" flip that might
never happen. Having two days attempting to probe with the same
binding just won't work.

So yeah, I agree with Kishon that we should stick with generic
and sdhci bindings. And then we can start already using it for
boards that can use it, then eventually when we're ready, start
parsing also the legacy bindings and maybe drop the old driver.

Regards,

Tony

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Aug. 29, 2017, 2:43 p.m. UTC | #7
* Tony Lindgren <tony@atomide.com> [170829 06:58]:
> It's best to enable devices to use the new compatible as things are
> tested rather than hope for some magic "flag day" flip that might
> never happen. Having two days attempting to probe with the same
> binding just won't work.

Hehe I mean "Having two drivers attempting to probe with the same
binding" above.

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Aug. 29, 2017, 5:09 p.m. UTC | #8
On Tue, Aug 29, 2017 at 06:58:23AM -0700, Tony Lindgren wrote:
> * Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170829 04:51]:
> > On Tue, Aug 29, 2017 at 04:50:22PM +0530, Kishon Vijay Abraham I wrote:
> > > On Thursday 24 August 2017 04:59 PM, Ulf Hansson wrote:
> > > > I guess we have two options.
> > > > 
> > > > 1) Allow us to invent and use new bindings - and a new compatible.
> > > > When everything is implemented in sdhci-omap, we can deprecate the old
> > > > omap_hsmmc driver and its corresponding compatible/bindings. At some
> > > > point later we can remove the legacy driver/bindings altogether - of
> > > > course that might take a while. This option allows us to re-think some
> > > > of the old bindings and really clean up some if its related code. For
> > > > example, I think "ti,dual-volt" is a bad binding. Instead it would be
> > > > better to use the existing mmc bindings about which speed mode the
> > > > controller/board supports (as the voltage level comes with it).
> > > > 
> > > > 2) Invent only a new compatible, but stick to use the old omap hsmmc
> > > > bindings and thus also deploy the similar code dealing with them. When
> > > > everything is implemented move the old omap_hsmmc compatibles into the
> > > > new sdhci-omap driver and them remove the old omap_hsmmc driver. At
> > > > that point we could also deprecate the old omap hsmmc compatibles, but
> > > > to me that is rather pointless.
> > > > 
> > > > The two options has different advantages, feel free to pick any of them!
> > > 
> > > Okay. I'll send a new version with option '1' (new compatible/new bindings).
> 
> Agreed.
> 
> > > And when we deprecate the omap_hsmmc driver (some time later), we'll add
> > > support for the legacy bindings in sdhci-omap driver (so that old dtbs continue
> > > to work). Tony, are you okay with this?
> > 
> > I think you should Cc Rob Herring and Mark Rutland (DT binding
> > maintainers). This sounds like "I use DT to describe driver
> > behaviour" instead of "I use DT to describe hardware".

Indeed...

> 
> Yes please.
> 
> > I would expect the conversion to look like the one done for UART,
> > see CONFIG_SERIAL_OMAP vs CONFIG_SERIAL_8250_OMAP. Both use the
> > same compatible value and you can choose using kernel configuration.
> 
> That does not work unfortunately :( We are now stuck in a situation
> where two drivers are attempting to probe with the same compatible
> and we can't enable 8250_OMAP because of the user space breakage
> with the device names. And I'm actuallly thinking we should add a
> new compatible for 8250-omap to be able to start enabling it one
> board at a time.

Is that the only problem? Presumably, the SD driver doesn't have a 
userspace facing issue.

> It's best to enable devices to use the new compatible as things are
> tested rather than hope for some magic "flag day" flip that might
> never happen. Having two days attempting to probe with the same
> binding just won't work.

Aren't you just picking whether the flag day is in DT or the kernel? I 
guess you're assuming one kernel build and it would be switching all 
boards at one. 

> So yeah, I agree with Kishon that we should stick with generic
> and sdhci bindings. And then we can start already using it for
> boards that can use it, then eventually when we're ready, start
> parsing also the legacy bindings and maybe drop the old driver.

I assume there are some other common properties you would switch to in 
the transition?  You could make the legacy driver bail from probe based 
on presence or absence of other properties. Or you could just blacklist 
converted platforms in the legacy driver. The point is that the problems 
are solvable in the kernel.

But if your really want a new compatible, I don't really care. It's 
only one device.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Aug. 29, 2017, 5:11 p.m. UTC | #9
On Wed, Aug 23, 2017 at 11:12:46AM +0530, Kishon Vijay Abraham I wrote:
> Add binding for the TI's sdhci-omap controller. This now includes only
> a subset of properties documented in ti-omap-hsmmc.txt but will eventually
> include all the properties.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> Changes from v2:
> *) Fixed example to use the updated compatible
> 
> Changes from v1:
> *) Create a new sdhci-omap.txt document for TI's sdhci-omap controller instead
>    of using the ti-omap-hsmmc.txt as suggested by Tony
>  .../devicetree/bindings/mmc/sdhci-omap.txt         | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-omap.txt
> 
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
> new file mode 100644
> index 000000000000..139695ad2d58
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
> @@ -0,0 +1,22 @@
> +* TI OMAP SDHCI Controller
> +
> +Refer to mmc.txt for standard MMC bindings.
> +
> +Required properties:
> +- compatible: Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
> +- ti,hwmods: Must be "mmc<n>", <n> is controller instance starting 1
> +
> +Optional properties:
> +- ti,dual-volt: boolean, supports dual voltage cards
> +- ti,non-removable: non-removable slot (like eMMC)

Well, if you are doing a new compatible, then why aren't you using 
common properties?

> +
> +Example:
> +	mmc1: mmc@0x4809c000 {

Drop the '0x'.

> +		compatible = "ti,dra7-sdhci";
> +		reg = <0x4809c000 0x400>;
> +		ti,hwmods = "mmc1";
> +		ti,dual-volt;
> +		bus-width = <4>;
> +		vmmc-supply = <&vmmc>; /* phandle to regulator node */
> +		ti,non-removable;
> +	};
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Aug. 29, 2017, 5:39 p.m. UTC | #10
* Rob Herring <robh@kernel.org> [170829 10:09]:
> On Tue, Aug 29, 2017 at 06:58:23AM -0700, Tony Lindgren wrote:
> > * Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170829 04:51]:
> > > I would expect the conversion to look like the one done for UART,
> > > see CONFIG_SERIAL_OMAP vs CONFIG_SERIAL_8250_OMAP. Both use the
> > > same compatible value and you can choose using kernel configuration.
> > 
> > That does not work unfortunately :( We are now stuck in a situation
> > where two drivers are attempting to probe with the same compatible
> > and we can't enable 8250_OMAP because of the user space breakage
> > with the device names. And I'm actuallly thinking we should add a
> > new compatible for 8250-omap to be able to start enabling it one
> > board at a time.
> 
> Is that the only problem? Presumably, the SD driver doesn't have a 
> userspace facing issue.

No userspace issue with the sdhci-omap. But the sdhci-omap driver
still has the issue of trying enable it for everything at once and
expect everything to work. The sdhci-omap driver can already be
used for boards that don't need power management for example, but
will break things for devices running on batteries.

> > It's best to enable devices to use the new compatible as things are
> > tested rather than hope for some magic "flag day" flip that might
> > never happen. Having two days attempting to probe with the same
> > binding just won't work.
> 
> Aren't you just picking whether the flag day is in DT or the kernel? I 
> guess you're assuming one kernel build and it would be switching all 
> boards at one. 

Right, this would be risky and would take unnecessarily long
to use the new driver on boards that can already use it. While
power management won't work yet, I'd expect the sdhci-omap be
faster on SoCs that can do ADMA.

> > So yeah, I agree with Kishon that we should stick with generic
> > and sdhci bindings. And then we can start already using it for
> > boards that can use it, then eventually when we're ready, start
> > parsing also the legacy bindings and maybe drop the old driver.
> 
> I assume there are some other common properties you would switch to in 
> the transition?  You could make the legacy driver bail from probe based 
> on presence or absence of other properties. Or you could just blacklist 
> converted platforms in the legacy driver. The point is that the problems 
> are solvable in the kernel.

Yes this could be done too. But let's enable it on per-board
basis rather than attempt to flip it on at once.

> But if your really want a new compatible, I don't really care. It's 
> only one device.

Both a new compatible or a check for some resource work just fine
for me as long as the driver can be selected on per-board basis.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kishon Vijay Abraham I Sept. 5, 2017, 8:52 a.m. UTC | #11
Hi,

On Tuesday 29 August 2017 11:09 PM, Tony Lindgren wrote:
> * Rob Herring <robh@kernel.org> [170829 10:09]:
>> On Tue, Aug 29, 2017 at 06:58:23AM -0700, Tony Lindgren wrote:
>>> * Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170829 04:51]:
>>>> I would expect the conversion to look like the one done for UART,
>>>> see CONFIG_SERIAL_OMAP vs CONFIG_SERIAL_8250_OMAP. Both use the
>>>> same compatible value and you can choose using kernel configuration.
>>>
>>> That does not work unfortunately :( We are now stuck in a situation
>>> where two drivers are attempting to probe with the same compatible
>>> and we can't enable 8250_OMAP because of the user space breakage
>>> with the device names. And I'm actuallly thinking we should add a
>>> new compatible for 8250-omap to be able to start enabling it one
>>> board at a time.
>>
>> Is that the only problem? Presumably, the SD driver doesn't have a 
>> userspace facing issue.
> 
> No userspace issue with the sdhci-omap. But the sdhci-omap driver
> still has the issue of trying enable it for everything at once and
> expect everything to work. The sdhci-omap driver can already be
> used for boards that don't need power management for example, but
> will break things for devices running on batteries.
> 
>>> It's best to enable devices to use the new compatible as things are
>>> tested rather than hope for some magic "flag day" flip that might
>>> never happen. Having two days attempting to probe with the same
>>> binding just won't work.
>>
>> Aren't you just picking whether the flag day is in DT or the kernel? I 
>> guess you're assuming one kernel build and it would be switching all 
>> boards at one. 
> 
> Right, this would be risky and would take unnecessarily long
> to use the new driver on boards that can already use it. While
> power management won't work yet, I'd expect the sdhci-omap be
> faster on SoCs that can do ADMA.
> 
>>> So yeah, I agree with Kishon that we should stick with generic
>>> and sdhci bindings. And then we can start already using it for
>>> boards that can use it, then eventually when we're ready, start
>>> parsing also the legacy bindings and maybe drop the old driver.
>>
>> I assume there are some other common properties you would switch to in 
>> the transition?  You could make the legacy driver bail from probe based 
>> on presence or absence of other properties. Or you could just blacklist 
>> converted platforms in the legacy driver. The point is that the problems 
>> are solvable in the kernel.
> 
> Yes this could be done too. But let's enable it on per-board
> basis rather than attempt to flip it on at once.
> 
>> But if your really want a new compatible, I don't really care. It's 
>> only one device.
> 
> Both a new compatible or a check for some resource work just fine
> for me as long as the driver can be selected on per-board basis.

New compatible sounds simpler to me since it allows to select a driver per-MMC
instance. (SDIO support is not added/validated in sdhci-omap).

To summarize, we'll create new compatible and new bindings for sdhci-omap and
start enabling sdhci-omap on per-board basis. When all the TI platforms starts
to use sdhci-omap, omap-hsmmc will be removed at which point support for old
compatible and old dt bindings will be added to sdhci-omap.c. Does this sound
reasonable?

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kishon Vijay Abraham I Sept. 5, 2017, 8:53 a.m. UTC | #12
Hi,

On Tuesday 29 August 2017 10:41 PM, Rob Herring wrote:
> On Wed, Aug 23, 2017 at 11:12:46AM +0530, Kishon Vijay Abraham I wrote:
>> Add binding for the TI's sdhci-omap controller. This now includes only
>> a subset of properties documented in ti-omap-hsmmc.txt but will eventually
>> include all the properties.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> Changes from v2:
>> *) Fixed example to use the updated compatible
>>
>> Changes from v1:
>> *) Create a new sdhci-omap.txt document for TI's sdhci-omap controller instead
>>    of using the ti-omap-hsmmc.txt as suggested by Tony
>>  .../devicetree/bindings/mmc/sdhci-omap.txt         | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>> new file mode 100644
>> index 000000000000..139695ad2d58
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>> @@ -0,0 +1,22 @@
>> +* TI OMAP SDHCI Controller
>> +
>> +Refer to mmc.txt for standard MMC bindings.
>> +
>> +Required properties:
>> +- compatible: Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
>> +- ti,hwmods: Must be "mmc<n>", <n> is controller instance starting 1
>> +
>> +Optional properties:
>> +- ti,dual-volt: boolean, supports dual voltage cards
>> +- ti,non-removable: non-removable slot (like eMMC)
> 
> Well, if you are doing a new compatible, then why aren't you using 
> common properties?

yeah, now moved to generic bindings.
> 
>> +
>> +Example:
>> +	mmc1: mmc@0x4809c000 {
> 
> Drop the '0x'.

sure, will fix it.

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Sept. 5, 2017, 4:51 p.m. UTC | #13
* Kishon Vijay Abraham I <kishon@ti.com> [170905 01:53]:
> Hi,
> 
> On Tuesday 29 August 2017 11:09 PM, Tony Lindgren wrote:
> > * Rob Herring <robh@kernel.org> [170829 10:09]:
> >> On Tue, Aug 29, 2017 at 06:58:23AM -0700, Tony Lindgren wrote:
> >>> * Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170829 04:51]:
> >>>> I would expect the conversion to look like the one done for UART,
> >>>> see CONFIG_SERIAL_OMAP vs CONFIG_SERIAL_8250_OMAP. Both use the
> >>>> same compatible value and you can choose using kernel configuration.
> >>>
> >>> That does not work unfortunately :( We are now stuck in a situation
> >>> where two drivers are attempting to probe with the same compatible
> >>> and we can't enable 8250_OMAP because of the user space breakage
> >>> with the device names. And I'm actuallly thinking we should add a
> >>> new compatible for 8250-omap to be able to start enabling it one
> >>> board at a time.
> >>
> >> Is that the only problem? Presumably, the SD driver doesn't have a 
> >> userspace facing issue.
> > 
> > No userspace issue with the sdhci-omap. But the sdhci-omap driver
> > still has the issue of trying enable it for everything at once and
> > expect everything to work. The sdhci-omap driver can already be
> > used for boards that don't need power management for example, but
> > will break things for devices running on batteries.
> > 
> >>> It's best to enable devices to use the new compatible as things are
> >>> tested rather than hope for some magic "flag day" flip that might
> >>> never happen. Having two days attempting to probe with the same
> >>> binding just won't work.
> >>
> >> Aren't you just picking whether the flag day is in DT or the kernel? I 
> >> guess you're assuming one kernel build and it would be switching all 
> >> boards at one. 
> > 
> > Right, this would be risky and would take unnecessarily long
> > to use the new driver on boards that can already use it. While
> > power management won't work yet, I'd expect the sdhci-omap be
> > faster on SoCs that can do ADMA.
> > 
> >>> So yeah, I agree with Kishon that we should stick with generic
> >>> and sdhci bindings. And then we can start already using it for
> >>> boards that can use it, then eventually when we're ready, start
> >>> parsing also the legacy bindings and maybe drop the old driver.
> >>
> >> I assume there are some other common properties you would switch to in 
> >> the transition?  You could make the legacy driver bail from probe based 
> >> on presence or absence of other properties. Or you could just blacklist 
> >> converted platforms in the legacy driver. The point is that the problems 
> >> are solvable in the kernel.
> > 
> > Yes this could be done too. But let's enable it on per-board
> > basis rather than attempt to flip it on at once.
> > 
> >> But if your really want a new compatible, I don't really care. It's 
> >> only one device.
> > 
> > Both a new compatible or a check for some resource work just fine
> > for me as long as the driver can be selected on per-board basis.
> 
> New compatible sounds simpler to me since it allows to select a driver per-MMC
> instance. (SDIO support is not added/validated in sdhci-omap).
> 
> To summarize, we'll create new compatible and new bindings for sdhci-omap and
> start enabling sdhci-omap on per-board basis. When all the TI platforms starts
> to use sdhci-omap, omap-hsmmc will be removed at which point support for old
> compatible and old dt bindings will be added to sdhci-omap.c. Does this sound
> reasonable?

Sounds like a plan to me.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
new file mode 100644
index 000000000000..139695ad2d58
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
@@ -0,0 +1,22 @@ 
+* TI OMAP SDHCI Controller
+
+Refer to mmc.txt for standard MMC bindings.
+
+Required properties:
+- compatible: Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
+- ti,hwmods: Must be "mmc<n>", <n> is controller instance starting 1
+
+Optional properties:
+- ti,dual-volt: boolean, supports dual voltage cards
+- ti,non-removable: non-removable slot (like eMMC)
+
+Example:
+	mmc1: mmc@0x4809c000 {
+		compatible = "ti,dra7-sdhci";
+		reg = <0x4809c000 0x400>;
+		ti,hwmods = "mmc1";
+		ti,dual-volt;
+		bus-width = <4>;
+		vmmc-supply = <&vmmc>; /* phandle to regulator node */
+		ti,non-removable;
+	};