[U-Boot] [PATCH] ns16550: change reg-shift property default to zero
Change reg-shift property default to zero. When the integer property is missing, it should be taken as zero. This is consistent to Linux drivers/tty/serial/of_serial.c.
The x86 and most powerpc use reg-shift of 0. Most others use reg-shift of 2. While reg-shift of 1 is rarely used.
Signed-off-by: Thomas Chou thomas@wytron.com.tw --- drivers/serial/ns16550.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index d5bcbc3..5b64f7c 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -403,7 +403,7 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
plat->base = addr; plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset, - "reg-shift", 1); + "reg-shift", 0); plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "clock-frequency", CONFIG_SYS_NS16550_CLK);
On Sun, Nov 29, 2015 at 2:01 PM, Thomas Chou thomas@wytron.com.tw wrote:
Change reg-shift property default to zero. When the integer property is missing, it should be taken as zero. This is consistent to Linux drivers/tty/serial/of_serial.c.
The x86 and most powerpc use reg-shift of 0. Most others use reg-shift of 2. While reg-shift of 1 is rarely used.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
drivers/serial/ns16550.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index d5bcbc3..5b64f7c 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -403,7 +403,7 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
plat->base = addr; plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
"reg-shift", 1);
"reg-shift", 0); plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "clock-frequency", CONFIG_SYS_NS16550_CLK);--
Reviewed-by: Bin Meng bmeng.cn@gmail.com
On Sunday 29 November 2015 11:31 AM, Thomas Chou wrote:
Change reg-shift property default to zero. When the integer property is missing, it should be taken as zero. This is consistent to Linux drivers/tty/serial/of_serial.c.
The x86 and most powerpc use reg-shift of 0. Most others use reg-shift of 2. While reg-shift of 1 is rarely used.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
Reviewed-by: Mugunthan V N mugunthanvnm@ti.com
Regards Mugunthan V N
On 29 November 2015 at 23:07, Mugunthan V N mugunthanvnm@ti.com wrote:
On Sunday 29 November 2015 11:31 AM, Thomas Chou wrote:
Change reg-shift property default to zero. When the integer property is missing, it should be taken as zero. This is consistent to Linux drivers/tty/serial/of_serial.c.
The x86 and most powerpc use reg-shift of 0. Most others use reg-shift of 2. While reg-shift of 1 is rarely used.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
Reviewed-by: Mugunthan V N mugunthanvnm@ti.com
Acked-by: Simon Glass sjg@chromium.org
On Sun, Nov 29, 2015 at 02:01:03PM +0800, Thomas Chou wrote:
Change reg-shift property default to zero. When the integer property is missing, it should be taken as zero. This is consistent to Linux drivers/tty/serial/of_serial.c.
The x86 and most powerpc use reg-shift of 0. Most others use reg-shift of 2. While reg-shift of 1 is rarely used.
Signed-off-by: Thomas Chou thomas@wytron.com.tw Reviewed-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Mugunthan V N mugunthanvnm@ti.com Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
On 12/06/2015 03:07 PM, Tom Rini wrote:
On Sun, Nov 29, 2015 at 02:01:03PM +0800, Thomas Chou wrote:
Change reg-shift property default to zero. When the integer property is missing, it should be taken as zero. This is consistent to Linux drivers/tty/serial/of_serial.c.
It's not generally true that missing properties have value zero. Rather, whatever values was assumed by the semantics of the binding before that optional property was defined should be assumed.
What the Linux kernel driver does also isn't justification for this change, since DT bindings define how they work, rather than a particular OS implementation forcing the hand of the binding.
The DT binding documentation must state the default value/semantics for any optional property. Can you please make sure the DT binding documentation is updated to describe this case?
(Note that I have no objection to this patch; the actual change seems fine. It's simply that the justifications given in the patch description for it aren't entirely robust.)
Hi Stephen,
On 2015年12月08日 01:32, Stephen Warren wrote:
On 12/06/2015 03:07 PM, Tom Rini wrote:
On Sun, Nov 29, 2015 at 02:01:03PM +0800, Thomas Chou wrote:
Change reg-shift property default to zero. When the integer property is missing, it should be taken as zero. This is consistent to Linux drivers/tty/serial/of_serial.c.
It's not generally true that missing properties have value zero. Rather, whatever values was assumed by the semantics of the binding before that optional property was defined should be assumed.
What the Linux kernel driver does also isn't justification for this change, since DT bindings define how they work, rather than a particular OS implementation forcing the hand of the binding.
The DT binding documentation must state the default value/semantics for any optional property. Can you please make sure the DT binding documentation is updated to describe this case?
(Note that I have no objection to this patch; the actual change seems fine. It's simply that the justifications given in the patch description for it aren't entirely robust.)
Thanks. Will send a follow-up patch for the DT binding.
Best regards, Thomas
participants (6)
-
Bin Meng -
Mugunthan V N -
Simon Glass -
Stephen Warren -
Thomas Chou -
Tom Rini