[PATCH v2 0/4] mmc: Add SDIO function devicetree subnode parsing

[PATCH v2 0/4] mmc: Add SDIO function devicetree subnode parsing

[Hans de Goede]

Hi All,

Here is v2 of my submission of Sascha Hauer's
"mmc: Add SDIO function devicetree subnode parsing" patch.

This version has been reworked as a result of the discussion about sdio
powerup sequencing. Specifically the location of the per sdio-function subnodes
has been moved from being a subnode under the mmc host, to being a subnode
under the mmc-slot node (which itself is a subnode under the mmc host).

This not only matches the notion of their needing to be a slot abstraction
between the 2 which came up during the powerup discussion, it actually matches
what we are already doing, as while working on this patch-set I learned that
several dt-bindings already use slot child-nodes.

Note this patch-set is not about powerup sequencing, it is related though,
it is about the need to store extra info on sdio devices in devicetree in
general. The specific itch I'm scratching here is getting oob interrupts to
work for sdio wifi modules (which in my case do not need any special powerup).

Changes since v1:
- Submit as a stand-alone patch-set, rather then as part of a larger oob irq
  support patchset crossing multiple subsystems
- Add a patch to document the already existing practice of using slot subnodes
- Split of the dt bindings documentation for sdio-func child nodes into its
  own patch
- Add a preparation patch, setting slot_no where relevant
- And finally the "Add SDIO function devicetree subnode parsing" patch itself
  has been modified to look for the sdio function childnodes under a slot node
  instead of directly under the host node

Regards,

Hans

[PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Hans de Goede]

The following existing MMC host controller bindings use slot subnodes:

Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt

This commit documents this practice in the standard mmc bindings documentation.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/devicetree/bindings/mmc/mmc.txt | 46 ++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 9dce540..44c9e53 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -60,7 +60,31 @@ Optional SDIO properties:
 - keep-power-in-suspend: Preserves card power during a suspend/resume cycle
 - enable-sdio-wakeup: Enables wake up of host system on SDIO IRQ assertion
 
-Example:
+
+Use of slot subnodes
+--------------------
+
+Some hosts have multiple MMC slots connected to a single MMC host, in this
+case each slot gets its own subnode representing the slot:
+
+Required host node properties when using slots:
+- #address-cells: should be one. The cell is the slot id.
+- #size-cells: should be zero.
+
+Required slot subnode properties:
+- reg: Must contain the MMC host slot number of the slot this subnode
+       describes. Slot numbers start at 0.
+
+Optional slot subnode properties:
+Any of the optional host node properties can be used inside a slot node too,
+if a property is specified at both the host and the slot level the slot
+level takes precedence.
+
+
+Examples
+--------
+
+Basic example:
 
 sdhci at ab000000 {
 	compatible = "sdhci";
@@ -74,3 +98,23 @@ sdhci at ab000000 {
 	keep-power-in-suspend;
 	enable-sdio-wakeup;
 }
+
+Example with slot subnodes:
+
+mmc0: mmc at f0008000 {
+	compatible = "atmel,hsmci";
+	reg = <0xf0008000 0x600>;
+	interrupts = <12 4>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+	slot at 0 {
+		reg = <0>;
+		bus-width = <4>;
+		cd-gpios = <&pioD 15 0>
+		cd-inverted;
+	};
+	slot at 1 {
+		reg = <1>;
+		bus-width = <4>;
+	};
+};
-- 
2.0.0

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Olof Johansson]

On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> The following existing MMC host controller bindings use slot subnodes:
>
> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>
> This commit documents this practice in the standard mmc bindings documentation.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

There are today only two drivers that use this kind of binding, dw_mmc
and the at91 one. Neither seems to actually ever have been used with
more than one slot. I doubt anyone building an exynos-based system
will ever do a multi-slot solution, and it seems that the at91 driver
doesn't actually handle more than one slot.

I'm personally not that excited about complicating the bindings by
opening up for this -- I would rather work towards removing the
concept of slots if it's one of those things that are going to remain
unused. We have actually been talking about reworking the dw_mmc
binding to remove the slot concept (and simplify the driver by doing
so).


-Olof

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Hans de Goede]

Hi,

On 05/31/2014 10:13 PM, Olof Johansson wrote:
> On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> The following existing MMC host controller bindings use slot subnodes:
>>
>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
>> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>>
>> This commit documents this practice in the standard mmc bindings documentation.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> There are today only two drivers that use this kind of binding, dw_mmc
> and the at91 one.

Correct.

> Neither seems to actually ever have been used with
> more than one slot. I doubt anyone building an exynos-based system
> will ever do a multi-slot solution, and it seems that the at91 driver
> doesn't actually handle more than one slot.
>
> I'm personally not that excited about complicating the bindings by
> opening up for this -- I would rather work towards removing the
> concept of slots if it's one of those things that are going to remain
> unused. We have actually been talking about reworking the dw_mmc
> binding to remove the slot concept (and simplify the driver by doing
> so).

I'm fine with removing the slot subnode, I added it because of it being
brought up in the powerup sequence discussion. I explicitly asked there
if adding such a subnode level was seen as desirable but nobody
answered :|

Anyways, either way works for me. I can do a v3 dropping the slot subnode
level again. I would really like to move forward with a decision on how-to
represent non probable info for sdio devices in device nodes. So do you
have any other remarks other then that the slot subnode should be dropped ?
And if not can you please review and ack (*) v3 of this patch-set once
I've send it?

Chris Ball and Ulf Hansson, what is your take on this, are you willing to
take this patch set? And do you want it with or without the slot subnodes ?

Thanks & Regards,

Hans


*) Assuming you don't find any issues

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Sascha Hauer]

On Sun, Jun 01, 2014 at 11:23:48AM +0200, Hans de Goede wrote:
> Hi,
> 
> >Neither seems to actually ever have been used with
> >more than one slot. I doubt anyone building an exynos-based system
> >will ever do a multi-slot solution, and it seems that the at91 driver
> >doesn't actually handle more than one slot.
> >
> >I'm personally not that excited about complicating the bindings by
> >opening up for this -- I would rather work towards removing the
> >concept of slots if it's one of those things that are going to remain
> >unused. We have actually been talking about reworking the dw_mmc
> >binding to remove the slot concept (and simplify the driver by doing
> >so).
> 
> I'm fine with removing the slot subnode, I added it because of it being
> brought up in the powerup sequence discussion. I explicitly asked there
> if adding such a subnode level was seen as desirable but nobody
> answered :|

MMC bus support was removed back in 2007:

