On Thu, Mar 10, 2022 at 08:27:43PM +0100, Miquel Raynal wrote: > andriy.shevchenko@xxxxxxxxxxxxxxx wrote on Thu, 10 Mar 2022 20:25:52 > +0200: > > On Thu, Mar 10, 2022 at 05:16:49PM +0100, Miquel Raynal wrote: ... > > > +/* Offsets for the Renesas RZ/N1 DesignWare specific registers */ > > > +/* DMA Software Ack */ > > > +#define RZN1_UART_DMASA 0xa8 > > > > Is it specific to Renesas? IIRC it's Synopsys DesignWare register, makes > > sense to use appropriate prefix or no prefix. > > I have no idea, I can use a more common prefix. It's a register described in Synopsys DesignWare specification. It's not related to Renesas IP integration. ... > > > +#define RZN1_UART_xDMACR_1_WORD_BURST 0 > > > +#define RZN1_UART_xDMACR_4_WORD_BURST BIT(1) > > > +#define RZN1_UART_xDMACR_8_WORD_BURST (BIT(1) | BIT(2)) > > > > This looks like incorrect use of BIT() macro. > > Please, use plain decimal integers. Something like > > > > 1 (0 << 1) > > 4 (1 << 1) > > 8 (3 << 1) > > > > If I'm mistaken, describe the meaning of each bit there. > > Matter of taste, I believe, whatever. Actually no, when one uses BIT() masks it implies that in the datasheet each bit is meaningful. So, please clarify this and update accordingly. ... > > > + data->is_rzn1 = of_device_is_compatible(dev->of_node, "renesas,rzn1-uart"); > > > > Device property API. > > I'm not sure to get what you mean here again. The Information is in the > device tree, the compatible string already gives us what we need, why > would we need a device property? (or perhaps I misunderstand what > "device property API" means) Use non-OF APIs, which usually starts with device_property_ or fwnode_. But Geert already suggested something better. > > > /* Always ask for fixed clock rate from a property. */ > > > device_property_read_u32(dev, "clock-frequency", &p->uartclk); -- With Best Regards, Andy Shevchenko
