diff mbox

[RFC] mmc: add slot argument to mmc_of_parse

Message ID 1399457217-22846-1-git-send-email-ludovic.desroches@atmel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ludovic Desroches May 7, 2014, 10:06 a.m. UTC
Some hosts manage several slots. In these case information such as the bus
width, chip detect and others are into the slot node. So we have to parse
child nodes. If not NULL, slot node will be used instead of the device
node.

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---

Hi,

Since this patch is only a RFC, I have not yet updated drivers using this
function.

I would like to use mmc_of_parse to reduce code duplication. My issue is that
atmel mci is a bit different from others mci host since it can provide
several slots, so it allocates several mmc hosts. By the way, it is not the
only one.

When calling mmc_alloc_host, host->parent is set to &pdev->dev. mmc_of_parse
uses host->parent->of_node but in my case settings are in the slot nodes so in
the child nodes. That's why I would like to have a way to tell which node I
want to use.


Regards

Ludovic



 drivers/mmc/core/host.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Ulf Hansson May 14, 2014, 9:53 a.m. UTC | #1
On 7 May 2014 12:06, Ludovic Desroches <ludovic.desroches@atmel.com> wrote:
> Some hosts manage several slots. In these case information such as the bus
> width, chip detect and others are into the slot node. So we have to parse
> child nodes. If not NULL, slot node will be used instead of the device
> node.
>
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> ---
>
> Hi,
>
> Since this patch is only a RFC, I have not yet updated drivers using this
> function.
>
> I would like to use mmc_of_parse to reduce code duplication. My issue is that
> atmel mci is a bit different from others mci host since it can provide
> several slots, so it allocates several mmc hosts. By the way, it is not the
> only one.
>
> When calling mmc_alloc_host, host->parent is set to &pdev->dev. mmc_of_parse
> uses host->parent->of_node but in my case settings are in the slot nodes so in
> the child nodes. That's why I would like to have a way to tell which node I
> want to use.

Seems reasonable, thanks for working on this!