| commit b855885e3b60cf6f9452848712a62517b94583eb
| Author: Pierre Ossman <drzeus@drzeus.cx>
| Date:   Wed Jan 3 19:47:29 2007 +0100
| 
|     mmc: deprecate mmc bus topology
|     
|     The classic MMC bus was defined as multi card bus
|     system, which is reflected in the design in the MMC
|     layer.
|     
|     When SD showed up, the bus topology was abandoned
|     and a star topology (one card per host) was mandated.
|     MMC version 4 has followed this, officially deprecating
|     the bus topology.
|     
|     As we do not have any known users of the bus
|     topology we can remove support for it. This will
|     simplify the code and rectify some incorrect
|     assumptions in the newer additions.
|     
|     Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>

I doubt we will ever need support for it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Ulf Hansson]

On 1 June 2014 11:23, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 05/31/2014 10:13 PM, Olof Johansson wrote:
>>
>> On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> The following existing MMC host controller bindings use slot subnodes:
>>>
>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>>> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>>> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
>>> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
>>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>>>
>>> This commit documents this practice in the standard mmc bindings
>>> documentation.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>
>>
>> There are today only two drivers that use this kind of binding, dw_mmc
>> and the at91 one.
>
>
> Correct.
>
>
>> Neither seems to actually ever have been used with
>> more than one slot. I doubt anyone building an exynos-based system
>> will ever do a multi-slot solution, and it seems that the at91 driver
>> doesn't actually handle more than one slot.
>>
>> I'm personally not that excited about complicating the bindings by
>> opening up for this -- I would rather work towards removing the
>> concept of slots if it's one of those things that are going to remain
>> unused. We have actually been talking about reworking the dw_mmc
>> binding to remove the slot concept (and simplify the driver by doing
>> so).
>
>
> I'm fine with removing the slot subnode, I added it because of it being
> brought up in the powerup sequence discussion. I explicitly asked there
> if adding such a subnode level was seen as desirable but nobody
> answered :|
>
> Anyways, either way works for me. I can do a v3 dropping the slot subnode
> level again. I would really like to move forward with a decision on how-to
> represent non probable info for sdio devices in device nodes. So do you
> have any other remarks other then that the slot subnode should be dropped ?
> And if not can you please review and ack (*) v3 of this patch-set once
> I've send it?
>
> Chris Ball and Ulf Hansson, what is your take on this, are you willing to
> take this patch set? And do you want it with or without the slot subnodes ?

I certainly appreciate you working actively on this Hans, I will look
into the patchset as soon as I can.

I share Olof's view about the slot nodes, we must not add DT bindings
that isn't really needed.

Regarding the slot subnodes; Jaehoon Chung recently posted a patchset
for adding the parsing of it, intended for dwmmc. I withdraw my ack
for it, and let's try to go in the other direction instead.

[PATCHv3 0/4] mmc: fixed the mmc_of_parse for dwmmc.

Thus I suggest we should clean-up host drivers to support only one
card per host, and entirely skip the slot concept.

Kind regards
Uffe

>
> Thanks & Regards,
>
> Hans
>
>
> *) Assuming you don't find any issues

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Hans de Goede]

Hi,

On 06/02/2014 10:29 AM, Ulf Hansson wrote:
> On 1 June 2014 11:23, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 05/31/2014 10:13 PM, Olof Johansson wrote:
>>>
>>> On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com>
>>> wrote:
>>>>
>>>> The following existing MMC host controller bindings use slot subnodes:
>>>>
>>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>>> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>>>> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>>>> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
>>>> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
>>>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>>>>
>>>> This commit documents this practice in the standard mmc bindings
>>>> documentation.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>
>>>
>>> There are today only two drivers that use this kind of binding, dw_mmc
>>> and the at91 one.
>>
>>
>> Correct.
>>
>>
>>> Neither seems to actually ever have been used with
>>> more than one slot. I doubt anyone building an exynos-based system
>>> will ever do a multi-slot solution, and it seems that the at91 driver
>>> doesn't actually handle more than one slot.
>>>
>>> I'm personally not that excited about complicating the bindings by
>>> opening up for this -- I would rather work towards removing the
>>> concept of slots if it's one of those things that are going to remain
>>> unused. We have actually been talking about reworking the dw_mmc
>>> binding to remove the slot concept (and simplify the driver by doing
>>> so).
>>
>>
>> I'm fine with removing the slot subnode, I added it because of it being
>> brought up in the powerup sequence discussion. I explicitly asked there
>> if adding such a subnode level was seen as desirable but nobody
>> answered :|
>>
>> Anyways, either way works for me. I can do a v3 dropping the slot subnode
>> level again. I would really like to move forward with a decision on how-to
>> represent non probable info for sdio devices in device nodes. So do you
>> have any other remarks other then that the slot subnode should be dropped ?
>> And if not can you please review and ack (*) v3 of this patch-set once
>> I've send it?
>>
>> Chris Ball and Ulf Hansson, what is your take on this, are you willing to
>> take this patch set? And do you want it with or without the slot subnodes ?
> 
> I certainly appreciate you working actively on this Hans, I will look
> into the patchset as soon as I can.

Thanks. If I read you correctly below, then you want the slot nodes
to be removed, correct? In that case it is probably best if you wait reviewing
until I've done a v3, with the slot nodes removed. I hope to find some time
to do this this evening (CET).

Regards,

Hans


> 
> I share Olof's view about the slot nodes, we must not add DT bindings
> that isn't really needed.
> 
> Regarding the slot subnodes; Jaehoon Chung recently posted a patchset
> for adding the parsing of it, intended for dwmmc. I withdraw my ack
> for it, and let's try to go in the other direction instead.
> 
> [PATCHv3 0/4] mmc: fixed the mmc_of_parse for dwmmc.
> 
> Thus I suggest we should clean-up host drivers to support only one
> card per host, and entirely skip the slot concept.
> 
> Kind regards
> Uffe
> 
>>
>> Thanks & Regards,
>>
>> Hans
>>
>>
>> *) Assuming you don't find any issues

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Jaehoon Chung]

