Skip to contentSkip to conversation

ns16550: change reg-shift property default to zero

PATCH In review Thomas Chou 6 participants 7 messages Nov 29, 2015 – Dec 8, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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.

---
 drivers/serial/ns16550.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/serial/ns16550.c +1 -1
index d5bcbc3..5b64f7c 100644
@@ -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);
Signature · 1 line
2.5.0
#1 lore raw

PATCH 1/1 ns16550: change reg-shift property default to zero

Quoted 17 lines · Thomas Chou

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.

---
 drivers/serial/ns16550.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/serial/ns16550.c +0 -0
index d5bcbc3..5b64f7c 100644

drivers/serial/ns16550.c int ns16550_serial_ofdata_to_platdata(struct udevice *dev)

Quoted 9 lines
drivers/serial/ns16550.c +1 -2
@@ -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);
--
#2 lore raw

PATCH 1/1 ns16550: change reg-shift property default to zero

In reply to Thomas Chou
Quoted 8 lines · Thomas Chou

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.

Regards
Mugunthan V N

#3 lore raw

PATCH 1/1 ns16550: change reg-shift property default to zero

Quoted 11 lines · Mugunthan V N
Quoted 8 lines · Thomas Chou

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.

#4 lore raw
In reply to Thomas Chou
Quoted 11 lines · Thomas Chou

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.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151206/c0746af8/attachment.sig>
#5 lore raw
Quoted 5 lines · Tom Rini
Quoted 3 lines · Thomas Chou

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.)

#6 lore raw

Hi Stephen,

Quoted 15 lines · Stephen Warren
Quoted 5 lines · Tom Rini
Quoted 3 lines · Thomas Chou

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.

Quoted 8 lines

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