* [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
2012-10-12 10:58 [PATCH 0/4] MMC: some omap_hsmmc fixes Daniel Mack
@ 2012-10-12 10:58 ` Daniel Mack
2012-10-12 14:29 ` Daniel Mack
2012-10-12 10:58 ` [PATCH 2/4] MMC: omap_hsmmc: fix DMA config block Daniel Mack
` (3 subsequent siblings)
4 siblings, 1 reply; 22+ messages in thread
From: Daniel Mack @ 2012-10-12 10:58 UTC (permalink / raw)
To: linux-mmc
Cc: Daniel Mack, Venkatraman S, Chris Ball, Grant Likely, Rob Herring,
linux-omap
When probed from DT, the self-allocated platform data has to be attached
to the actual device. Otherwise a NULL pointer will be dereferenced from
omap_hsmmc_card_detect if a gpio handle for card-detect has been passed.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: Venkatraman S <svenkatr@ti.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: linux-omap@vger.kernel.org
---
drivers/mmc/host/omap_hsmmc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 19ccb59..4b70823 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
const u16 *offsetp = match->data;
pdata->reg_offset = *offsetp;
}
+ pdev->dev.platform_data = pdata;
}
if (pdata == NULL) {
--
1.7.11.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
2012-10-12 10:58 ` [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node Daniel Mack
@ 2012-10-12 14:29 ` Daniel Mack
2012-10-12 14:56 ` Balaji T K
0 siblings, 1 reply; 22+ messages in thread
From: Daniel Mack @ 2012-10-12 14:29 UTC (permalink / raw)
To: linux-mmc
Cc: Venkatraman S, Chris Ball, Grant Likely, Rob Herring, linux-omap
On 12.10.2012 12:58, Daniel Mack wrote:
> When probed from DT, the self-allocated platform data has to be attached
> to the actual device. Otherwise a NULL pointer will be dereferenced from
> omap_hsmmc_card_detect if a gpio handle for card-detect has been passed.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> Cc: Venkatraman S <svenkatr@ti.com>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: linux-omap@vger.kernel.org
> ---
> drivers/mmc/host/omap_hsmmc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 19ccb59..4b70823 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
> const u16 *offsetp = match->data;
> pdata->reg_offset = *offsetp;
> }
> + pdev->dev.platform_data = pdata;
> }
>
> if (pdata == NULL) {
>
FWIW, this is the Oops I see without this patch:
[ 1.492727] Unable to handle kernel NULL pointer dereference at
virtual address 00000040
[ 1.501327] pgd = c0004000
[ 1.504166] [00000040] *pgd=00000000
[ 1.507993] Internal error: Oops: 5 [#1] SMP THUMB2
[ 1.513100] Modules linked in:
[ 1.516315] CPU: 0 Not tainted (3.6.0-10656-g4bc7e4d-dirty #75)
[ 1.522890] PC is at omap_hsmmc_card_detect+0x6/0x18
[ 1.528090] LR is at omap_hsmmc_get_cd+0x1d/0x24
[ 1.532929] pc : [<c025a3ee>] lr : [<c0259fed>] psr: 40000133
[ 1.532929] sp : cf1b5e78 ip : 0c288145 fp : 00000002
[ 1.544939] r10: cf1b4000 r9 : cf1b5eb8 r8 : 00000000
[ 1.550408] r7 : cf1a4600 r6 : 60000113 r5 : cf33ba98 r4 : cf33b800
[ 1.557240] r3 : 00000000 r2 : 00000000 r1 : 00000000 r0 : cf0d1c10
[ 1.564075] Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA Thumb
Segment kernel
[ 1.571906] Control: 50c5387d Table: 80004019 DAC: 00000015
[ 1.577921] Process kworker/u:1 (pid: 19, stack limit = 0xcf1b4240)
[ 1.584480] Stack: (0xcf1b5e78 to 0xcf1b6000)
[ 1.589045] 5e60:
c025a3e9 c0259fed
[ 1.597611] 5e80: c0259fd1 c024f959 cf33bb0c cf1a5dc0 c05c87c0
c003a42f 00000002 00000000
[ 1.606176] 5ea0: c003a3d8 c005e181 cf1b35c0 00000000 00000000
c024f829 c0adfe68 c0687048
[ 1.614741] 5ec0: 00000000 c0494f78 c05c8780 cf1a5dc0 c05c88e8
cf1b4000 c05c88f0 cf1a5dd0
[ 1.623306] 5ee0: c0531f40 c053d678 c05c8780 c003c2ab cf1b5f18
c005e935 c05c87c0 c0531f40
[ 1.631872] 5f00: c0531f40 c7e3f1f9 cf1b5f34 cf069e20 cf1b5f34
00000000 cf1a5dc0 c003c191
[ 1.640436] 5f20: 00000000 00000000 00000000 c003f809 00000000
00000000 00000000 cf1a5dc0
[ 1.649001] 5f40: 00000000 00000000 dead4ead ffffffff ffffffff
c05c8ef8 00000000 00000000
[ 1.657565] 5f60: c0455c08 cf1b5f64 cf1b5f64 00000000 00000000
dead4ead ffffffff ffffffff
[ 1.666130] 5f80: c05c8ef8 00000000 00000000 c0455c08 cf1b5f90
cf1b5f90 cf069e20 c003f791
[ 1.674694] 5fa0: 00000000 00000000 00000000 c000d1e1 aaa3aaab
baaaa1ba aae3ea32 aba2a3aa
[ 1.683260] 5fc0: baaababe bbbaaaba aa2baaaa bb2e2aba aabaa2bb
aaabb2bb aaabbaf3 f3bb2aaa
[ 1.691824] 5fe0: aabaabba 23ae2beb a2afab2a b2a2ab2a 00000013
aaabaaab aaaabfaf a0bbbba2
[ 1.700399] [<c025a3ee>] (omap_hsmmc_card_detect+0x6/0x18) from
[<c0259fed>] (omap_hsmmc_get_cd+0x1d/0x24)
[ 1.710528] [<c0259fed>] (omap_hsmmc_get_cd+0x1d/0x24) from
[<c024f959>] (mmc_rescan+0x131/0x1e4)
[ 1.719842] [<c024f959>] (mmc_rescan+0x131/0x1e4) from [<c003a42f>]
(process_one_work+0x137/0x37c)
[ 1.729234] [<c003a42f>] (process_one_work+0x137/0x37c) from
[<c003c2ab>] (worker_thread+0x11b/0x364)
[ 1.738899] [<c003c2ab>] (worker_thread+0x11b/0x364) from
[<c003f809>] (kthread+0x79/0x84)
[ 1.747572] [<c003f809>] (kthread+0x79/0x84) from [<c000d1e1>]
(ret_from_kernel_thread+0xd/0x14)
[ 1.756773] Code: bf00 b508 f8d0 3084 (6c18) f75f
[ 1.761943] ---[ end trace b94c447c4835422d ]---
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
2012-10-12 14:29 ` Daniel Mack
@ 2012-10-12 14:56 ` Balaji T K
2012-10-12 15:14 ` Daniel Mack
0 siblings, 1 reply; 22+ messages in thread
From: Balaji T K @ 2012-10-12 14:56 UTC (permalink / raw)
To: Daniel Mack
Cc: linux-mmc, Venkatraman S, Chris Ball, Grant Likely, Rob Herring,
linux-omap
On Friday 12 October 2012 07:59 PM, Daniel Mack wrote:
> On 12.10.2012 12:58, Daniel Mack wrote:
>> When probed from DT, the self-allocated platform data has to be attached
>> to the actual device. Otherwise a NULL pointer will be dereferenced from
>> omap_hsmmc_card_detect if a gpio handle for card-detect has been passed.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> Cc: Venkatraman S <svenkatr@ti.com>
>> Cc: Chris Ball <cjb@laptop.org>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Cc: Rob Herring <rob.herring@calxeda.com>
>> Cc: linux-omap@vger.kernel.org
>> ---
>> drivers/mmc/host/omap_hsmmc.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>> index 19ccb59..4b70823 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
>> const u16 *offsetp = match->data;
>> pdata->reg_offset = *offsetp;
>> }
>> + pdev->dev.platform_data = pdata;
>> }
>>
>> if (pdata == NULL) {
>>
>
> FWIW, this is the Oops I see without this patch:
Hi,
Shouldn't pdev->dev.platform_data be set to NULL on _remove ?
BTW, I posted a patch for the same by accessing saved version from
host->pdata
http://permalink.gmane.org/gmane.linux.kernel.mmc/16996
>
> [ 1.492727] Unable to handle kernel NULL pointer dereference at
> virtual address 00000040
> [ 1.501327] pgd = c0004000
> [ 1.504166] [00000040] *pgd=00000000
> [ 1.507993] Internal error: Oops: 5 [#1] SMP THUMB2
> [ 1.513100] Modules linked in:
> [ 1.516315] CPU: 0 Not tainted (3.6.0-10656-g4bc7e4d-dirty #75)
> [ 1.522890] PC is at omap_hsmmc_card_detect+0x6/0x18
> [ 1.528090] LR is at omap_hsmmc_get_cd+0x1d/0x24
> [ 1.532929] pc : [<c025a3ee>] lr : [<c0259fed>] psr: 40000133
> [ 1.532929] sp : cf1b5e78 ip : 0c288145 fp : 00000002
> [ 1.544939] r10: cf1b4000 r9 : cf1b5eb8 r8 : 00000000
> [ 1.550408] r7 : cf1a4600 r6 : 60000113 r5 : cf33ba98 r4 : cf33b800
> [ 1.557240] r3 : 00000000 r2 : 00000000 r1 : 00000000 r0 : cf0d1c10
> [ 1.564075] Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA Thumb
> Segment kernel
> [ 1.571906] Control: 50c5387d Table: 80004019 DAC: 00000015
> [ 1.577921] Process kworker/u:1 (pid: 19, stack limit = 0xcf1b4240)
> [ 1.584480] Stack: (0xcf1b5e78 to 0xcf1b6000)
> [ 1.589045] 5e60:
> c025a3e9 c0259fed
> [ 1.597611] 5e80: c0259fd1 c024f959 cf33bb0c cf1a5dc0 c05c87c0
> c003a42f 00000002 00000000
> [ 1.606176] 5ea0: c003a3d8 c005e181 cf1b35c0 00000000 00000000
> c024f829 c0adfe68 c0687048
> [ 1.614741] 5ec0: 00000000 c0494f78 c05c8780 cf1a5dc0 c05c88e8
> cf1b4000 c05c88f0 cf1a5dd0
> [ 1.623306] 5ee0: c0531f40 c053d678 c05c8780 c003c2ab cf1b5f18
> c005e935 c05c87c0 c0531f40
> [ 1.631872] 5f00: c0531f40 c7e3f1f9 cf1b5f34 cf069e20 cf1b5f34
> 00000000 cf1a5dc0 c003c191
> [ 1.640436] 5f20: 00000000 00000000 00000000 c003f809 00000000
> 00000000 00000000 cf1a5dc0
> [ 1.649001] 5f40: 00000000 00000000 dead4ead ffffffff ffffffff
> c05c8ef8 00000000 00000000
> [ 1.657565] 5f60: c0455c08 cf1b5f64 cf1b5f64 00000000 00000000
> dead4ead ffffffff ffffffff
> [ 1.666130] 5f80: c05c8ef8 00000000 00000000 c0455c08 cf1b5f90
> cf1b5f90 cf069e20 c003f791
> [ 1.674694] 5fa0: 00000000 00000000 00000000 c000d1e1 aaa3aaab
> baaaa1ba aae3ea32 aba2a3aa
> [ 1.683260] 5fc0: baaababe bbbaaaba aa2baaaa bb2e2aba aabaa2bb
> aaabb2bb aaabbaf3 f3bb2aaa
> [ 1.691824] 5fe0: aabaabba 23ae2beb a2afab2a b2a2ab2a 00000013
> aaabaaab aaaabfaf a0bbbba2
> [ 1.700399] [<c025a3ee>] (omap_hsmmc_card_detect+0x6/0x18) from
> [<c0259fed>] (omap_hsmmc_get_cd+0x1d/0x24)
> [ 1.710528] [<c0259fed>] (omap_hsmmc_get_cd+0x1d/0x24) from
> [<c024f959>] (mmc_rescan+0x131/0x1e4)
> [ 1.719842] [<c024f959>] (mmc_rescan+0x131/0x1e4) from [<c003a42f>]
> (process_one_work+0x137/0x37c)
> [ 1.729234] [<c003a42f>] (process_one_work+0x137/0x37c) from
> [<c003c2ab>] (worker_thread+0x11b/0x364)
> [ 1.738899] [<c003c2ab>] (worker_thread+0x11b/0x364) from
> [<c003f809>] (kthread+0x79/0x84)
> [ 1.747572] [<c003f809>] (kthread+0x79/0x84) from [<c000d1e1>]
> (ret_from_kernel_thread+0xd/0x14)
> [ 1.756773] Code: bf00 b508 f8d0 3084 (6c18) f75f
> [ 1.761943] ---[ end trace b94c447c4835422d ]---
>
> --
> 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
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
2012-10-12 14:56 ` Balaji T K
@ 2012-10-12 15:14 ` Daniel Mack
2012-10-12 15:23 ` Balaji T K
0 siblings, 1 reply; 22+ messages in thread
From: Daniel Mack @ 2012-10-12 15:14 UTC (permalink / raw)
To: Balaji T K
Cc: linux-mmc, Venkatraman S, Chris Ball, Grant Likely, Rob Herring,
linux-omap
On 12.10.2012 16:56, Balaji T K wrote:
> On Friday 12 October 2012 07:59 PM, Daniel Mack wrote:
>> On 12.10.2012 12:58, Daniel Mack wrote:
>>> When probed from DT, the self-allocated platform data has to be attached
>>> to the actual device. Otherwise a NULL pointer will be dereferenced from
>>> omap_hsmmc_card_detect if a gpio handle for card-detect has been passed.
>>>
>>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>>> Cc: Venkatraman S <svenkatr@ti.com>
>>> Cc: Chris Ball <cjb@laptop.org>
>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>> Cc: Rob Herring <rob.herring@calxeda.com>
>>> Cc: linux-omap@vger.kernel.org
>>> ---
>>> drivers/mmc/host/omap_hsmmc.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>>> index 19ccb59..4b70823 100644
>>> --- a/drivers/mmc/host/omap_hsmmc.c
>>> +++ b/drivers/mmc/host/omap_hsmmc.c
>>> @@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
>>> const u16 *offsetp = match->data;
>>> pdata->reg_offset = *offsetp;
>>> }
>>> + pdev->dev.platform_data = pdata;
>>> }
>>>
>>> if (pdata == NULL) {
>>>
>>
>> FWIW, this is the Oops I see without this patch:
> Hi,
> Shouldn't pdev->dev.platform_data be set to NULL on _remove ?
Why?
> BTW, I posted a patch for the same by accessing saved version from
> host->pdata
> http://permalink.gmane.org/gmane.linux.kernel.mmc/16996
Ok, that's another solution. I thought about this too, but then chose
the easier way :) I don't care which patch is taken, as long as we have
a fix in mainline.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
2012-10-12 15:14 ` Daniel Mack
@ 2012-10-12 15:23 ` Balaji T K
2012-10-13 0:05 ` Grant Likely
0 siblings, 1 reply; 22+ messages in thread
From: Balaji T K @ 2012-10-12 15:23 UTC (permalink / raw)
To: Daniel Mack
Cc: linux-mmc, Venkatraman S, Chris Ball, Grant Likely, Rob Herring,
linux-omap
On Friday 12 October 2012 08:44 PM, Daniel Mack wrote:
> On 12.10.2012 16:56, Balaji T K wrote:
>> On Friday 12 October 2012 07:59 PM, Daniel Mack wrote:
>>> On 12.10.2012 12:58, Daniel Mack wrote:
>>>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>>>> index 19ccb59..4b70823 100644
>>>> --- a/drivers/mmc/host/omap_hsmmc.c
>>>> +++ b/drivers/mmc/host/omap_hsmmc.c
>>>> @@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
>>>> const u16 *offsetp = match->data;
>>>> pdata->reg_offset = *offsetp;
>>>> }
>>>> + pdev->dev.platform_data = pdata;
>>>> }
>>>>
>>>> if (pdata == NULL) {
>>>>
>>>
>>> FWIW, this is the Oops I see without this patch:
>> Hi,
>> Shouldn't pdev->dev.platform_data be set to NULL on _remove ?
>
> Why?
To make sure on second insmod it is NULL, When built as module,
So that of_get_hsmmc_pdata is called to create pdata.
>
>> BTW, I posted a patch for the same by accessing saved version from
>> host->pdata
>> http://permalink.gmane.org/gmane.linux.kernel.mmc/16996
>
> Ok, that's another solution. I thought about this too, but then chose
> the easier way :) I don't care which patch is taken, as long as we have
> a fix in mainline.
>
Agree this patch is easiest :-)
>
> Thanks,
> Daniel
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
2012-10-12 15:23 ` Balaji T K
@ 2012-10-13 0:05 ` Grant Likely
2012-10-13 8:05 ` Daniel Mack
0 siblings, 1 reply; 22+ messages in thread
From: Grant Likely @ 2012-10-13 0:05 UTC (permalink / raw)
To: Balaji T K, Daniel Mack
Cc: linux-mmc, Venkatraman S, Chris Ball, Rob Herring, linux-omap
Balaji T K <balajitk@ti.com> wrote:
>On Friday 12 October 2012 08:44 PM, Daniel Mack wrote:
>> On 12.10.2012 16:56, Balaji T K wrote:
>>> On Friday 12 October 2012 07:59 PM, Daniel Mack wrote:
>>>> On 12.10.2012 12:58, Daniel Mack wrote:
>>>>> diff --git a/drivers/mmc/host/omap_hsmmc.c
>b/drivers/mmc/host/omap_hsmmc.c
>>>>> index 19ccb59..4b70823 100644
>>>>> --- a/drivers/mmc/host/omap_hsmmc.c
>>>>> +++ b/drivers/mmc/host/omap_hsmmc.c
>>>>> @@ -1728,6 +1728,7 @@ static int __devinit omap_hsmmc_probe(struct
>platform_device *pdev)
>>>>> const u16 *offsetp = match->data;
>>>>> pdata->reg_offset = *offsetp;
>>>>> }
>>>>> + pdev->dev.platform_data = pdata;
>>>>> }
>>>>>
>>>>> if (pdata == NULL) {
>>>>>
>>>>
>>>> FWIW, this is the Oops I see without this patch:
>>> Hi,
>>> Shouldn't pdev->dev.platform_data be set to NULL on _remove ?
>>
>> Why?
>
>To make sure on second insmod it is NULL, When built as module,
>So that of_get_hsmmc_pdata is called to create pdata.
Actually the driver should *never* modify the value of dev->platform_data. Ever. Make a copy instead.
g.
--
Grant Likely, P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
2012-10-13 0:05 ` Grant Likely
@ 2012-10-13 8:05 ` Daniel Mack
2012-10-13 8:48 ` Grant Likely
0 siblings, 1 reply; 22+ messages in thread
From: Daniel Mack @ 2012-10-13 8:05 UTC (permalink / raw)
To: Grant Likely
Cc: Balaji T K, linux-mmc, Venkatraman S, Chris Ball, Rob Herring,
linux-omap
Hi Grant,
On 13.10.2012 02:05, Grant Likely wrote:
>
>
> Balaji T K <balajitk@ti.com> wrote:
>
>> On Friday 12 October 2012 08:44 PM, Daniel Mack wrote:
>>> On 12.10.2012 16:56, Balaji T K wrote:
>>>> On Friday 12 October 2012 07:59 PM, Daniel Mack wrote:
>>>>> On 12.10.2012 12:58, Daniel Mack wrote:
>>>>>> diff --git a/drivers/mmc/host/omap_hsmmc.c
>> b/drivers/mmc/host/omap_hsmmc.c
>>>>>> index 19ccb59..4b70823 100644 ---
>>>>>> a/drivers/mmc/host/omap_hsmmc.c +++
>>>>>> b/drivers/mmc/host/omap_hsmmc.c @@ -1728,6 +1728,7 @@
>>>>>> static int __devinit omap_hsmmc_probe(struct
>> platform_device *pdev)
>>>>>> const u16 *offsetp = match->data; pdata->reg_offset =
>>>>>> *offsetp; } + pdev->dev.platform_data = pdata; }
>>>>>>
>>>>>> if (pdata == NULL) {
>>>>>>
>>>>>
>>>>> FWIW, this is the Oops I see without this patch:
>>>> Hi, Shouldn't pdev->dev.platform_data be set to NULL on _remove
>>>> ?
>>>
>>> Why?
>>
>> To make sure on second insmod it is NULL, When built as module, So
>> that of_get_hsmmc_pdata is called to create pdata.
>
> Actually the driver should *never* modify the value of
> dev->platform_data. Ever.
That's interesting, because many drivers do this, especially since they
were converted to DT probing. Mostly because that way, the platform data
logic in callback functions can be reused, and often the platform
specific data is only stored in pdata and taken from there during the
lifetime of a device.
Is there any particular reason why this approach is frowned upon?
> Make a copy instead.
A copy of what exactly? Of all members of the legacy pdata you mean?
Thanks,
Daniel
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
2012-10-13 8:05 ` Daniel Mack
@ 2012-10-13 8:48 ` Grant Likely
2012-10-13 8:53 ` Daniel Mack
0 siblings, 1 reply; 22+ messages in thread
From: Grant Likely @ 2012-10-13 8:48 UTC (permalink / raw)
To: Daniel Mack
Cc: Balaji T K, linux-mmc, Venkatraman S, Chris Ball, Rob Herring,
linux-omap, Linux Kernel Mailing List
On Sat, Oct 13, 2012 at 9:05 AM, Daniel Mack <zonque@gmail.com> wrote:
> Hi Grant,
>
> On 13.10.2012 02:05, Grant Likely wrote:
>>
>>
>> Balaji T K <balajitk@ti.com> wrote:
>>
>>> On Friday 12 October 2012 08:44 PM, Daniel Mack wrote:
>>>> On 12.10.2012 16:56, Balaji T K wrote:
>>>>> On Friday 12 October 2012 07:59 PM, Daniel Mack wrote:
>>>>>> On 12.10.2012 12:58, Daniel Mack wrote:
>>>>>>> diff --git a/drivers/mmc/host/omap_hsmmc.c
>>> b/drivers/mmc/host/omap_hsmmc.c
>>>>>>> index 19ccb59..4b70823 100644 ---
>>>>>>> a/drivers/mmc/host/omap_hsmmc.c +++
>>>>>>> b/drivers/mmc/host/omap_hsmmc.c @@ -1728,6 +1728,7 @@
>>>>>>> static int __devinit omap_hsmmc_probe(struct
>>> platform_device *pdev)
>>>>>>> const u16 *offsetp = match->data; pdata->reg_offset =
>>>>>>> *offsetp; } + pdev->dev.platform_data = pdata; }
>>>>>>>
>>>>>>> if (pdata == NULL) {
>>>>>>>
>>>>>>
>>>>>> FWIW, this is the Oops I see without this patch:
>>>>> Hi, Shouldn't pdev->dev.platform_data be set to NULL on _remove
>>>>> ?
>>>>
>>>> Why?
>>>
>>> To make sure on second insmod it is NULL, When built as module, So
>>> that of_get_hsmmc_pdata is called to create pdata.
>>
>> Actually the driver should *never* modify the value of
>> dev->platform_data. Ever.
>
> That's interesting, because many drivers do this, especially since they
> were converted to DT probing. Mostly because that way, the platform data
> logic in callback functions can be reused, and often the platform
> specific data is only stored in pdata and taken from there during the
> lifetime of a device.
>
> Is there any particular reason why this approach is frowned upon?
Yes. The platform data pointer is "owned" by the code that registered
the platform device, not by the device driver. Some drivers do it, but
it is definitely illegal. I should add code to the platform bus core
code to throw a warning to any drivers that do that. It is a problem
because it becomes easy to mess up the lifetime model of device data,
particularly when it comes to unbinding/rebinding devices. For
example, if a driver changes the pdata pointer and then gets unbound,
then there will be a stale pdata pointer that may point to either
incorrect data or a data structure that is no longer allocated. You
could argue that it is fine if the driver is smart about how it cleans
up after itself, but in my experience driver authors rarely get it
correct and it results in a lot more code than is necessary. It is far
better for the driver to either grab all the data it needs out of
pdata at .probe() time, or to keep a copy of the 'correct' pdata
(correct depending on where the device retrieved it's data) in it's
private data structure instead of modifying the device.
>
>> Make a copy instead.
>
> A copy of what exactly? Of all members of the legacy pdata you mean?
Yes. If the driver directly accesses the pdata structure outside of
the .probe() hook, then it should be modified to either copy the pdata
into the driver's private data structure, or it should copy the
pointer to the pdata so that OF usage can allocate one itself. For
example:
somedriver_probe(struct platform_device *pdev)
{
struct somedriver_private *somedriver;
somedriver = devm_kzalloc(sizeof (*somedriver), GFP_KERNEL);
somedriver->pdata = pdev->platform_data;
if (OF)
somedriver->pdata = devm_kzalloc(sizeof
(*somedriver->pdata), GFP_KERNEL);
}
The bonus with using devm_kzalloc is the driver doesn't even need to
do anything special to undo these allocations on failure or release.
:-)
g.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
2012-10-13 8:48 ` Grant Likely
@ 2012-10-13 8:53 ` Daniel Mack
2012-10-13 12:37 ` Grant Likely
0 siblings, 1 reply; 22+ messages in thread
From: Daniel Mack @ 2012-10-13 8:53 UTC (permalink / raw)
To: Grant Likely
Cc: Balaji T K, linux-mmc, Venkatraman S, Chris Ball, Rob Herring,
linux-omap, Linux Kernel Mailing List
On 13.10.2012 10:48, Grant Likely wrote:
> On Sat, Oct 13, 2012 at 9:05 AM, Daniel Mack <zonque@gmail.com> wrote:
>> Hi Grant,
>>
>> On 13.10.2012 02:05, Grant Likely wrote:
>>>
>>>
>>> Balaji T K <balajitk@ti.com> wrote:
>>>
>>>> On Friday 12 October 2012 08:44 PM, Daniel Mack wrote:
>>>>> On 12.10.2012 16:56, Balaji T K wrote:
>>>>>> On Friday 12 October 2012 07:59 PM, Daniel Mack wrote:
>>>>>>> On 12.10.2012 12:58, Daniel Mack wrote:
>>>>>>>> diff --git a/drivers/mmc/host/omap_hsmmc.c
>>>> b/drivers/mmc/host/omap_hsmmc.c
>>>>>>>> index 19ccb59..4b70823 100644 ---
>>>>>>>> a/drivers/mmc/host/omap_hsmmc.c +++
>>>>>>>> b/drivers/mmc/host/omap_hsmmc.c @@ -1728,6 +1728,7 @@
>>>>>>>> static int __devinit omap_hsmmc_probe(struct
>>>> platform_device *pdev)
>>>>>>>> const u16 *offsetp = match->data; pdata->reg_offset =
>>>>>>>> *offsetp; } + pdev->dev.platform_data = pdata; }
>>>>>>>>
>>>>>>>> if (pdata == NULL) {
>>>>>>>>
>>>>>>>
>>>>>>> FWIW, this is the Oops I see without this patch:
>>>>>> Hi, Shouldn't pdev->dev.platform_data be set to NULL on _remove
>>>>>> ?
>>>>>
>>>>> Why?
>>>>
>>>> To make sure on second insmod it is NULL, When built as module, So
>>>> that of_get_hsmmc_pdata is called to create pdata.
>>>
>>> Actually the driver should *never* modify the value of
>>> dev->platform_data. Ever.
>>
>> That's interesting, because many drivers do this, especially since they
>> were converted to DT probing. Mostly because that way, the platform data
>> logic in callback functions can be reused, and often the platform
>> specific data is only stored in pdata and taken from there during the
>> lifetime of a device.
>>
>> Is there any particular reason why this approach is frowned upon?
>
> Yes. The platform data pointer is "owned" by the code that registered
> the platform device, not by the device driver. Some drivers do it, but
> it is definitely illegal. I should add code to the platform bus core
> code to throw a warning to any drivers that do that. It is a problem
> because it becomes easy to mess up the lifetime model of device data,
> particularly when it comes to unbinding/rebinding devices. For
> example, if a driver changes the pdata pointer and then gets unbound,
> then there will be a stale pdata pointer that may point to either
> incorrect data or a data structure that is no longer allocated. You
> could argue that it is fine if the driver is smart about how it cleans
> up after itself, but in my experience driver authors rarely get it
> correct and it results in a lot more code than is necessary. It is far
> better for the driver to either grab all the data it needs out of
> pdata at .probe() time, or to keep a copy of the 'correct' pdata
> (correct depending on where the device retrieved it's data) in it's
> private data structure instead of modifying the device.
Ok, interesting. Thanks for writing this up.
>>> Make a copy instead.
>>
>> A copy of what exactly? Of all members of the legacy pdata you mean?
>
> Yes. If the driver directly accesses the pdata structure outside of
> the .probe() hook, then it should be modified to either copy the pdata
> into the driver's private data structure, or it should copy the
> pointer to the pdata so that OF usage can allocate one itself. For
> example:
>
> somedriver_probe(struct platform_device *pdev)
> {
> struct somedriver_private *somedriver;
>
> somedriver = devm_kzalloc(sizeof (*somedriver), GFP_KERNEL);
> somedriver->pdata = pdev->platform_data;
> if (OF)
> somedriver->pdata = devm_kzalloc(sizeof
> (*somedriver->pdata), GFP_KERNEL);
> }
>
> The bonus with using devm_kzalloc is the driver doesn't even need to
> do anything special to undo these allocations on failure or release.
> :-)
Ok, understood. Will keep an eye on this in the future. Thanks again for
the explanation.
For this particular driver, this means that both my and Balaji's ways of
fixing this are wrong?
Daniel
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node
2012-10-13 8:53 ` Daniel Mack
@ 2012-10-13 12:37 ` Grant Likely
0 siblings, 0 replies; 22+ messages in thread
From: Grant Likely @ 2012-10-13 12:37 UTC (permalink / raw)
To: Daniel Mack
Cc: Balaji T K, linux-mmc, Venkatraman S, Chris Ball, Rob Herring,
linux-omap, Linux Kernel Mailing List
Daniel Mack <zonque@gmail.com> wrote:
>On 13.10.2012 10:48, Grant Likely wrote:
>>
>> somedriver_probe(struct platform_device *pdev)
>> {
>> struct somedriver_private *somedriver;
>>
>> somedriver = devm_kzalloc(sizeof (*somedriver), GFP_KERNEL);
>> somedriver->pdata = pdev->platform_data;
>> if (OF)
>> somedriver->pdata = devm_kzalloc(sizeof
>> (*somedriver->pdata), GFP_KERNEL);
>> }
>>
>> The bonus with using devm_kzalloc is the driver doesn't even need to
>> do anything special to undo these allocations on failure or release.
>> :-)
>
>Ok, understood. Will keep an eye on this in the future. Thanks again
>for
>the explanation.
>
>For this particular driver, this means that both my and Balaji's ways
>of
>fixing this are wrong?
Balaji's patch looks fine since it uses a local copy of the platform data structure. It appears that the driver is already creating a local copy and Balaji is just bug fixing call sites that aren't using it yet.
g.
--
Grant Likely, P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/4] MMC: omap_hsmmc: fix DMA config block
2012-10-12 10:58 [PATCH 0/4] MMC: some omap_hsmmc fixes Daniel Mack
2012-10-12 10:58 ` [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node Daniel Mack
@ 2012-10-12 10:58 ` Daniel Mack
2012-10-12 11:07 ` Porter, Matt
2012-10-12 11:12 ` Russell King - ARM Linux
2012-10-12 10:58 ` [PATCH 3/4] MMC: omap_hsmmc: claim pinctrl at probe time Daniel Mack
` (2 subsequent siblings)
4 siblings, 2 replies; 22+ messages in thread
From: Daniel Mack @ 2012-10-12 10:58 UTC (permalink / raw)
To: linux-mmc
Cc: Daniel Mack, Venkatraman S, Chris Ball, Grant Likely, Rob Herring,
linux-omap, Matt Porter, Russell King
The .direction field of the config passed to dmaengine_slave_config()
must be set in order to make the generic code store the configured bus
width.
Without this patch, I keep getting the following error on a AM33xx
board:
[ 1.963144] edma-dma-engine edma-dma-engine.0: Undefined slave buswidth
[ 1.970158] omap_hsmmc mmc.4: prep_slave_sg() failed
[ 1.975454] mmc1: error -1 whilst initialising SD card
Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: Venkatraman S <svenkatr@ti.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: linux-omap@vger.kernel.org
Cc: Matt Porter <mporter@ti.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
---
drivers/mmc/host/omap_hsmmc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 4b70823..2c57b0d 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1260,6 +1260,8 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
cfg.src_maxburst = data->blksz / 4;
cfg.dst_maxburst = data->blksz / 4;
+ cfg.direction = data->flags & MMC_DATA_WRITE ?
+ DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
ret = dmaengine_slave_config(chan, &cfg);
if (ret)
@@ -1270,8 +1272,8 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
return ret;
tx = dmaengine_prep_slave_sg(chan, data->sg, data->sg_len,
- data->flags & MMC_DATA_WRITE ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM,
- DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ cfg.direction,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!tx) {
dev_err(mmc_dev(host->mmc), "prep_slave_sg() failed\n");
/* FIXME: cleanup */
--
1.7.11.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 2/4] MMC: omap_hsmmc: fix DMA config block
2012-10-12 10:58 ` [PATCH 2/4] MMC: omap_hsmmc: fix DMA config block Daniel Mack
@ 2012-10-12 11:07 ` Porter, Matt
2012-10-12 11:12 ` Russell King - ARM Linux
1 sibling, 0 replies; 22+ messages in thread
From: Porter, Matt @ 2012-10-12 11:07 UTC (permalink / raw)
To: Daniel Mack
Cc: <linux-mmc@vger.kernel.org>, S, Venkatraman, Chris Ball,
Grant Likely, Rob Herring, <linux-omap@vger.kernel.org>,
Russell King
On Oct 12, 2012, at 6:58 AM, Daniel Mack wrote:
> The .direction field of the config passed to dmaengine_slave_config()
> must be set in order to make the generic code store the configured bus
> width.
>
> Without this patch, I keep getting the following error on a AM33xx
> board:
>
> [ 1.963144] edma-dma-engine edma-dma-engine.0: Undefined slave buswidth
> [ 1.970158] omap_hsmmc mmc.4: prep_slave_sg() failed
> [ 1.975454] mmc1: error -1 whilst initialising SD card
Hi Daniel,
I posted a patch to fix the slave config handling in the EDMA DMA Engine
driver here: https://patchwork.kernel.org/patch/1474411/
Vinod didn't pick that up for 3.7, probably because it's not triggered by
any of the in-tree davinci drivers that are converted. However, I am
carrying the same patch in the am335x edma dmaengine v2 series
(https://lkml.org/lkml/2012/10/11/345) so there's no need to change this
driver.
-Matt
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> Cc: Venkatraman S <svenkatr@ti.com>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: linux-omap@vger.kernel.org
> Cc: Matt Porter <mporter@ti.com>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> drivers/mmc/host/omap_hsmmc.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 4b70823..2c57b0d 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1260,6 +1260,8 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
> cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> cfg.src_maxburst = data->blksz / 4;
> cfg.dst_maxburst = data->blksz / 4;
> + cfg.direction = data->flags & MMC_DATA_WRITE ?
> + DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
>
> ret = dmaengine_slave_config(chan, &cfg);
> if (ret)
> @@ -1270,8 +1272,8 @@ static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
> return ret;
>
> tx = dmaengine_prep_slave_sg(chan, data->sg, data->sg_len,
> - data->flags & MMC_DATA_WRITE ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM,
> - DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> + cfg.direction,
> + DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> if (!tx) {
> dev_err(mmc_dev(host->mmc), "prep_slave_sg() failed\n");
> /* FIXME: cleanup */
> --
> 1.7.11.4
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/4] MMC: omap_hsmmc: fix DMA config block
2012-10-12 10:58 ` [PATCH 2/4] MMC: omap_hsmmc: fix DMA config block Daniel Mack
2012-10-12 11:07 ` Porter, Matt
@ 2012-10-12 11:12 ` Russell King - ARM Linux
2012-10-12 14:19 ` Daniel Mack
1 sibling, 1 reply; 22+ messages in thread
From: Russell King - ARM Linux @ 2012-10-12 11:12 UTC (permalink / raw)
To: Daniel Mack
Cc: linux-mmc, Venkatraman S, Chris Ball, Grant Likely, Rob Herring,
linux-omap, Matt Porter
On Fri, Oct 12, 2012 at 12:58:13PM +0200, Daniel Mack wrote:
> The .direction field of the config passed to dmaengine_slave_config()
> must be set in order to make the generic code store the configured bus
> width.
NAK. The direction field is deprecated, and should not be used anymore.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] MMC: omap_hsmmc: fix DMA config block
2012-10-12 11:12 ` Russell King - ARM Linux
@ 2012-10-12 14:19 ` Daniel Mack
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Mack @ 2012-10-12 14:19 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-mmc, Venkatraman S, Chris Ball, Grant Likely, Rob Herring,
linux-omap, Matt Porter
On 12.10.2012 13:12, Russell King - ARM Linux wrote:
> On Fri, Oct 12, 2012 at 12:58:13PM +0200, Daniel Mack wrote:
>> The .direction field of the config passed to dmaengine_slave_config()
>> must be set in order to make the generic code store the configured bus
>> width.
>
> NAK. The direction field is deprecated, and should not be used anymore.
>
Yes, thanks for the heads-up. Matt already pointed me to an updated
series of his EDMA patches, so this patch can be dropped.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/4] MMC: omap_hsmmc: claim pinctrl at probe time
2012-10-12 10:58 [PATCH 0/4] MMC: some omap_hsmmc fixes Daniel Mack
2012-10-12 10:58 ` [PATCH 1/4] MMC: omap_hsmmc: set platform data after probe from DT node Daniel Mack
2012-10-12 10:58 ` [PATCH 2/4] MMC: omap_hsmmc: fix DMA config block Daniel Mack
@ 2012-10-12 10:58 ` Daniel Mack
2012-10-12 11:08 ` Linus Walleij
2012-10-12 10:58 ` [PATCH 4/4] MMC: omap_hsmmc: add DT property for max bus frequency Daniel Mack
2012-10-13 9:01 ` [PATCH 0/4] MMC: some omap_hsmmc fixes Daniel Mack
4 siblings, 1 reply; 22+ messages in thread
From: Daniel Mack @ 2012-10-12 10:58 UTC (permalink / raw)
To: linux-mmc
Cc: Daniel Mack, Venkatraman S, Chris Ball, Grant Likely, Rob Herring,
Linus Walleij, linux-omap
This allows DT-driven board to set up the pin mux only when the driver
is in use.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: Venkatraman S <svenkatr@ti.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-omap@vger.kernel.org
---
drivers/mmc/host/omap_hsmmc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 2c57b0d..86f0759 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -37,6 +37,7 @@
#include <linux/io.h>
#include <linux/gpio.h>
#include <linux/regulator/consumer.h>
+#include <linux/pinctrl/consumer.h>
#include <linux/pm_runtime.h>
#include <mach/hardware.h>
#include <plat/mmc.h>
@@ -1722,6 +1723,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
const struct of_device_id *match;
dma_cap_mask_t mask;
unsigned tx_req, rx_req;
+ struct pinctrl *pinctrl;
match = of_match_device(of_match_ptr(omap_mmc_of_match), &pdev->dev);
if (match) {
@@ -1948,6 +1950,11 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
omap_hsmmc_disable_irq(host);
+ pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+ if (IS_ERR(pinctrl))
+ dev_warn(&pdev->dev,
+ "pins are not configured from the driver\n");
+
omap_hsmmc_protect_card(host);
mmc_add_host(mmc);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 3/4] MMC: omap_hsmmc: claim pinctrl at probe time
2012-10-12 10:58 ` [PATCH 3/4] MMC: omap_hsmmc: claim pinctrl at probe time Daniel Mack
@ 2012-10-12 11:08 ` Linus Walleij
0 siblings, 0 replies; 22+ messages in thread
From: Linus Walleij @ 2012-10-12 11:08 UTC (permalink / raw)
To: Daniel Mack
Cc: linux-mmc, Venkatraman S, Chris Ball, Grant Likely, Rob Herring,
linux-omap
On Fri, Oct 12, 2012 at 12:58 PM, Daniel Mack <zonque@gmail.com> wrote:
> This allows DT-driven board to set up the pin mux only when the driver
> is in use.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> Cc: Venkatraman S <svenkatr@ti.com>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: linux-omap@vger.kernel.org
Looks nice!
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/4] MMC: omap_hsmmc: add DT property for max bus frequency
2012-10-12 10:58 [PATCH 0/4] MMC: some omap_hsmmc fixes Daniel Mack
` (2 preceding siblings ...)
2012-10-12 10:58 ` [PATCH 3/4] MMC: omap_hsmmc: claim pinctrl at probe time Daniel Mack
@ 2012-10-12 10:58 ` Daniel Mack
2012-10-12 15:25 ` Rob Herring
2012-10-13 9:01 ` [PATCH 0/4] MMC: some omap_hsmmc fixes Daniel Mack
4 siblings, 1 reply; 22+ messages in thread
From: Daniel Mack @ 2012-10-12 10:58 UTC (permalink / raw)
To: linux-mmc
Cc: Daniel Mack, Venkatraman S, Chris Ball, Grant Likely, Rob Herring,
linux-omap
Maximum bus frequency can be limited by external circuitry like level
shifters etc. Allow passing this value from DT.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: Venkatraman S <svenkatr@ti.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: linux-omap@vger.kernel.org
---
drivers/mmc/host/omap_hsmmc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 86f0759..4650ef7 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1676,7 +1676,7 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
{
struct omap_mmc_platform_data *pdata;
struct device_node *np = dev->of_node;
- u32 bus_width;
+ u32 bus_width, max_freq;
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
@@ -1703,6 +1703,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
if (of_find_property(np, "ti,needs-special-reset", NULL))
pdata->slots[0].features |= HSMMC_HAS_UPDATED_RESET;
+ if (!of_property_read_u32(np, "max-frequency", &max_freq))
+ pdata->max_freq = max_freq;
+
return pdata;
}
#else
--
1.7.11.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 4/4] MMC: omap_hsmmc: add DT property for max bus frequency
2012-10-12 10:58 ` [PATCH 4/4] MMC: omap_hsmmc: add DT property for max bus frequency Daniel Mack
@ 2012-10-12 15:25 ` Rob Herring
2012-10-12 15:26 ` Daniel Mack
0 siblings, 1 reply; 22+ messages in thread
From: Rob Herring @ 2012-10-12 15:25 UTC (permalink / raw)
To: Daniel Mack
Cc: linux-mmc, Venkatraman S, Chris Ball, Grant Likely, linux-omap
On 10/12/2012 05:58 AM, Daniel Mack wrote:
> Maximum bus frequency can be limited by external circuitry like level
> shifters etc. Allow passing this value from DT.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> Cc: Venkatraman S <svenkatr@ti.com>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: linux-omap@vger.kernel.org
> ---
> drivers/mmc/host/omap_hsmmc.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 86f0759..4650ef7 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1676,7 +1676,7 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
> {
> struct omap_mmc_platform_data *pdata;
> struct device_node *np = dev->of_node;
> - u32 bus_width;
> + u32 bus_width, max_freq;
>
> pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> if (!pdata)
> @@ -1703,6 +1703,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
> if (of_find_property(np, "ti,needs-special-reset", NULL))
> pdata->slots[0].features |= HSMMC_HAS_UPDATED_RESET;
>
> + if (!of_property_read_u32(np, "max-frequency", &max_freq))
> + pdata->max_freq = max_freq;
> +
Is this property documented?
Rob
> return pdata;
> }
> #else
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 4/4] MMC: omap_hsmmc: add DT property for max bus frequency
2012-10-12 15:25 ` Rob Herring
@ 2012-10-12 15:26 ` Daniel Mack
0 siblings, 0 replies; 22+ messages in thread
From: Daniel Mack @ 2012-10-12 15:26 UTC (permalink / raw)
To: Rob Herring
Cc: linux-mmc, Venkatraman S, Chris Ball, Grant Likely, linux-omap
On 12.10.2012 17:25, Rob Herring wrote:
> On 10/12/2012 05:58 AM, Daniel Mack wrote:
>> Maximum bus frequency can be limited by external circuitry like level
>> shifters etc. Allow passing this value from DT.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> Cc: Venkatraman S <svenkatr@ti.com>
>> Cc: Chris Ball <cjb@laptop.org>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Cc: Rob Herring <rob.herring@calxeda.com>
>> Cc: linux-omap@vger.kernel.org
>> ---
>> drivers/mmc/host/omap_hsmmc.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>> index 86f0759..4650ef7 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -1676,7 +1676,7 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
>> {
>> struct omap_mmc_platform_data *pdata;
>> struct device_node *np = dev->of_node;
>> - u32 bus_width;
>> + u32 bus_width, max_freq;
>>
>> pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> if (!pdata)
>> @@ -1703,6 +1703,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
>> if (of_find_property(np, "ti,needs-special-reset", NULL))
>> pdata->slots[0].features |= HSMMC_HAS_UPDATED_RESET;
>>
>> + if (!of_property_read_u32(np, "max-frequency", &max_freq))
>> + pdata->max_freq = max_freq;
>> +
>
> Is this property documented?
Yes, in the generic part. Forgot to mention that.
Documentation/devicetree/bindings/mmc/mmc.txt
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/4] MMC: some omap_hsmmc fixes
2012-10-12 10:58 [PATCH 0/4] MMC: some omap_hsmmc fixes Daniel Mack
` (3 preceding siblings ...)
2012-10-12 10:58 ` [PATCH 4/4] MMC: omap_hsmmc: add DT property for max bus frequency Daniel Mack
@ 2012-10-13 9:01 ` Daniel Mack
2012-10-15 16:07 ` Venkatraman S
4 siblings, 1 reply; 22+ messages in thread
From: Daniel Mack @ 2012-10-13 9:01 UTC (permalink / raw)
To: Linux MMC List, Venkatraman S, Chris Ball, Grant Likely,
rob.herring@calxeda.com, linux-omap@vger.kernel.org,
Linus Walleij
On 12.10.2012 12:58, Daniel Mack wrote:
> Here are some assorted patches for the omap_hsmmc driver that I need on
> top Linus' current development branch to make it work on a AM33xx board.
>
> 1/4 and 2/4 qualify as bug fixes and I'm puzzled that these didn't hit
> anyone else yet.
>
>
> Daniel Mack (4):
> MMC: omap_hsmmc: set platform data after probe from DT node
> MMC: omap_hsmmc: fix DMA config block
> MMC: omap_hsmmc: claim pinctrl at probe time
> MMC: omap_hsmmc: add DT property for max bus frequency
Ok, so 1/4 and 2/4 will be solved differently upstream, so they can be
dropped.
The other two remain, and 3/4 got an Acked-by Linus W.
Thanks for the feedback,
Daniel
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 0/4] MMC: some omap_hsmmc fixes
2012-10-13 9:01 ` [PATCH 0/4] MMC: some omap_hsmmc fixes Daniel Mack
@ 2012-10-15 16:07 ` Venkatraman S
0 siblings, 0 replies; 22+ messages in thread
From: Venkatraman S @ 2012-10-15 16:07 UTC (permalink / raw)
To: Daniel Mack
Cc: Linux MMC List, Chris Ball, Grant Likely, rob.herring@calxeda.com,
linux-omap@vger.kernel.org, Linus Walleij
Daniel,
On Sat, Oct 13, 2012 at 2:31 PM, Daniel Mack <zonque@gmail.com> wrote:
> On 12.10.2012 12:58, Daniel Mack wrote:
>> Here are some assorted patches for the omap_hsmmc driver that I need on
>> top Linus' current development branch to make it work on a AM33xx board.
>>
>> 1/4 and 2/4 qualify as bug fixes and I'm puzzled that these didn't hit
>> anyone else yet.
>>
>>
>> Daniel Mack (4):
>> MMC: omap_hsmmc: set platform data after probe from DT node
>> MMC: omap_hsmmc: fix DMA config block
>> MMC: omap_hsmmc: claim pinctrl at probe time
>> MMC: omap_hsmmc: add DT property for max bus frequency
>
> Ok, so 1/4 and 2/4 will be solved differently upstream, so they can be
> dropped.
>
> The other two remain, and 3/4 got an Acked-by Linus W.
>
Thanks.
I've just clubbed your two patches with Balaji's fixes and sent to Chris.
^ permalink raw reply [flat|nested] 22+ messages in thread