Abhilash Kesavan wrote:
>
> Hi Kukjin,
>
Hi,
> 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 ?
>
Sorry for late response, I've missed this :(
I see and agreed. Please keep go ahead with my ack.
Acked-by: Kukjin Kim <kgene@xxxxxxxxxx>
Thanks,
Kukjin
--
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
