On 03/11/17 15:16, Ladislav Michl wrote:
> On Fri, Nov 03, 2017 at 02:09:09PM +0100, Ladislav Michl wrote:
>> On Fri, Nov 03, 2017 at 02:15:21PM +0200, Roger Quadros wrote:
>>> On 03/11/17 11:57, Ladislav Michl wrote:
>>>> Value of device_width is previously read from documented
>>>> 'gpmc,device-width' property. For now leave 'bank-width'
>>>> as a fallback for older DT blobs.
>>>
>>> bank-width is mandatory for NOR devices only.
>>>
>>>>
>>>> Signed-off-by: Ladislav Michl <ladis@xxxxxxxxxxxxxx>
>>>> ---
>>>> drivers/memory/omap-gpmc.c | 9 +++++++--
>>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>>>> index 24b584b74d84..3dc9706e1f15 100644
>>>> --- a/drivers/memory/omap-gpmc.c
>>>> +++ b/drivers/memory/omap-gpmc.c
>>>> @@ -2152,10 +2152,15 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>>> gpmc_configure(GPMC_CONFIG_WP, 0);
>>>> gpmc_s.device_nand = true;
>>>> } else {
>>>> + /*
>>>> + * Some older DT blobs are missing device-width property,
>>>> + * so try to read bank-width and fail if neither works.
>>>> + */
>>>
>>> I'd get rid of the above comment and rather say, NOR devices use bank-width
>>> to specify device width.
>>>
>>>> ret = of_property_read_u32(child, "bank-width",
>>>> &gpmc_s.device_width);
>>>> - if (ret < 0) {
>>>> - dev_err(&pdev->dev, "%pOF has no 'bank-width' property\n",
>>>> + if (ret < 0 && !gpmc_s.device_width) {
>>>> + dev_err(&pdev->dev,
>>>> + "%pOF has no 'gpmc,device-width' property\n",
>>>> child);
>>>> goto err;
>>>> }
>>>>
>>>
>>> I'm ok with the approach.
>>> We could also update the DT binding documentation to say
>>> "'gpmc,device-width' is mandatory if device is not NOR or NAND type.
>>> For NOR, bank-width is used and for NAND nand-bus-width is used to figure
>>> out the device-width"
>>
>> Will update documentaton in separate patch.
>>
>>> And update the patch commit log accordingly.
>>
>> Something like the following?
>>
>> 8<-----------------------
>> Subject: [PATCH] memory: omap-gpmc: Make 'bank-width' NOR mandatory only
>>
>> Only NOR devices are using 'bank-width' property to configure device width.
>> Note that code does not do exactly that - it fails only if 'bank-width' is
>> missing and device width was not configured previously using
>> 'gpmc,device-width'.
>>
>> Signed-off-by: Ladislav Michl <ladis@xxxxxxxxxxxxxx>
>> ---
>> drivers/memory/omap-gpmc.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>> index ff73f2ad468c..26cc447662f0 100644
>> --- a/drivers/memory/omap-gpmc.c
>> +++ b/drivers/memory/omap-gpmc.c
>> @@ -2157,9 +2157,10 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>> gpmc_configure(GPMC_CONFIG_WP, 0);
>> gpmc_s.device_nand = true;
>> } else {
>> + /* NOR devices use bank-width to specify device width */
>
> Actually this comment is still wrong (and thus commit message).
> Documentation/devicetree/bindings/net/gpmc-eth.txt
> Ethernet is using this as well...
Oh :(. Ideally it shouldn't have been the case. But damage is already done.
I think we should still fix up the gpmc-eth binding document to use gpmc,device-width there.
And continue to support in code anybody already using bank-width for ethernet.
>
>> ret = of_property_read_u32(child, "bank-width",
>> &gpmc_s.device_width);
>> - if (ret < 0) {
>> + if (ret < 0 && !gpmc_s.device_width) {
>> dev_err(&pdev->dev, "%pOF has no 'bank-width' property\n",
>> child);
>> goto err;
>> --
>> 2.11.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
