Hi Kukjin,
On Wed, Mar 4, 2015 at 11:06 AM, Abhilash Kesavan
<kesavan.abhilash@xxxxxxxxx> wrote:
> Hi Kukjin,
>
> Thanks for the review.
>
> On Tue, Mar 3, 2015 at 1:01 AM, Kukjin Kim <kgene@xxxxxxxxxx> wrote:
>> On 03/02/15 01:22, Abhilash Kesavan wrote:
>>> In the existing code, the optional fifosizes property passed via device
>>> tree was being overwritten. Ensure the setting of correct fifo sizes for
>>> both dt and non-dt cases.
>>>
>>> Signed-off-by: Abhilash Kesavan <a.kesavan@xxxxxxxxxxx>
>>> ---
>>> drivers/tty/serial/samsung.c | 25 +++++++++++++++++--------
>>> 1 file changed, 17 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>>> index af821a9..e04bb5e 100644
>>> --- a/drivers/tty/serial/samsung.c
>>> +++ b/drivers/tty/serial/samsung.c
>>> @@ -1827,14 +1827,23 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
>>> dev_get_platdata(&pdev->dev) :
>>> ourport->drv_data->def_cfg;
>>>
>>> - if (np)
>>> - of_property_read_u32(np,
>>> - "samsung,uart-fifosize", &ourport->port.fifosize);
>>> -
>>> - if (ourport->drv_data->fifosize[index])
>>> - ourport->port.fifosize = ourport->drv_data->fifosize[index];
>>> - else if (ourport->info->fifosize)
>>> - ourport->port.fifosize = ourport->info->fifosize;
>>> + if (np) {
>>> + if (of_property_read_u32(np,
>>> + "samsung,uart-fifosize", &ourport->port.fifosize)) {
>>> + if (ourport->drv_data->fifosize[index])
>>> + ourport->port.fifosize =
>>> + ourport->drv_data->fifosize[index];
>>> + else if (ourport->info->fifosize)
>>> + ourport->port.fifosize =
>>> + ourport->info->fifosize;
>>> + }
>>> + } else {
>>> + if (ourport->drv_data->fifosize[index])
>>> + ourport->port.fifosize =
>>> + ourport->drv_data->fifosize[index];
>>> + else if (ourport->info->fifosize)
>>> + ourport->port.fifosize = ourport->info->fifosize;
>>> + }
>>
>
> Let me elaborate on the need for this patch. The exynos7 serial
> controller is similar to that present in earlier exynos4/5 SoCs except
> for the channel fifo sizes. Rather than adding a new compatible string
> and driver data for exynos7 just to handle this difference, I decided
> to use the optional "samsung,uart-fifosize" property. However, while
> doing so I noticed that the dt passed value gets overwritten in the
> driver code.
>
>> Hmm...if 'np' is true and there is no property "samsung,uart-fifo-size"
>> in DT?
>
> This case is being handled in my patch. If the serial node is present
> but the "samsung,uart-fifosize" property is not, then it will set the
> fifo size from wither the driver data or info structure. Now, if the
> both the serial node and "samsung,uart-fifosize" property are present
> then it will use the value passed via dt.
>
>>
>> How about following? I need to think, in case of existing uart-fifosize
>> in DT, which one should be overwritten though...
>>
>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>> index af821a9..0ad38ec 100644
>> --- a/drivers/tty/serial/samsung.c
>> +++ b/drivers/tty/serial/samsung.c
>> @@ -1827,12 +1827,11 @@ static int s3c24xx_serial_probe(struct
>> platform_device *pdev)
>> dev_get_platdata(&pdev->dev) :
>> ourport->drv_data->def_cfg;
>>
>> - if (np)
>> - of_property_read_u32(np,
>> - "samsung,uart-fifosize", &ourport->port.fifosize);
>> -
>> if (ourport->drv_data->fifosize[index])
>> ourport->port.fifosize = ourport->drv_data->fifosize[index];
>> + else if (np)
>> + of_property_read_u32(np,
>> + "samsung,uart-fifosize", &ourport->port.fifosize);
>> else if (ourport->info->fifosize)
>> ourport->port.fifosize = ourport->info->fifosize;
>>
> This approach gives priority to the driver data fifo size and so for
> all the current SoCs adding "samsung,uart-fifosize" property would
> have no effect.
Gentle ping. Do you have any further thoughts on this ?
Regards,
Abhilash
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