On 06/02/2014 05:29 PM, Ulf Hansson wrote:
> On 1 June 2014 11:23, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 05/31/2014 10:13 PM, Olof Johansson wrote:
>>>
>>> On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com>
>>> wrote:
>>>>
>>>> The following existing MMC host controller bindings use slot subnodes:
>>>>
>>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>>> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>>>> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>>>> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
>>>> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
>>>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>>>>
>>>> This commit documents this practice in the standard mmc bindings
>>>> documentation.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>
>>>
>>> There are today only two drivers that use this kind of binding, dw_mmc
>>> and the at91 one.
>>
>>
>> Correct.
>>
>>
>>> Neither seems to actually ever have been used with
>>> more than one slot. I doubt anyone building an exynos-based system
>>> will ever do a multi-slot solution, and it seems that the at91 driver
>>> doesn't actually handle more than one slot.
>>>
>>> I'm personally not that excited about complicating the bindings by
>>> opening up for this -- I would rather work towards removing the
>>> concept of slots if it's one of those things that are going to remain
>>> unused. We have actually been talking about reworking the dw_mmc
>>> binding to remove the slot concept (and simplify the driver by doing
>>> so).
>>
>>
>> I'm fine with removing the slot subnode, I added it because of it being
>> brought up in the powerup sequence discussion. I explicitly asked there
>> if adding such a subnode level was seen as desirable but nobody
>> answered :|
>>
>> Anyways, either way works for me. I can do a v3 dropping the slot subnode
>> level again. I would really like to move forward with a decision on how-to
>> represent non probable info for sdio devices in device nodes. So do you
>> have any other remarks other then that the slot subnode should be dropped ?
>> And if not can you please review and ack (*) v3 of this patch-set once
>> I've send it?
>>
>> Chris Ball and Ulf Hansson, what is your take on this, are you willing to
>> take this patch set? And do you want it with or without the slot subnodes ?
> 
> I certainly appreciate you working actively on this Hans, I will look
> into the patchset as soon as I can.
> 
> I share Olof's view about the slot nodes, we must not add DT bindings
> that isn't really needed.
> 
> Regarding the slot subnodes; Jaehoon Chung recently posted a patchset
> for adding the parsing of it, intended for dwmmc. I withdraw my ack
> for it, and let's try to go in the other direction instead.
> 
> [PATCHv3 0/4] mmc: fixed the mmc_of_parse for dwmmc.
> 
> Thus I suggest we should clean-up host drivers to support only one
> card per host, and entirely skip the slot concept.

Well, almost platform is used the only one card per host, although some controller is supported the slot concept.
But we don't know that controller should be used the multi slot per host, in future.
So I think we can't skip the slot concept.

Best Regards,
Jaehoon Chung

> 
> Kind regards
> Uffe
> 
>>
>> Thanks & Regards,
>>
>> Hans
>>
>>
>> *) Assuming you don't find any issues
> 

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Jaehoon Chung]

On 06/02/2014 05:38 PM, Jaehoon Chung wrote:
> On 06/02/2014 05:29 PM, Ulf Hansson wrote:
>> On 1 June 2014 11:23, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>>
>>> On 05/31/2014 10:13 PM, Olof Johansson wrote:
>>>>
>>>> On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com>
>>>> wrote:
>>>>>
>>>>> The following existing MMC host controller bindings use slot subnodes:
>>>>>
>>>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>>>> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>>>>> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>>>>> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
>>>>> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
>>>>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>>>>>
>>>>> This commit documents this practice in the standard mmc bindings
>>>>> documentation.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>
>>>>
>>>> There are today only two drivers that use this kind of binding, dw_mmc
>>>> and the at91 one.
>>>
>>>
>>> Correct.
>>>
>>>
>>>> Neither seems to actually ever have been used with
>>>> more than one slot. I doubt anyone building an exynos-based system
>>>> will ever do a multi-slot solution, and it seems that the at91 driver
>>>> doesn't actually handle more than one slot.
>>>>
>>>> I'm personally not that excited about complicating the bindings by
>>>> opening up for this -- I would rather work towards removing the
>>>> concept of slots if it's one of those things that are going to remain
>>>> unused. We have actually been talking about reworking the dw_mmc
>>>> binding to remove the slot concept (and simplify the driver by doing
>>>> so).
>>>
>>>
>>> I'm fine with removing the slot subnode, I added it because of it being
>>> brought up in the powerup sequence discussion. I explicitly asked there
>>> if adding such a subnode level was seen as desirable but nobody
>>> answered :|
>>>
>>> Anyways, either way works for me. I can do a v3 dropping the slot subnode
>>> level again. I would really like to move forward with a decision on how-to
>>> represent non probable info for sdio devices in device nodes. So do you
>>> have any other remarks other then that the slot subnode should be dropped ?
>>> And if not can you please review and ack (*) v3 of this patch-set once
>>> I've send it?
>>>
>>> Chris Ball and Ulf Hansson, what is your take on this, are you willing to
>>> take this patch set? And do you want it with or without the slot subnodes ?
>>
>> I certainly appreciate you working actively on this Hans, I will look
>> into the patchset as soon as I can.
>>
>> I share Olof's view about the slot nodes, we must not add DT bindings
>> that isn't really needed.
>>
>> Regarding the slot subnodes; Jaehoon Chung recently posted a patchset
>> for adding the parsing of it, intended for dwmmc. I withdraw my ack
>> for it, and let's try to go in the other direction instead.
>>
>> [PATCHv3 0/4] mmc: fixed the mmc_of_parse for dwmmc.
>>
>> Thus I suggest we should clean-up host drivers to support only one
>> card per host, and entirely skip the slot concept.
> 
> Well, almost platform is used the only one card per host, although some controller is supported the slot concept.
> But we don't know that controller should be used the multi slot per host, in future.
> So I think we can't skip the slot concept.
If we need to change the dw-mmc controller, let me know, plz.
I want to fix this problem before release the 3.16.
Actually, i think it can remove the subnode, if ensure not to use multi-slot at dwmmc.

Anyway, I will also consider to get more better solution. Thanks for pointing out.

Best Regards,
Jaehoon Chung

> 
> Best Regards,
> Jaehoon Chung
> 
>>
>> Kind regards
>> Uffe
>>
>>>
>>> Thanks & Regards,
>>>
>>> Hans
>>>
>>>
>>> *) Assuming you don't find any issues
>>
> 

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Ulf Hansson]