>
>
> Regards
>
> Ludovic
>
>
>
>  drivers/mmc/core/host.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index fdea825..ed6cea5 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -300,13 +300,15 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
>  /**
>   *     mmc_of_parse() - parse host's device-tree node
>   *     @host: host whose node should be parsed.
> + *     @slot: some devices provide several slots so the node to parse
> + *     is not the host one.
>   *
>   * To keep the rest of the MMC subsystem unaware of whether DT has been
>   * used to to instantiate and configure this host instance or not, we
>   * parse the properties and set respective generic mmc-host flags and
>   * parameters.
>   */
> -int mmc_of_parse(struct mmc_host *host)
> +int mmc_of_parse(struct mmc_host *host, struct device_node *slot)
>  {
>         struct device_node *np;
>         u32 bus_width;
> @@ -317,7 +319,10 @@ int mmc_of_parse(struct mmc_host *host)
>         if (!host->parent || !host->parent->of_node)
>                 return 0;
>
> -       np = host->parent->of_node;
> +       if (slot)
> +               np = slot;
> +       else
> +               np = host->parent->of_node;
>
>         /* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
>         if (of_property_read_u32(np, "bus-width", &bus_width) < 0) {
> --
> 1.7.9.5
>

How about adding a new API, __mmc_of_parse((struct mmc_host *host,
struct device_node *slot)
Then let the old API mmc_of_parse() remain as is, but let it call the
new API with slot == NULL.

Atmel can then use the new API, but the other drivers can remain as is.

Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jaehoon Chung May 23, 2014, 4:38 a.m. UTC | #2
Hi, All.

This patch is working on progress?
I want to merge this patch for fixing dw-mmc controller problem.

If this patch didn't work on progress, i will send the patch based-on this patch.

Best Regards,
Jaehoon Chung

On 05/14/2014 06:53 PM, Ulf Hansson wrote:
> On 7 May 2014 12:06, Ludovic Desroches <ludovic.desroches@atmel.com> wrote:
>> Some hosts manage several slots. In these case information such as the bus
>> width, chip detect and others are into the slot node. So we have to parse
>> child nodes. If not NULL, slot node will be used instead of the device
>> node.
>>
>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>> ---
>>
>> Hi,
>>
>> Since this patch is only a RFC, I have not yet updated drivers using this
>> function.
>>
>> I would like to use mmc_of_parse to reduce code duplication. My issue is that
>> atmel mci is a bit different from others mci host since it can provide
>> several slots, so it allocates several mmc hosts. By the way, it is not the
>> only one.
>>
>> When calling mmc_alloc_host, host->parent is set to &pdev->dev. mmc_of_parse
>> uses host->parent->of_node but in my case settings are in the slot nodes so in
>> the child nodes. That's why I would like to have a way to tell which node I
>> want to use.
> 
> Seems reasonable, thanks for working on this!
> 
>>
>>
>> Regards
>>
>> Ludovic
>>
>>
>>
>>  drivers/mmc/core/host.c |    9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>> index fdea825..ed6cea5 100644
>> --- a/drivers/mmc/core/host.c
>> +++ b/drivers/mmc/core/host.c
>> @@ -300,13 +300,15 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
>>  /**
>>   *     mmc_of_parse() - parse host's device-tree node
>>   *     @host: host whose node should be parsed.
>> + *     @slot: some devices provide several slots so the node to parse
>> + *     is not the host one.
>>   *
>>   * To keep the rest of the MMC subsystem unaware of whether DT has been
>>   * used to to instantiate and configure this host instance or not, we
>>   * parse the properties and set respective generic mmc-host flags and
>>   * parameters.
>>   */
>> -int mmc_of_parse(struct mmc_host *host)
>> +int mmc_of_parse(struct mmc_host *host, struct device_node *slot)
>>  {
>>         struct device_node *np;
>>         u32 bus_width;
>> @@ -317,7 +319,10 @@ int mmc_of_parse(struct mmc_host *host)
>>         if (!host->parent || !host->parent->of_node)
>>                 return 0;
>>
>> -       np = host->parent->of_node;
>> +       if (slot)
>> +               np = slot;
>> +       else
>> +               np = host->parent->of_node;
>>
>>         /* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
>>         if (of_property_read_u32(np, "bus-width", &bus_width) < 0) {
>> --
>> 1.7.9.5
>>
> 
> How about adding a new API, __mmc_of_parse((struct mmc_host *host,
> struct device_node *slot)
> Then let the old API mmc_of_parse() remain as is, but let it call the
> new API with slot == NULL.
> 
> Atmel can then use the new API, but the other drivers can remain as is.
> 
> Kind regards
> Ulf Hansson
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ludovic Desroches May 26, 2014, 7:03 a.m. UTC | #3
Hi,

Sorry I was on vacation, I'll send a cleaner patch updating other drivers
this week.

Regards

Ludovic

On Fri, May 23, 2014 at 01:38:06PM +0900, Jaehoon Chung wrote:
> Hi, All.
> 
> This patch is working on progress?
> I want to merge this patch for fixing dw-mmc controller problem.
> 
> If this patch didn't work on progress, i will send the patch based-on this patch.
> 
> Best Regards,
> Jaehoon Chung
> 
> On 05/14/2014 06:53 PM, Ulf Hansson wrote:
> > On 7 May 2014 12:06, Ludovic Desroches <ludovic.desroches@atmel.com> wrote:
> >> Some hosts manage several slots. In these case information such as the bus
> >> width, chip detect and others are into the slot node. So we have to parse
> >> child nodes. If not NULL, slot node will be used instead of the device
> >> node.
> >>
> >> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> >> ---
> >>
> >> Hi,
> >>
> >> Since this patch is only a RFC, I have not yet updated drivers using this
> >> function.
> >>
> >> I would like to use mmc_of_parse to reduce code duplication. My issue is that
> >> atmel mci is a bit different from others mci host since it can provide
> >> several slots, so it allocates several mmc hosts. By the way, it is not the
> >> only one.
> >>
> >> When calling mmc_alloc_host, host->parent is set to &pdev->dev. mmc_of_parse
> >> uses host->parent->of_node but in my case settings are in the slot nodes so in
> >> the child nodes. That's why I would like to have a way to tell which node I
> >> want to use.
> > 
> > Seems reasonable, thanks for working on this!
> > 
> >>
> >>
> >> Regards
> >>
> >> Ludovic
> >>
> >>
> >>
> >>  drivers/mmc/core/host.c |    9 +++++++--
> >>  1 file changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> >> index fdea825..ed6cea5 100644
> >> --- a/drivers/mmc/core/host.c
> >> +++ b/drivers/mmc/core/host.c
> >> @@ -300,13 +300,15 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
> >>  /**
> >>   *     mmc_of_parse() - parse host's device-tree node
> >>   *     @host: host whose node should be parsed.
> >> + *     @slot: some devices provide several slots so the node to parse
> >> + *     is not the host one.
> >>   *
> >>   * To keep the rest of the MMC subsystem unaware of whether DT has been
> >>   * used to to instantiate and configure this host instance or not, we
> >>   * parse the properties and set respective generic mmc-host flags and
> >>   * parameters.
> >>   */
> >> -int mmc_of_parse(struct mmc_host *host)
> >> +int mmc_of_parse(struct mmc_host *host, struct device_node *slot)
> >>  {
> >>         struct device_node *np;
> >>         u32 bus_width;
> >> @@ -317,7 +319,10 @@ int mmc_of_parse(struct mmc_host *host)
> >>         if (!host->parent || !host->parent->of_node)
> >>                 return 0;
> >>
> >> -       np = host->parent->of_node;
> >> +       if (slot)
> >> +               np = slot;
> >> +       else
> >> +               np = host->parent->of_node;
> >>
> >>         /* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
> >>         if (of_property_read_u32(np, "bus-width", &bus_width) < 0) {
> >> --
> >> 1.7.9.5
> >>
> > 
> > How about adding a new API, __mmc_of_parse((struct mmc_host *host,
> > struct device_node *slot)
> > Then let the old API mmc_of_parse() remain as is, but let it call the
> > new API with slot == NULL.
> > 
> > Atmel can then use the new API, but the other drivers can remain as is.
> > 
> > Kind regards
> > Ulf Hansson
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jaehoon Chung May 26, 2014, 7:15 a.m. UTC | #4
Hi, Ludovic.

I have sent the patch based on your RFC patch..how about?
If you have the comment or other opinion, let me know, plz.

https://patchwork.kernel.org/patch/4230101/

It needs to use your suggestion at dw-mmc controller, so i have posted it.

Best Regards,
Jaehoon Chung

On 05/26/2014 04:03 PM, Ludovic Desroches wrote:
> Hi,
> 
> Sorry I was on vacation, I'll send a cleaner patch updating other drivers
> this week.
> 
> Regards
> 
> Ludovic
> 
> On Fri, May 23, 2014 at 01:38:06PM +0900, Jaehoon Chung wrote:
>> Hi, All.
>>
>> This patch is working on progress?
>> I want to merge this patch for fixing dw-mmc controller problem.
>>
>> If this patch didn't work on progress, i will send the patch based-on this patch.
>>
>> Best Regards,
>> Jaehoon Chung
>>
>> On 05/14/2014 06:53 PM, Ulf Hansson wrote:
>>> On 7 May 2014 12:06, Ludovic Desroches <ludovic.desroches@atmel.com> wrote:
>>>> Some hosts manage several slots. In these case information such as the bus
>>>> width, chip detect and others are into the slot node. So we have to parse
>>>> child nodes. If not NULL, slot node will be used instead of the device
>>>> node.
>>>>
>>>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>>>> ---
>>>>
>>>> Hi,
>>>>
>>>> Since this patch is only a RFC, I have not yet updated drivers using this
>>>> function.
>>>>
>>>> I would like to use mmc_of_parse to reduce code duplication. My issue is that
>>>> atmel mci is a bit different from others mci host since it can provide
>>>> several slots, so it allocates several mmc hosts. By the way, it is not the
>>>> only one.
>>>>
>>>> When calling mmc_alloc_host, host->parent is set to &pdev->dev. mmc_of_parse
>>>> uses host->parent->of_node but in my case settings are in the slot nodes so in
>>>> the child nodes. That's why I would like to have a way to tell which node I
>>>> want to use.
>>>
>>> Seems reasonable, thanks for working on this!
>>>
>>>>
>>>>
>>>> Regards
>>>>
>>>> Ludovic
>>>>
>>>>
>>>>
>>>>  drivers/mmc/core/host.c |    9 +++++++--
>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
>>>> index fdea825..ed6cea5 100644
>>>> --- a/drivers/mmc/core/host.c
>>>> +++ b/drivers/mmc/core/host.c
>>>> @@ -300,13 +300,15 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
>>>>  /**
>>>>   *     mmc_of_parse() - parse host's device-tree node
>>>>   *     @host: host whose node should be parsed.
>>>> + *     @slot: some devices provide several slots so the node to parse
>>>> + *     is not the host one.
>>>>   *
>>>>   * To keep the rest of the MMC subsystem unaware of whether DT has been
>>>>   * used to to instantiate and configure this host instance or not, we
>>>>   * parse the properties and set respective generic mmc-host flags and
>>>>   * parameters.
>>>>   */
>>>> -int mmc_of_parse(struct mmc_host *host)
>>>> +int mmc_of_parse(struct mmc_host *host, struct device_node *slot)
>>>>  {
>>>>         struct device_node *np;
>>>>         u32 bus_width;
>>>> @@ -317,7 +319,10 @@ int mmc_of_parse(struct mmc_host *host)
>>>>         if (!host->parent || !host->parent->of_node)
>>>>                 return 0;
>>>>
>>>> -       np = host->parent->of_node;
>>>> +       if (slot)
>>>> +               np = slot;
>>>> +       else
>>>> +               np = host->parent->of_node;
>>>>
>>>>         /* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
>>>>         if (of_property_read_u32(np, "bus-width", &bus_width) < 0) {
>>>> --
>>>> 1.7.9.5
>>>>
>>>
>>> How about adding a new API, __mmc_of_parse((struct mmc_host *host,
>>> struct device_node *slot)
>>> Then let the old API mmc_of_parse() remain as is, but let it call the
>>> new API with slot == NULL.
>>>
>>> Atmel can then use the new API, but the other drivers can remain as is.
>>>
>>> Kind regards
>>> Ulf Hansson
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ludovic Desroches May 26, 2014, 7:49 a.m. UTC | #5
On Mon, May 26, 2014 at 09:03:01AM +0200, Ludovic Desroches wrote:
> Hi,
> 
> Sorry I was on vacation, I'll send a cleaner patch updating other drivers
> this week.

Continuing to clear out my inbox, I have seen you send a patch for it.
Thanks. 

> 
> Regards
> 
> Ludovic
> 
> On Fri, May 23, 2014 at 01:38:06PM +0900, Jaehoon Chung wrote:
> > Hi, All.
> > 
> > This patch is working on progress?
> > I want to merge this patch for fixing dw-mmc controller problem.
> > 
> > If this patch didn't work on progress, i will send the patch based-on this patch.
> > 
> > Best Regards,
> > Jaehoon Chung
> > 
> > On 05/14/2014 06:53 PM, Ulf Hansson wrote:
> > > On 7 May 2014 12:06, Ludovic Desroches <ludovic.desroches@atmel.com> wrote:
> > >> Some hosts manage several slots. In these case information such as the bus
> > >> width, chip detect and others are into the slot node. So we have to parse
> > >> child nodes. If not NULL, slot node will be used instead of the device
> > >> node.
> > >>
> > >> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > >> ---
> > >>
> > >> Hi,
> > >>
> > >> Since this patch is only a RFC, I have not yet updated drivers using this
> > >> function.
> > >>
> > >> I would like to use mmc_of_parse to reduce code duplication. My issue is that
> > >> atmel mci is a bit different from others mci host since it can provide
> > >> several slots, so it allocates several mmc hosts. By the way, it is not the
> > >> only one.
> > >>
> > >> When calling mmc_alloc_host, host->parent is set to &pdev->dev. mmc_of_parse
> > >> uses host->parent->of_node but in my case settings are in the slot nodes so in
> > >> the child nodes. That's why I would like to have a way to tell which node I
> > >> want to use.
> > > 
> > > Seems reasonable, thanks for working on this!
> > > 
> > >>
> > >>
> > >> Regards
> > >>
> > >> Ludovic
> > >>
> > >>
> > >>
> > >>  drivers/mmc/core/host.c |    9 +++++++--
> > >>  1 file changed, 7 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> > >> index fdea825..ed6cea5 100644
> > >> --- a/drivers/mmc/core/host.c
> > >> +++ b/drivers/mmc/core/host.c
> > >> @@ -300,13 +300,15 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
> > >>  /**
> > >>   *     mmc_of_parse() - parse host's device-tree node
> > >>   *     @host: host whose node should be parsed.
> > >> + *     @slot: some devices provide several slots so the node to parse
> > >> + *     is not the host one.
> > >>   *
> > >>   * To keep the rest of the MMC subsystem unaware of whether DT has been
> > >>   * used to to instantiate and configure this host instance or not, we
> > >>   * parse the properties and set respective generic mmc-host flags and
> > >>   * parameters.
> > >>   */
> > >> -int mmc_of_parse(struct mmc_host *host)
> > >> +int mmc_of_parse(struct mmc_host *host, struct device_node *slot)
> > >>  {
> > >>         struct device_node *np;
> > >>         u32 bus_width;
> > >> @@ -317,7 +319,10 @@ int mmc_of_parse(struct mmc_host *host)
> > >>         if (!host->parent || !host->parent->of_node)
> > >>                 return 0;
> > >>
> > >> -       np = host->parent->of_node;
> > >> +       if (slot)
> > >> +               np = slot;
> > >> +       else
> > >> +               np = host->parent->of_node;
> > >>
> > >>         /* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
> > >>         if (of_property_read_u32(np, "bus-width", &bus_width) < 0) {
> > >> --
> > >> 1.7.9.5
> > >>
> > > 
> > > How about adding a new API, __mmc_of_parse((struct mmc_host *host,
> > > struct device_node *slot)
> > > Then let the old API mmc_of_parse() remain as is, but let it call the
> > > new API with slot == NULL.
> > > 
> > > Atmel can then use the new API, but the other drivers can remain as is.
> > > 
> > > Kind regards
> > > Ulf Hansson
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" 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/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index fdea825..ed6cea5 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -300,13 +300,15 @@  static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
 /**
  *	mmc_of_parse() - parse host's device-tree node
  *	@host: host whose node should be parsed.
+ *	@slot: some devices provide several slots so the node to parse
+ *	is not the host one.
  *
  * To keep the rest of the MMC subsystem unaware of whether DT has been
  * used to to instantiate and configure this host instance or not, we
  * parse the properties and set respective generic mmc-host flags and
  * parameters.
  */
-int mmc_of_parse(struct mmc_host *host)
+int mmc_of_parse(struct mmc_host *host, struct device_node *slot)
 {
 	struct device_node *np;
 	u32 bus_width;
@@ -317,7 +319,10 @@  int mmc_of_parse(struct mmc_host *host)
 	if (!host->parent || !host->parent->of_node)
 		return 0;
 
-	np = host->parent->of_node;
+	if (slot)
+		np = slot;
+	else
+		np = host->parent->of_node;
 
 	/* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
 	if (of_property_read_u32(np, "bus-width", &bus_width) < 0) {