On Wed, Apr 01, 2020 at 6:20:20PM +0900, Krzysztof Kozlowski wrote: > On Wed, Apr 01, 2020 at 10:55:48AM +0200, Greg Kroah-Hartman > wrote: > > On Wed, Apr 01, 2020 at 05:27:20PM +0900, Hyunki Koo wrote: > > > - if (np) > > > + if (np) { > > > of_property_read_u32(np, > > > "samsung,uart-fifosize", &ourport->port.fifosize); > > > > > > + if (of_property_read_u32(np, "reg-io-width", &prop) == > 0) { > > > + switch (prop) { > > > + case 1: > > > + ourport->port.iotype = UPIO_MEM; > > > + break; > > > + case 4: > > > + ourport->port.iotype = UPIO_MEM32; > > > + break; > > > + default: > > > + dev_warn(&pdev->dev, "unsupported > reg-io-width (%d)\n", > > > + prop); > > > + ret = -EINVAL; > > > + break; > > > + } > > > + } > > > + } > > > + > > > > Does this mean that reg-io-width is now a required property for all > > samsung uarts? Does this break older dts files? Or should you > > fall-back to the previous operation if the attribute is not there? > > Yes, it looks like silently breaking all boards. Since > of_property_read_u32() will return errno, the warning message won't be > printed and all register reads will fail (return 0). > > This looks like not tested on real HW. > > Best regards, > Krzysztof [Hyunki Koo] reg-io-width =4 is required for Samsung uart To do not break older dts files, I will set default value in else of of_property_read_u32 like below. + if (of_property_read_u32(np, "reg-io-width", &prop) == 0) { + ... + } else { + ourport->port.iotype = UPIO_MEM; + }