On 2 June 2014 10:46, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 06/02/2014 05:38 PM, Jaehoon Chung wrote:
>> On 06/02/2014 05:29 PM, Ulf Hansson wrote:
>>> On 1 June 2014 11:23, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Hi,
>>>>
>>>>
>>>> On 05/31/2014 10:13 PM, Olof Johansson wrote:
>>>>>
>>>>> On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com>
>>>>> wrote:
>>>>>>
>>>>>> The following existing MMC host controller bindings use slot subnodes:
>>>>>>
>>>>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>>>>> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>>>>>> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>>>>>> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
>>>>>> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
>>>>>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>>>>>>
>>>>>> This commit documents this practice in the standard mmc bindings
>>>>>> documentation.
>>>>>>
>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>
>>>>>
>>>>> There are today only two drivers that use this kind of binding, dw_mmc
>>>>> and the at91 one.
>>>>
>>>>
>>>> Correct.
>>>>
>>>>
>>>>> Neither seems to actually ever have been used with
>>>>> more than one slot. I doubt anyone building an exynos-based system
>>>>> will ever do a multi-slot solution, and it seems that the at91 driver
>>>>> doesn't actually handle more than one slot.
>>>>>
>>>>> I'm personally not that excited about complicating the bindings by
>>>>> opening up for this -- I would rather work towards removing the
>>>>> concept of slots if it's one of those things that are going to remain
>>>>> unused. We have actually been talking about reworking the dw_mmc
>>>>> binding to remove the slot concept (and simplify the driver by doing
>>>>> so).
>>>>
>>>>
>>>> I'm fine with removing the slot subnode, I added it because of it being
>>>> brought up in the powerup sequence discussion. I explicitly asked there
>>>> if adding such a subnode level was seen as desirable but nobody
>>>> answered :|
>>>>
>>>> Anyways, either way works for me. I can do a v3 dropping the slot subnode
>>>> level again. I would really like to move forward with a decision on how-to
>>>> represent non probable info for sdio devices in device nodes. So do you
>>>> have any other remarks other then that the slot subnode should be dropped ?
>>>> And if not can you please review and ack (*) v3 of this patch-set once
>>>> I've send it?
>>>>
>>>> Chris Ball and Ulf Hansson, what is your take on this, are you willing to
>>>> take this patch set? And do you want it with or without the slot subnodes ?
>>>
>>> I certainly appreciate you working actively on this Hans, I will look
>>> into the patchset as soon as I can.
>>>
>>> I share Olof's view about the slot nodes, we must not add DT bindings
>>> that isn't really needed.
>>>
>>> Regarding the slot subnodes; Jaehoon Chung recently posted a patchset
>>> for adding the parsing of it, intended for dwmmc. I withdraw my ack
>>> for it, and let's try to go in the other direction instead.
>>>
>>> [PATCHv3 0/4] mmc: fixed the mmc_of_parse for dwmmc.
>>>
>>> Thus I suggest we should clean-up host drivers to support only one
>>> card per host, and entirely skip the slot concept.
>>
>> Well, almost platform is used the only one card per host, although some controller is supported the slot concept.
>> But we don't know that controller should be used the multi slot per host, in future.
>> So I think we can't skip the slot concept.
> If we need to change the dw-mmc controller, let me know, plz.
> I want to fix this problem before release the 3.16.
> Actually, i think it can remove the subnode, if ensure not to use multi-slot at dwmmc.

That seems like the best approach. Please try to remove the subnodes
and make use of mmc_of_parse, as is.

Kind regards
Uffe

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Jaehoon Chung]

+ Seungwon Jeon

On 06/02/2014 05:52 PM, Ulf Hansson wrote:
> On 2 June 2014 10:46, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> On 06/02/2014 05:38 PM, Jaehoon Chung wrote:
>>> On 06/02/2014 05:29 PM, Ulf Hansson wrote:
>>>> On 1 June 2014 11:23, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>> Hi,
>>>>>
>>>>>
>>>>> On 05/31/2014 10:13 PM, Olof Johansson wrote:
>>>>>>
>>>>>> On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> The following existing MMC host controller bindings use slot subnodes:
>>>>>>>
>>>>>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>>>>>> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>>>>>>> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>>>>>>> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
>>>>>>> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
>>>>>>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>>>>>>>
>>>>>>> This commit documents this practice in the standard mmc bindings
>>>>>>> documentation.
>>>>>>>
>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>
>>>>>>
>>>>>> There are today only two drivers that use this kind of binding, dw_mmc
>>>>>> and the at91 one.
>>>>>
>>>>>
>>>>> Correct.
>>>>>
>>>>>
>>>>>> Neither seems to actually ever have been used with
>>>>>> more than one slot. I doubt anyone building an exynos-based system
>>>>>> will ever do a multi-slot solution, and it seems that the at91 driver
>>>>>> doesn't actually handle more than one slot.
>>>>>>
>>>>>> I'm personally not that excited about complicating the bindings by
>>>>>> opening up for this -- I would rather work towards removing the
>>>>>> concept of slots if it's one of those things that are going to remain
>>>>>> unused. We have actually been talking about reworking the dw_mmc
>>>>>> binding to remove the slot concept (and simplify the driver by doing
>>>>>> so).
>>>>>
>>>>>
>>>>> I'm fine with removing the slot subnode, I added it because of it being
>>>>> brought up in the powerup sequence discussion. I explicitly asked there
>>>>> if adding such a subnode level was seen as desirable but nobody
>>>>> answered :|
>>>>>
>>>>> Anyways, either way works for me. I can do a v3 dropping the slot subnode
>>>>> level again. I would really like to move forward with a decision on how-to
>>>>> represent non probable info for sdio devices in device nodes. So do you
>>>>> have any other remarks other then that the slot subnode should be dropped ?
>>>>> And if not can you please review and ack (*) v3 of this patch-set once
>>>>> I've send it?
>>>>>
>>>>> Chris Ball and Ulf Hansson, what is your take on this, are you willing to
>>>>> take this patch set? And do you want it with or without the slot subnodes ?
>>>>
>>>> I certainly appreciate you working actively on this Hans, I will look
>>>> into the patchset as soon as I can.
>>>>
>>>> I share Olof's view about the slot nodes, we must not add DT bindings
>>>> that isn't really needed.
>>>>
>>>> Regarding the slot subnodes; Jaehoon Chung recently posted a patchset
>>>> for adding the parsing of it, intended for dwmmc. I withdraw my ack
>>>> for it, and let's try to go in the other direction instead.
>>>>
>>>> [PATCHv3 0/4] mmc: fixed the mmc_of_parse for dwmmc.
>>>>
>>>> Thus I suggest we should clean-up host drivers to support only one
>>>> card per host, and entirely skip the slot concept.
>>>
>>> Well, almost platform is used the only one card per host, although some controller is supported the slot concept.
>>> But we don't know that controller should be used the multi slot per host, in future.
>>> So I think we can't skip the slot concept.
>> If we need to change the dw-mmc controller, let me know, plz.
>> I want to fix this problem before release the 3.16.
>> Actually, i think it can remove the subnode, if ensure not to use multi-slot at dwmmc.
> 
> That seems like the best approach. Please try to remove the subnodes
> and make use of mmc_of_parse, as is.
> 
> Kind regards
> Uffe
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Ulf Hansson]

