Re: [PATCH 6/7] serial: 8250_dw: Add support for RZ/N1 DMA

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Geert,

geert@linux-m68k.org wrote on Fri, 11 Mar 2022 10:51:53 +0100:

> Hi Miquel,
> 
> CC esmil
> 

> > > --- a/drivers/tty/serial/8250/8250_dma.c
> > > +++ b/drivers/tty/serial/8250/8250_dma.c  
> >  
> > > @@ -501,6 +589,8 @@ static int dw8250_probe(struct platform_device *pdev)
> > >                 data->msr_mask_off |= UART_MSR_TERI;
> > >         }
> > >
> > > +       data->is_rzn1 = of_device_is_compatible(dev->of_node, "renesas,rzn1-uart");  
> >
> > Explicit checks for compatible values are frowned upon if you have
> > a match table.
> > Please handle this through of_device.data, cfr. the various quirks.  
> 
> Oops, these are not yet upstream, but present in my tree due to including
> support for StarLight, cfr.
> https://github.com/esmil/linux/commits/visionfive/drivers/tty/serial/8250/8250_dw.c

Oh thanks for pointing it! Too bad that these quirks were not
introduced inside a wider structure, I think it's always a must even if
there is only one parameter there. Anyway, I'll introduce a wider
specific structure and use it.

> But you do already have:
> 
> +       { .compatible = "renesas,rzn1-uart", .data = &rzn1_pdata },
> 
> since "[PATCH 4/7] serial: 8250_dw: Provide the RZN1 CPR register value".
> 
> > Please rename "is_rzn1" to something that describes the feature.
> >  
> > > +
> > >         /* Always ask for fixed clock rate from a property. */
> > >         device_property_read_u32(dev, "clock-frequency", &p->uartclk);  
> 
> Gr{oetje,eeting}s,
> 
>                         Geert


Thanks,
Miquèl