On 2 June 2014 10:38, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 06/02/2014 05:29 PM, Ulf Hansson wrote:
>> On 1 June 2014 11:23, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>>
>>> On 05/31/2014 10:13 PM, Olof Johansson wrote:
>>>>
>>>> On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com>
>>>> wrote:
>>>>>
>>>>> The following existing MMC host controller bindings use slot subnodes:
>>>>>
>>>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>>>> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>>>>> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>>>>> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
>>>>> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
>>>>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>>>>>
>>>>> This commit documents this practice in the standard mmc bindings
>>>>> documentation.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>
>>>>
>>>> There are today only two drivers that use this kind of binding, dw_mmc
>>>> and the at91 one.
>>>
>>>
>>> Correct.
>>>
>>>
>>>> Neither seems to actually ever have been used with
>>>> more than one slot. I doubt anyone building an exynos-based system
>>>> will ever do a multi-slot solution, and it seems that the at91 driver
>>>> doesn't actually handle more than one slot.
>>>>
>>>> I'm personally not that excited about complicating the bindings by
>>>> opening up for this -- I would rather work towards removing the
>>>> concept of slots if it's one of those things that are going to remain
>>>> unused. We have actually been talking about reworking the dw_mmc
>>>> binding to remove the slot concept (and simplify the driver by doing
>>>> so).
>>>
>>>
>>> I'm fine with removing the slot subnode, I added it because of it being
>>> brought up in the powerup sequence discussion. I explicitly asked there
>>> if adding such a subnode level was seen as desirable but nobody
>>> answered :|
>>>
>>> Anyways, either way works for me. I can do a v3 dropping the slot subnode
>>> level again. I would really like to move forward with a decision on how-to
>>> represent non probable info for sdio devices in device nodes. So do you
>>> have any other remarks other then that the slot subnode should be dropped ?
>>> And if not can you please review and ack (*) v3 of this patch-set once
>>> I've send it?
>>>
>>> Chris Ball and Ulf Hansson, what is your take on this, are you willing to
>>> take this patch set? And do you want it with or without the slot subnodes ?
>>
>> I certainly appreciate you working actively on this Hans, I will look
>> into the patchset as soon as I can.
>>
>> I share Olof's view about the slot nodes, we must not add DT bindings
>> that isn't really needed.
>>
>> Regarding the slot subnodes; Jaehoon Chung recently posted a patchset
>> for adding the parsing of it, intended for dwmmc. I withdraw my ack
>> for it, and let's try to go in the other direction instead.
>>
>> [PATCHv3 0/4] mmc: fixed the mmc_of_parse for dwmmc.
>>
>> Thus I suggest we should clean-up host drivers to support only one
>> card per host, and entirely skip the slot concept.
>
> Well, almost platform is used the only one card per host, although some controller is supported the slot concept.
> But we don't know that controller should be used the multi slot per host, in future.
> So I think we can't skip the slot concept.

The mmc core only supports one card per host.

Adding DT bindings for something that seems unlikely to be supported
in future, seems like a bad idea. It's better to add it when/if
needed.

Kind regards
Uffe

>
> Best Regards,
> Jaehoon Chung
>
>>
>> Kind regards
>> Uffe
>>
>>>
>>> Thanks & Regards,
>>>
>>> Hans
>>>
>>>
>>> *) Assuming you don't find any issues
>>
>

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Jaehoon Chung]

+Suegnwon Jeon

On 06/02/2014 05:48 PM, Ulf Hansson wrote:
> On 2 June 2014 10:38, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> On 06/02/2014 05:29 PM, Ulf Hansson wrote:
>>> On 1 June 2014 11:23, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Hi,
>>>>
>>>>
>>>> On 05/31/2014 10:13 PM, Olof Johansson wrote:
>>>>>
>>>>> On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com>
>>>>> wrote:
>>>>>>
>>>>>> The following existing MMC host controller bindings use slot subnodes:
>>>>>>
>>>>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>>>>> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>>>>>> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>>>>>> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
>>>>>> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
>>>>>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>>>>>>
>>>>>> This commit documents this practice in the standard mmc bindings
>>>>>> documentation.
>>>>>>
>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>
>>>>>
>>>>> There are today only two drivers that use this kind of binding, dw_mmc
>>>>> and the at91 one.
>>>>
>>>>
>>>> Correct.
>>>>
>>>>
>>>>> Neither seems to actually ever have been used with
>>>>> more than one slot. I doubt anyone building an exynos-based system
>>>>> will ever do a multi-slot solution, and it seems that the at91 driver
>>>>> doesn't actually handle more than one slot.
>>>>>
>>>>> I'm personally not that excited about complicating the bindings by
>>>>> opening up for this -- I would rather work towards removing the
>>>>> concept of slots if it's one of those things that are going to remain
>>>>> unused. We have actually been talking about reworking the dw_mmc
>>>>> binding to remove the slot concept (and simplify the driver by doing
>>>>> so).
>>>>
>>>>
>>>> I'm fine with removing the slot subnode, I added it because of it being
>>>> brought up in the powerup sequence discussion. I explicitly asked there
>>>> if adding such a subnode level was seen as desirable but nobody
>>>> answered :|
>>>>
>>>> Anyways, either way works for me. I can do a v3 dropping the slot subnode
>>>> level again. I would really like to move forward with a decision on how-to
>>>> represent non probable info for sdio devices in device nodes. So do you
>>>> have any other remarks other then that the slot subnode should be dropped ?
>>>> And if not can you please review and ack (*) v3 of this patch-set once
>>>> I've send it?
>>>>
>>>> Chris Ball and Ulf Hansson, what is your take on this, are you willing to
>>>> take this patch set? And do you want it with or without the slot subnodes ?
>>>
>>> I certainly appreciate you working actively on this Hans, I will look
>>> into the patchset as soon as I can.
>>>
>>> I share Olof's view about the slot nodes, we must not add DT bindings
>>> that isn't really needed.
>>>
>>> Regarding the slot subnodes; Jaehoon Chung recently posted a patchset
>>> for adding the parsing of it, intended for dwmmc. I withdraw my ack
>>> for it, and let's try to go in the other direction instead.
>>>
>>> [PATCHv3 0/4] mmc: fixed the mmc_of_parse for dwmmc.
>>>
>>> Thus I suggest we should clean-up host drivers to support only one
>>> card per host, and entirely skip the slot concept.
>>
>> Well, almost platform is used the only one card per host, although some controller is supported the slot concept.
>> But we don't know that controller should be used the multi slot per host, in future.
>> So I think we can't skip the slot concept.
> 
> The mmc core only supports one card per host.

Right, mmc core supports one card per host, but host controller can be supported the multiple slot, right?
Of course, it should be handled at host controller, not core.
> 
> Adding DT bindings for something that seems unlikely to be supported
> in future, seems like a bad idea. It's better to add it when/if
> needed.
If some SoC use the multiple slot for dw-mmc controller, we can't prevent to use the multiple slot.
So i'm not sure that host controller's subnode didn't need to support.
Right. this is bad idea, i also hope that it will not use the multiple slot at dw-mmc in future.

To Seungwon,

how about this?

Best Regards,
Jaehoon Chung
> 
> Kind regards
> Uffe
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> Kind regards
>>> Uffe
>>>
>>>>
>>>> Thanks & Regards,
>>>>
>>>> Hans
>>>>
>>>>
>>>> *) Assuming you don't find any issues
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
> 

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Ulf Hansson]

On 3 June 2014 03:50, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> +Suegnwon Jeon
>
> On 06/02/2014 05:48 PM, Ulf Hansson wrote:
>> On 2 June 2014 10:38, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> On 06/02/2014 05:29 PM, Ulf Hansson wrote:
>>>> On 1 June 2014 11:23, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>> Hi,
>>>>>
>>>>>
>>>>> On 05/31/2014 10:13 PM, Olof Johansson wrote:
>>>>>>
>>>>>> On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> The following existing MMC host controller bindings use slot subnodes:
>>>>>>>
>>>>>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>>>>>> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
>>>>>>> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
>>>>>>> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
>>>>>>> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
>>>>>>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>>>>>>>
>>>>>>> This commit documents this practice in the standard mmc bindings
>>>>>>> documentation.
>>>>>>>
>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>
>>>>>>
>>>>>> There are today only two drivers that use this kind of binding, dw_mmc
>>>>>> and the at91 one.
>>>>>
>>>>>
>>>>> Correct.
>>>>>
>>>>>
>>>>>> Neither seems to actually ever have been used with
>>>>>> more than one slot. I doubt anyone building an exynos-based system
>>>>>> will ever do a multi-slot solution, and it seems that the at91 driver
>>>>>> doesn't actually handle more than one slot.
>>>>>>
>>>>>> I'm personally not that excited about complicating the bindings by
>>>>>> opening up for this -- I would rather work towards removing the
>>>>>> concept of slots if it's one of those things that are going to remain
>>>>>> unused. We have actually been talking about reworking the dw_mmc
>>>>>> binding to remove the slot concept (and simplify the driver by doing
>>>>>> so).
>>>>>
>>>>>
>>>>> I'm fine with removing the slot subnode, I added it because of it being
>>>>> brought up in the powerup sequence discussion. I explicitly asked there
>>>>> if adding such a subnode level was seen as desirable but nobody
>>>>> answered :|
>>>>>
>>>>> Anyways, either way works for me. I can do a v3 dropping the slot subnode
>>>>> level again. I would really like to move forward with a decision on how-to
>>>>> represent non probable info for sdio devices in device nodes. So do you
>>>>> have any other remarks other then that the slot subnode should be dropped ?
>>>>> And if not can you please review and ack (*) v3 of this patch-set once
>>>>> I've send it?
>>>>>
>>>>> Chris Ball and Ulf Hansson, what is your take on this, are you willing to
>>>>> take this patch set? And do you want it with or without the slot subnodes ?
>>>>
>>>> I certainly appreciate you working actively on this Hans, I will look
>>>> into the patchset as soon as I can.
>>>>
>>>> I share Olof's view about the slot nodes, we must not add DT bindings
>>>> that isn't really needed.
>>>>
>>>> Regarding the slot subnodes; Jaehoon Chung recently posted a patchset
>>>> for adding the parsing of it, intended for dwmmc. I withdraw my ack
>>>> for it, and let's try to go in the other direction instead.
>>>>
>>>> [PATCHv3 0/4] mmc: fixed the mmc_of_parse for dwmmc.
>>>>
>>>> Thus I suggest we should clean-up host drivers to support only one
>>>> card per host, and entirely skip the slot concept.
>>>
>>> Well, almost platform is used the only one card per host, although some controller is supported the slot concept.
>>> But we don't know that controller should be used the multi slot per host, in future.
>>> So I think we can't skip the slot concept.
>>
>> The mmc core only supports one card per host.
>
> Right, mmc core supports one card per host, but host controller can be supported the multiple slot, right?
> Of course, it should be handled at host controller, not core.

The core needs to be involved as well. How will the core otherwise be
able to tell which card to switch to (which also involves sending
actual CMDs to the card), when sending requests.

I would be surprised if SOCs/boards ever want to use this kind of
configuration - simply because of the bad impact on performance and
latency. Until we have a valid case, I just want us to continue to
ignore this option.

Kind regards
Ulf Hansson

>>
>> Adding DT bindings for something that seems unlikely to be supported
>> in future, seems like a bad idea. It's better to add it when/if
>> needed.
> If some SoC use the multiple slot for dw-mmc controller, we can't prevent to use the multiple slot.
> So i'm not sure that host controller's subnode didn't need to support.
> Right. this is bad idea, i also hope that it will not use the multiple slot at dw-mmc in future.
>
> To Seungwon,
>
> how about this?
>
> Best Regards,
> Jaehoon Chung
>>
>> Kind regards
>> Uffe
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>>
>>>> Kind regards
>>>> Uffe
>>>>
>>>>>
>>>>> Thanks & Regards,
>>>>>
>>>>> Hans
>>>>>
>>>>>
>>>>> *) Assuming you don't find any issues
>>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> .
>>
>

[linux-sunxi] [PATCH v2 1/4] dt: bindings: mmc: Document the practice of using subnodes for slots

[Seungwon Jeon]

On Tue, June 03, 2014, Jaehoon Chung wrote:
> +Suegnwon Jeon
> 
> On 06/02/2014 05:48 PM, Ulf Hansson wrote:
> > On 2 June 2014 10:38, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> >> On 06/02/2014 05:29 PM, Ulf Hansson wrote:
> >>> On 1 June 2014 11:23, Hans de Goede <hdegoede@redhat.com> wrote:
> >>>> Hi,
> >>>>
> >>>>
> >>>> On 05/31/2014 10:13 PM, Olof Johansson wrote:
> >>>>>
> >>>>> On Sat, May 31, 2014 at 12:03 PM, Hans de Goede <hdegoede@redhat.com>
> >>>>> wrote:
> >>>>>>
> >>>>>> The following existing MMC host controller bindings use slot subnodes:
> >>>>>>
> >>>>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> >>>>>> Documentation/devicetree/bindings/mmc/k3-dw-mshc.txt
> >>>>>> Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
> >>>>>> Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
> >>>>>> Documentation/devicetree/bindings/mmc/atmel-hsmci.txt
> >>>>>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> >>>>>>
> >>>>>> This commit documents this practice in the standard mmc bindings
> >>>>>> documentation.
> >>>>>>
> >>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>>>>
> >>>>>
> >>>>> There are today only two drivers that use this kind of binding, dw_mmc
> >>>>> and the at91 one.
> >>>>
> >>>>
> >>>> Correct.
> >>>>
> >>>>
> >>>>> Neither seems to actually ever have been used with
> >>>>> more than one slot. I doubt anyone building an exynos-based system
> >>>>> will ever do a multi-slot solution, and it seems that the at91 driver
> >>>>> doesn't actually handle more than one slot.
> >>>>>
> >>>>> I'm personally not that excited about complicating the bindings by
> >>>>> opening up for this -- I would rather work towards removing the
> >>>>> concept of slots if it's one of those things that are going to remain
> >>>>> unused. We have actually been talking about reworking the dw_mmc
> >>>>> binding to remove the slot concept (and simplify the driver by doing
> >>>>> so).
> >>>>
> >>>>
> >>>> I'm fine with removing the slot subnode, I added it because of it being
> >>>> brought up in the powerup sequence discussion. I explicitly asked there
> >>>> if adding such a subnode level was seen as desirable but nobody
> >>>> answered :|
> >>>>
> >>>> Anyways, either way works for me. I can do a v3 dropping the slot subnode
> >>>> level again. I would really like to move forward with a decision on how-to
> >>>> represent non probable info for sdio devices in device nodes. So do you
> >>>> have any other remarks other then that the slot subnode should be dropped ?
> >>>> And if not can you please review and ack (*) v3 of this patch-set once
> >>>> I've send it?
> >>>>
> >>>> Chris Ball and Ulf Hansson, what is your take on this, are you willing to
> >>>> take this patch set? And do you want it with or without the slot subnodes ?
> >>>
> >>> I certainly appreciate you working actively on this Hans, I will look
> >>> into the patchset as soon as I can.
> >>>
> >>> I share Olof's view about the slot nodes, we must not add DT bindings
> >>> that isn't really needed.
> >>>
> >>> Regarding the slot subnodes; Jaehoon Chung recently posted a patchset
> >>> for adding the parsing of it, intended for dwmmc. I withdraw my ack
> >>> for it, and let's try to go in the other direction instead.
> >>>
> >>> [PATCHv3 0/4] mmc: fixed the mmc_of_parse for dwmmc.
> >>>
> >>> Thus I suggest we should clean-up host drivers to support only one
> >>> card per host, and entirely skip the slot concept.
> >>
> >> Well, almost platform is used the only one card per host, although some controller is supported the
> slot concept.
> >> But we don't know that controller should be used the multi slot per host, in future.
> >> So I think we can't skip the slot concept.
> >
> > The mmc core only supports one card per host.
> 
> Right, mmc core supports one card per host, but host controller can be supported the multiple slot,
> right?
> Of course, it should be handled at host controller, not core.
> >
> > Adding DT bindings for something that seems unlikely to be supported
> > in future, seems like a bad idea. It's better to add it when/if
> > needed.
> If some SoC use the multiple slot for dw-mmc controller, we can't prevent to use the multiple slot.
> So i'm not sure that host controller's subnode didn't need to support.
> Right. this is bad idea, i also hope that it will not use the multiple slot at dw-mmc in future.
> 
> To Seungwon,
> 
> how about this?

I have no objection to remove multi-slot.
It seems not useful considering performance. Above all, there is no actual use case.

Thanks,
Seungwon Jeon

[PATCH v2 2/4] dt: bindings: mmc: Add sdio function subnode documentation

[Hans de Goede]

From: Sascha Hauer <s.hauer@pengutronix.de>

While SDIO devices are runtime probable they sometimes need nonprobable
additional information on embedded systems, like an additional gpio
interrupt or a clock. This binding describes how to add child nodes to the
devicetree to supply this information.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
[hdegoede at redhat.com: Updated the bindings to put the sdio functions inside
 slot subnodes]
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/devicetree/bindings/mmc/mmc.txt | 49 +++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 44c9e53..2903775 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -81,6 +81,27 @@ if a property is specified at both the host and the slot level the slot
 level takes precedence.
 
 
+Use of Function subnodes
+------------------------
+
+On embedded systems the cards connected to a host may need additional
+properties. These can be specified in subnodes to the slot subnodes, with
+the card / SDIO-function identified by the standard 'reg' property.
+Note that the use of slot subnodes is mandatory in this case.
+Which information exactly can be specified depends on the bindings for the
+SDIO function driver for the node, as specified by the compatible string.
+
+Required slot subnode properties when using function subnodes:
+- #address-cells: should be one. The cell is the slot id.
+- #size-cells: should be zero.
+
+Required function subnode properties:
+- compatible: name of SDIO function following generic names recommended practice
+- reg: Must contain the SDIO function number of the function this subnode
+       describes. A value of 0 denotes the memory SD function, values from
+       1 to 7 denote the SDIO functions.
+
+
 Examples
 --------
 
@@ -118,3 +139,31 @@ mmc0: mmc at f0008000 {
 		bus-width = <4>;
 	};
 };
+
+Example with sdio function subnodes:
+
+mmc3: mmc at 01c12000 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc3_pins_a>;
+	vmmc-supply = <&reg_vmmc3>;
+	bus-width = <4>;
+	non-removable;
+	status = "okay";
+
+	mmc3_slot0: mmc3_slot at 0 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0>;
+
+		brcmf: bcrmf at 1 {
+			reg = <1>;
+			compatible = "brcm,bcm43xx-fmac";
+			interrupt-parent = <&pio>;
+			interrupts = <10 8>; /* PH10 / EINT10 */
+			interrupt-names = "host-wake";
+		};
+	};
+};
-- 
2.0.0

[PATCH v2 3/4] mmc: Set slot_no on devicetree / of systems too

[Hans de Goede]

Some host controllers have multiple slots. The mmc-core sofar does not
really have any knowledge of this, the host drivers for these controllers
simple call mmc_add_host() multiple times.

These mmc_host-s will share their parent device and on devicetree systems
also their parent->of_node.

This commit makes multi-slot mmc host drivers set the already existing, but
sofar only used for ACPI, slot_no member of mmc_host so that mmc-core functions
parsing the devicetree can determine what the right slot devicetree subnode is
to parse for a multi-slot host.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mmc/host/atmel-mci.c | 1 +
 drivers/mmc/host/dw_mmc.c    | 1 +
 include/linux/mmc/host.h     | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 42706ea..a58906b 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -2170,6 +2170,7 @@ static int __init atmci_init_slot(struct atmel_mci *host,
 		slot_data->wp_pin);
 
 	mmc->ops = &atmci_ops;
+	mmc->slotno = id;
 	mmc->f_min = DIV_ROUND_UP(host->bus_hz, 512);
 	mmc->f_max = host->bus_hz / 2;
 	mmc->ocr_avail	= MMC_VDD_32_33 | MMC_VDD_33_34;
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index cced599..c27a86b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2149,6 +2149,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 	slot->quirks = dw_mci_of_get_slot_quirks(host->dev, slot->id);
 
 	mmc->ops = &dw_mci_ops;
+	mmc->slotno = id;
 	if (of_property_read_u32_array(host->dev->of_node,
 				       "clock-freq-min-max", freq, 2)) {
 		mmc->f_min = DW_MCI_FREQ_MIN;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index cb61ea4..10d1091 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -356,7 +356,7 @@ struct mmc_host {
 
 	unsigned int		actual_clock;	/* Actual HC clock rate */
 
-	unsigned int		slotno;	/* used for sdio acpi binding */
+	unsigned int		slotno;	/* used for sdio acpi / of binding */
 
 	unsigned long		private[0] ____cacheline_aligned;
 };
-- 
2.0.0

[PATCH v2 4/4] mmc: Add SDIO function devicetree subnode parsing

[Hans de Goede]

From: Sascha Hauer <s.hauer@pengutronix.de>

This adds SDIO devicetree subnode parsing to the mmc core. While
SDIO devices are runtime probable they sometimes need nonprobable
additional information on embedded systems, like an additional gpio
interrupt or a clock. This patch makes it possible to supply this
information from the devicetree. SDIO drivers will find a pointer
to the devicenode in their devices of_node pointer.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
[hdegoede at redhat.com: Updated to parse sdio functions inside slot subnodes]
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mmc/core/bus.c      |  4 ++++
 drivers/mmc/core/core.c     | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/mmc/core/core.h     |  3 +++
 drivers/mmc/core/sdio_bus.c | 11 +++++++++++
 4 files changed, 56 insertions(+)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 8246448..90b0ce8 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -16,6 +16,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/stat.h>
+#include <linux/of.h>
 #include <linux/pm_runtime.h>
 
 #include <linux/mmc/card.h>
@@ -359,6 +360,8 @@ int mmc_add_card(struct mmc_card *card)
 #endif
 	mmc_init_context_info(card->host);
 
+	card->dev.of_node = mmc_of_find_child_device(card->host, 0);
+
 	ret = device_add(&card->dev);
 	if (ret)
 		return ret;
@@ -387,6 +390,7 @@ void mmc_remove_card(struct mmc_card *card)
 				mmc_hostname(card->host), card->rca);
 		}
 		device_del(&card->dev);
+		of_node_put(card->dev.of_node);
 	}
 
 	put_device(&card->dev);
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index acbc3f2..70d9f88 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1201,6 +1201,44 @@ EXPORT_SYMBOL(mmc_of_parse_voltage);
 
 #endif /* CONFIG_OF */
 
+static int mmc_of_get_reg(struct device_node *node)
+{
+	u32 reg;
+	int ret;
+
+	ret = of_property_read_u32(node, "reg", &reg);
+	if (ret < 0)
+		return ret;
+
+	return reg;
+}
+
+struct device_node *mmc_of_find_child_device(struct mmc_host *host,
+		unsigned func_num)
+{
+	struct device_node *parent = host->parent->of_node;
+	struct device_node *node, *slot = NULL;
+
+	if (!parent)
+		return NULL;
+
+	for_each_child_of_node(parent, node) {
+		if (mmc_of_get_reg(node) == host->slotno) {
+			slot = node;
+			break;
+		}
+	}
+	if (!slot)
+		return NULL;
+
+	for_each_child_of_node(slot, node) {
+		if (mmc_of_get_reg(node) == func_num)
+			return node;
+	}
+
+	return NULL;
+}
+
 #ifdef CONFIG_REGULATOR
 
 /**
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 443a584..f712f6e 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -32,6 +32,9 @@ struct mmc_bus_ops {
 void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops);
 void mmc_detach_bus(struct mmc_host *host);
 
+struct device_node *mmc_of_find_child_device(struct mmc_host *host,
+		unsigned func_num);
+
 void mmc_init_erase(struct mmc_card *card);
 
 void mmc_set_chip_select(struct mmc_host *host, int mode);
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 92d1ba8..35c23ae 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -21,7 +21,9 @@
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/sdio_func.h>
+#include <linux/of.h>
 
+#include "core.h"
 #include "sdio_cis.h"
 #include "sdio_bus.h"
 
@@ -314,6 +316,13 @@ static void sdio_acpi_set_handle(struct sdio_func *func)
 static inline void sdio_acpi_set_handle(struct sdio_func *func) {}
 #endif
 
+static void sdio_set_of_node(struct sdio_func *func)
+{
+	struct mmc_host *host = func->card->host;
+
+	func->dev.of_node = mmc_of_find_child_device(host, func->num);
+}
+
 /*
  * Register a new SDIO function with the driver model.
  */
@@ -323,6 +332,7 @@ int sdio_add_func(struct sdio_func *func)
 
 	dev_set_name(&func->dev, "%s:%d", mmc_card_id(func->card), func->num);
 
+	sdio_set_of_node(func);
 	sdio_acpi_set_handle(func);
 	ret = device_add(&func->dev);
 	if (ret == 0) {
@@ -346,6 +356,7 @@ void sdio_remove_func(struct sdio_func *func)
 
 	acpi_dev_pm_detach(&func->dev, false);
 	device_del(&func->dev);
+	of_node_put(func->dev.of_node);
 	put_device(&func->dev);
 }
 
-- 
2.0.0