serial: ns16550: Add register shift variable
PATCH 1/1 serial: ns16550: Add register shift variable
This patch adds a new Kconfig variable that allows setting the register shift value for the ns16550 driver to some other value then 0 if not defined by the DT. All credit for this patch goes to Lokesh Vutla as it was his idea.
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current am33xx.dtsi file from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+) am33xx.dtsi file from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example, SYS_NS16550_REG_SHIFT is set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.
---
drivers/serial/Kconfig | 12 ++++++++++++ drivers/serial/ns16550.c | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-)
drivers/serial/Kconfig +12 -0
@@ -530,6 +530,18 @@ config SYS_NS16550 be used. It can be a constant or a function to get clock, eg, get_serial_clock(). +config SYS_NS16550_REG_SHIFT + int "Number of bytes to shift register offsets" + default 0 + depends on SYS_NS16550 + help + Use this to specify the amount of bytes between discrete + device registers. If, for example, the device registers are + located at 0x00, 0x04, 0x08, 0x0C and so forth than set + this to 2. The default value is 0. + Note that a <reg-shift> property defined in a UART node of + the device tree will always take precedence. + config INTEL_MID_SERIAL bool "Intel MID platform UART support" depends on DM_SERIAL && OF_CONTROL
drivers/serial/ns16550.c +2 -1
@@ -442,7 +442,8 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)#endif plat->reg_offset =dev_read_u32_default (dev,"reg-offset" ,0 ); - plat->reg_shift =dev_read_u32_default (dev,"reg-shift" ,0 ); + plat->reg_shift =dev_read_u32_default (dev,"reg-shift" , +CONFIG_SYS_NS16550_REG_SHIFT ); err =clk_get_by_index (dev,0 , &clk);if (!err) {
Signature · 1 line
2.7.4
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 10 lines · Felix Brack
This patch adds a new
Kconfigvariable that allows setting the register shift value for the ns16550 driver to some other value then 0 if not defined by the DT. All credit for this patch goes to Lokesh Vutla as it was his idea.The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent
How does Linux determine the shift value then?
Alex
Quoted 50 lines
(probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,
SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.Signed-off-by Felix Brack <[email protected]>---
drivers/serial/Kconfig | 12 ++++++++++++ drivers/serial/ns16550.c | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-)drivers/serial/Kconfig +12
-0@@ -530,6 +530,18 @@ config SYS_NS16550 be used. It can be a constant or a function to get clock, eg, get_serial_clock(). +config SYS_NS16550_REG_SHIFT + int "Number of bytes to shift register offsets" + default 0 + depends on SYS_NS16550 + help + Use this to specify the amount of bytes between discrete + device registers. If, for example, the device registers are + located at 0x00, 0x04, 0x08, 0x0C and so forth than set + this to 2. The default value is 0. + Note that a <reg-shift> property defined in a UART node of + the device tree will always take precedence. + config INTEL_MID_SERIAL bool "Intel MID platform UART support" depends on DM_SERIAL && OF_CONTROLdrivers/serial/ns16550.c +2
-1@@ -442,7 +442,8 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)#endif plat->reg_offset =dev_read_u32_default (dev,"reg-offset" ,0 ); - plat->reg_shift =dev_read_u32_default (dev,"reg-shift" ,0 ); + plat->reg_shift =dev_read_u32_default (dev,"reg-shift" , +CONFIG_SYS_NS16550_REG_SHIFT ); err =clk_get_by_index (dev,0 , &clk);if (!err) {
PATCH 1/1 serial: ns16550: Add register shift variable
In reply to Felix BrackDear Felix,
Quoted 11 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.
Does this mean that U-Boot will not be able to use the same DTB as Linux?
drivers/serial/Kconfig config SYS_NS16550 · line 541
Quoted 9 lines
drivers/serial/Kconfig +9
-0+config SYS_NS16550_REG_SHIFT + int "Number of bytes to shift register offsets" + default 0 + depends on SYS_NS16550 + help + Use this to specify the amount of bytes between discrete + device registers. If, for example, the device registers are + located at 0x00, 0x04, 0x08, 0x0C and so forth than set + this to 2. The default value is 0.
This description is inconsistent or misleading. How do you define "space between registers"? The unused gaps? In the example, the registers are spaced at 4 bytes intervals, so a value of 2 would only make sense of we have 16 bit registters and you count the gap bytes.
But this is a very strange and uncommon way to describe such a situation, especially when you write that you "shift register offsets". Here I think about something like a LSL operation, so shifing by 2 bits would result in a 2^2 = 4 byte spacing.
This needs to be rewritten.
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Program maintenance is an entropy-increasing process, and even its most skilfull execution only delays the subsidence of the system into unfixable obsolescence. - Fred Brooks, "The Mythical Man Month"
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 15 lines · Wolfgang Denk
Dear Felix,
Quoted 11 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
Quoted 17 lines
drivers/serial/Kconfig config SYS_NS16550 · line 541
Quoted 10 lines
drivers/serial/Kconfig +9
-0+config SYS_NS16550_REG_SHIFT + int "Number of bytes to shift register offsets" + default 0 + depends on SYS_NS16550 + help + Use this to specify the amount of bytes between discrete + device registers. If, for example, the device registers are + located at 0x00, 0x04, 0x08, 0x0C and so forth than set + this to 2. The default value is 0.This description is inconsistent or misleading. How do you define "space between registers"? The unused gaps? In the example, the registers are spaced at 4 bytes intervals, so a value of 2 would only make sense of we have 16 bit registters and you count the gap bytes.
Quoted 6 lines
But this is a very strange and uncommon way to describe such a situation, especially when you write that you "shift register offsets". Here I think about something like a LSL operation, so shifing by 2 bits would result in a 2^2 = 4 byte spacing.
This needs to be rewritten.
To try and help clarify, the property in question means "quantity to shift the register offsets by." It should be clear in our Kconfig help entry as well that this is what we're looking for.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180714/f1dd1952/attachment.sig>
PATCH 1/1 serial: ns16550: Add register shift variable
Hi Tom, Felix, all
Quoted 24 lines
-----Original Message-----
From: Tom Rini [mailto:trini at konsulko.com]
Sent: Saturday, July 14, 2018 6:50 PM
To: Wolfgang Denk <[email protected]>
Cc: Felix Brack <[email protected]>; u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexey Brodkin <[email protected]>; Alexander Graf <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableQuoted 15 lines · Wolfgang Denk
Dear Felix,
Quoted 11 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.
Quoted 6 lines
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
Any chance to get a reference to the commit in Linux kernel that introduces that change?
I still see a bunch of:
----------------------------->8---------------------- reg-shift = <2>;
reg-io-width = <4>; ----------------------------->8---------------------- in .dts files for ARC boards even in linux-next git tree.
-Alexey
PATCH 1/1 serial: ns16550: Add register shift variable
Hi Alexey,
Quoted 26 lines · Alexey Brodkin
Hi Tom, Felix, all
Quoted 24 lines
-----Original Message-----
From: Tom Rini [mailto:trini at konsulko.com]
Sent: Saturday, July 14, 2018 6:50 PM
To: Wolfgang Denk <[email protected]>
Cc: Felix Brack <[email protected]>; u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexey Brodkin <[email protected]>; Alexander Graf <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableQuoted 15 lines · Wolfgang Denk
Dear Felix,
Quoted 11 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.
Quoted 9 lines
Quoted 6 lines
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
Any chance to get a reference to the commit in Linux kernel that introduces that change?
In fact I believe that the <reg-shift> property never existed in the am33xx.dtsi file from Linux. U-Boot commit 85cf0e6299 shows that the property has been added to U-Boot's am33xx.dtsi file. The commit log clearly states why this happened:
"With the commit 'c7b9686d5d48 ("ns16550: unify serial_omap")' all TI platforms are broken with DM/DT boot as ns16550 driver expects reg-shift from DT which is not populated for TI platforms. Earlier it worked as it was hard coded to 2 in serial-omap driver. So adding the reg-shift to serial nodes for dra7, am4372 and am33xx dtsi files. Tested this patch on am437x-sk-evm, am437x-gp-evm, am335x-boneblack, dra74x-evm and dra72x-evm."
Quoted 9 lines
I still see a bunch of:
----------------------------->8---------------------- reg-shift = <2>;
reg-io-width = <4>; ----------------------------->8---------------------- in .dts files for ARC boards even in linux-next git tree.-Alexey
regards Felix
PATCH 1/1 serial: ns16550: Add register shift variable
Hi Felix,
Quoted 51 lines
-----Original Message-----
From: Felix Brack [mailto:fb at ltec.ch]
Sent: Sunday, July 15, 2018 5:02 PM
To: Alexey Brodkin <[email protected]>; Tom Rini <[email protected]>
Cc: u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexander Graf <[email protected]>; Michal Simek <[email protected]>; Wolfgang Denk <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableHi Alexey,
Quoted 26 lines · Alexey Brodkin
Hi Tom, Felix, all
Quoted 24 lines
-----Original Message-----
From: Tom Rini [mailto:trini at konsulko.com]
Sent: Saturday, July 14, 2018 6:50 PM
To: Wolfgang Denk <[email protected]>
Cc: Felix Brack <[email protected]>; u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexey Brodkin <[email protected]>; Alexander Graf <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableQuoted 15 lines · Wolfgang Denk
Dear Felix,
Quoted 11 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.Quoted 9 lines
Quoted 6 lines
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
Any chance to get a reference to the commit in Linux kernel that introduces that change?
In fact I believe that the <reg-shift> property never existed in the
am33xx.dtsifile from Linux. U-Boot commit85cf0e6299shows that the property has been added to U-Boot'sam33xx.dtsifile. The commit log clearly states why this happened:
Quoted 7 lines
"With the commit '
c7b9686d5d48("ns16550: unifyserial_omap")' all TI platforms are broken with DM/DT boot as ns16550 driver expects reg-shift from DT which is not populated for TI platforms. Earlier it worked as it was hard coded to 2 in serial-omap driver. So adding the reg-shift to serial nodes for dra7, am4372 and am33xx dtsi files. Tested this patch on am437x-sk-evm, am437x-gp-evm, am335x-boneblack, dra74x-evm and dra72x-evm."
Ok so this has nothing to do with changes in Linux kernel I guess. Why don't we go the other way around and not just submit
a change to .dts files in Linux kernel that are missing required properties?
-Alexey
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 15 lines · Alexey Brodkin
Quoted 7 lines
Quoted 2 lines
Any chance to get a reference to the commit in Linux kernel that introduces that change?
In fact I believe that the <reg-shift> property never existed in the
am33xx.dtsifile from Linux. U-Boot commit85cf0e6299shows that the property has been added to U-Boot'sam33xx.dtsifile. The commit log clearly states why this happened:Quoted 7 lines
"With the commit '
c7b9686d5d48("ns16550: unifyserial_omap")' all TI platforms are broken with DM/DT boot as ns16550 driver expects reg-shift from DT which is not populated for TI platforms. Earlier it worked as it was hard coded to 2 in serial-omap driver. So adding the reg-shift to serial nodes for dra7, am4372 and am33xx dtsi files. Tested this patch on am437x-sk-evm, am437x-gp-evm, am335x-boneblack, dra74x-evm and dra72x-evm."
Quoted 3 lines
Ok so this has nothing to do with changes in Linux kernel I guess. Why don't we go the other way around and not just submit
a change to .dts files in Linux kernel that are missing required properties?
You may change DTSi files in the Linux kernel sources tree, but you wouldn't be able to fix all DTB:s in the wild.
In this case Linux should have a fallback to the previous value in case property is not found.
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 18 lines · Andy Shevchenko
Quoted 15 lines · Alexey Brodkin
Quoted 14 lines
Quoted 2 lines
Any chance to get a reference to the commit in Linux kernel that introduces that change?
In fact I believe that the <reg-shift> property never existed in the
am33xx.dtsifile from Linux. U-Boot commit85cf0e6299shows that the property has been added to U-Boot'sam33xx.dtsifile. The commit log clearly states why this happened:"With the commit '
c7b9686d5d48("ns16550: unifyserial_omap")' all TI platforms are broken with DM/DT boot as ns16550 driver expects reg-shift from DT which is not populated for TI platforms. Earlier it worked as it was hard coded to 2 in serial-omap driver. So adding the reg-shift to serial nodes for dra7, am4372 and am33xx dtsi files. Tested this patch on am437x-sk-evm, am437x-gp-evm, am335x-boneblack, dra74x-evm and dra72x-evm."
Quoted 8 lines
Quoted 3 lines
Ok so this has nothing to do with changes in Linux kernel I guess. Why don't we go the other way around and not just submit
a change to .dts files in Linux kernel that are missing required properties?You may change DTSi files in the Linux kernel sources tree, but you wouldn't be able to fix all DTB:s in the wild.
In this case Linux should have a fallback to the previous value in case property is not found.
No, we don't allow for U-Boot to be run with an arbitrary "wild" DTB file nor the same DTB as the kernel (the kernel could run with a subset of our DTB however).
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180717/6a0c5673/attachment.sig>
PATCH 1/1 serial: ns16550: Add register shift variable
In reply to Alexey BrodkinHello Alexey,
Quoted 61 lines · Alexey Brodkin
Hi Felix,
Quoted 51 lines
-----Original Message-----
From: Felix Brack [mailto:fb at ltec.ch]
Sent: Sunday, July 15, 2018 5:02 PM
To: Alexey Brodkin <[email protected]>; Tom Rini <[email protected]>
Cc: u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexander Graf <[email protected]>; Michal Simek <[email protected]>; Wolfgang Denk <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableHi Alexey,
Quoted 26 lines · Alexey Brodkin
Hi Tom, Felix, all
Quoted 24 lines
-----Original Message-----
From: Tom Rini [mailto:trini at konsulko.com]
Sent: Saturday, July 14, 2018 6:50 PM
To: Wolfgang Denk <[email protected]>
Cc: Felix Brack <[email protected]>; u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexey Brodkin <[email protected]>; Alexander Graf <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableQuoted 15 lines · Wolfgang Denk
Dear Felix,
Quoted 11 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.Quoted 9 lines
Quoted 6 lines
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
Any chance to get a reference to the commit in Linux kernel that introduces that change?
In fact I believe that the <reg-shift> property never existed in the
am33xx.dtsifile from Linux. U-Boot commit85cf0e6299shows that the property has been added to U-Boot'sam33xx.dtsifile. The commit log clearly states why this happened:Quoted 7 lines
"With the commit '
c7b9686d5d48("ns16550: unifyserial_omap")' all TI platforms are broken with DM/DT boot as ns16550 driver expects reg-shift from DT which is not populated for TI platforms. Earlier it worked as it was hard coded to 2 in serial-omap driver. So adding the reg-shift to serial nodes for dra7, am4372 and am33xx dtsi files. Tested this patch on am437x-sk-evm, am437x-gp-evm, am335x-boneblack, dra74x-evm and dra72x-evm."
Quoted 4 lines
Ok so this has nothing to do with changes in Linux kernel I guess. Why don't we go the other way around and not just submit
a change to .dts files in Linux kernel that are missing required properties?
I think there is nothing wrong with the kernel's .dtsi files, hence nothing to be fixed in Linux.
Please read the RFC I posted May 24. It (hopefully) explains the need of this patch. In short: syncing U-Boot DT files for AM33xx SoC with Linux DT files will break things. This patch would fix the problem of the "missing <reg-shift>" property.
regards Felix
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 63 lines · Felix Brack
Hello Alexey,
Quoted 56 lines · Alexey Brodkin
Hi Felix,
Quoted 47 lines
-----Original Message-----
From: Felix Brack [mailto:fb at ltec.ch]
Sent: Sunday, July 15, 2018 5:02 PM
To: Alexey Brodkin <[email protected]>; Tom Rini <[email protected]>
Cc: u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexander Graf <[email protected]>; Michal Simek <[email protected]>; Wolfgang Denk <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableHi Alexey,
Quoted 24 lines · Alexey Brodkin
Hi Tom, Felix, all
Quoted 22 lines
-----Original Message-----
From: Tom Rini [mailto:trini at konsulko.com]
Sent: Saturday, July 14, 2018 6:50 PM
To: Wolfgang Denk <[email protected]>
Cc: Felix Brack <[email protected]>; u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexey Brodkin <[email protected]>; Alexander Graf <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableQuoted 13 lines · Wolfgang Denk
Dear Felix,
Quoted 10 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
Any chance to get a reference to the commit in Linux kernel that introduces that change?
In fact I believe that the <reg-shift> property never existed in the
am33xx.dtsifile from Linux. U-Boot commit85cf0e6299shows that the property has been added to U-Boot'sam33xx.dtsifile. The commit log clearly states why this happened:Quoted 7 lines
"With the commit '
c7b9686d5d48("ns16550: unifyserial_omap")' all TI platforms are broken with DM/DT boot as ns16550 driver expects reg-shift from DT which is not populated for TI platforms. Earlier it worked as it was hard coded to 2 in serial-omap driver. So adding the reg-shift to serial nodes for dra7, am4372 and am33xx dtsi files. Tested this patch on am437x-sk-evm, am437x-gp-evm, am335x-boneblack, dra74x-evm and dra72x-evm."Quoted 4 lines
Ok so this has nothing to do with changes in Linux kernel I guess. Why don't we go the other way around and not just submit
a change to .dts files in Linux kernel that are missing required properties?
Quoted 6 lines
I think there is nothing wrong with the kernel's .dtsi files, hence nothing to be fixed in Linux.
Please read the RFC I posted May 24. It (hopefully) explains the need of this patch. In short: syncing U-Boot DT files for AM33xx SoC with Linux DT files will break things. This patch would fix the problem of the "missing <reg-shift>" property.
It's not missing, we're just simply misimplementing the semantics of the "compatible" property.
Really, with a DT described system you should not need any per-device Kconfig options. Any of those are a very good indicator you're doing something wrong :).
Alex
PATCH 1/1 serial: ns16550: Add register shift variable
In reply to Tom RiniHi Tom,
Quoted 16 lines · Tom Rini
Quoted 15 lines · Wolfgang Denk
Dear Felix,
Quoted 11 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.
Quoted 7 lines
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
This is exactly the point. These files have diverged quite a lot between U-Boot and Linux since the last synchronization. I am trying to find a method to re-synchronize U-Boot generating minimal collateral damage - this patch is supposed to be part of that effort.
In fact we could even set the default value of SYS_NS16550_REG_SHIFT to 2 for _all_ am33xx SOC boards. That would probably further reduce possible problems.
Quoted 22 lines
Quoted 22 lines
drivers/serial/Kconfig config SYS_NS16550 · line 541
Quoted 10 lines
drivers/serial/Kconfig +9
-0+config SYS_NS16550_REG_SHIFT + int "Number of bytes to shift register offsets" + default 0 + depends on SYS_NS16550 + help + Use this to specify the amount of bytes between discrete + device registers. If, for example, the device registers are + located at 0x00, 0x04, 0x08, 0x0C and so forth than set + this to 2. The default value is 0.This description is inconsistent or misleading. How do you define "space between registers"? The unused gaps? In the example, the registers are spaced at 4 bytes intervals, so a value of 2 would only make sense of we have 16 bit registters and you count the gap bytes.
But this is a very strange and uncommon way to describe such a situation, especially when you write that you "shift register offsets". Here I think about something like a LSL operation, so shifing by 2 bits would result in a 2^2 = 4 byte spacing.
Quoted 6 lines
Quoted 1 line
This needs to be rewritten.
To try and help clarify, the property in question means "quantity to shift the register offsets by." It should be clear in our
Kconfighelp entry as well that this is what we're looking for.
Thanks for this! The help text will be fixed in v2.
regards Felix
PATCH 1/1 serial: ns16550: Add register shift variable
Dear Felix,
Quoted 8 lines · In message <[email protected]> you wrote:
Quoted 7 lines
Quoted 2 lines
This needs to be rewritten.
To try and help clarify, the property in question means "quantity to shift the register offsets by." It should be clear in our
Kconfighelp entry as well that this is what we're looking for.Thanks for this! The help text will be fixed in v2.
None of the places I could find in the Linux Documentation/devicetree/bindings directory actually explains what
"shift the register offsets" means. Only the code reveals that
this means a LEFT shift of the register address (offset).
At least one place in Linux interprets <reg-shift> differently; here it operates on the _data_ (instead there they use <reg-spacing>):
"devicetree/bindings/ipmi/ipmi-smic.txt":
- reg-shift - The amount to shift the registers to the right to get the data into bit zero.
(see ipmi_si_mem_io.c
So can we please add an explanation what _exactly_ this means?
Thanks.
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de It is easier to write an incorrect program than understand a correct one.
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 22 lines · Wolfgang Denk
Dear Felix,
Quoted 8 lines · In message <[email protected]> you wrote:
Quoted 7 lines
Quoted 2 lines
This needs to be rewritten.
To try and help clarify, the property in question means "quantity to shift the register offsets by." It should be clear in our
Kconfighelp entry as well that this is what we're looking for.Thanks for this! The help text will be fixed in v2.
None of the places I could find in the Linux
Documentation/devicetree/bindingsdirectory actually explains what
"shift the register offsets" means. Only the code reveals that
this means a LEFT shift of the register address (offset).At least one place in Linux interprets <reg-shift> differently; here it operates on the _data_ (instead there they use <reg-spacing>):
"
devicetree/bindings/ipmi/ipmi-smic.txt":
Quoted 8 lines
- reg-shift - The amount to shift the registers to the right to get the data into bit zero.
(see
ipmi_si_mem_io.cSo can we please add an explanation what _exactly_ this means?
I will propose one with v2 post.
Quoted 5 lines
Thanks.
Wolfgang Denk
regards, Felix
PATCH 1/1 serial: ns16550: Add register shift variable
In reply to Tom RiniQuoted 14 lines · Tom Rini
Quoted 13 lines · Wolfgang Denk
Dear Felix,
Quoted 10 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.
Quoted 5 lines
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
What does "differently" mean? Linux tries quite hard to be platform agnostic, so a per-build-target #define surely isn't what they're doing.
Are they inferring the shift from the compatible? In that case, we should do the same.
Alex
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 15 lines · Alexander Graf
Quoted 14 lines · Tom Rini
Quoted 13 lines · Wolfgang Denk
Dear Felix,
Quoted 10 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
What does "differently" mean? Linux tries quite hard to be platform agnostic, so a per-build-target #define surely isn't what they're doing.
Yes, what exactly is the Linux kernel doing here?
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180717/389d3f26/attachment.sig>
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 16 lines · Tom Rini
Quoted 15 lines · Alexander Graf
Quoted 14 lines · Tom Rini
Quoted 13 lines · Wolfgang Denk
Dear Felix,
Quoted 10 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.
Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
What does "differently" mean? Linux tries quite hard to be platform agnostic, so a per-build-target #define surely isn't what they're doing.
Yes, what exactly is the Linux kernel doing here?
Linux has a completely separate driver for omap3 (which is wrong too). But in a nutshell, it basically determines the shift value by the "compatible" string, so we should too.
Alex
PATCH 1/1 serial: ns16550: Add register shift variable
Hi Alexander, Tom,
Quoted 30 lines
-----Original Message-----
From: Alexander Graf [mailto:agraf at suse.de]
Sent: Tuesday, July 17, 2018 4:33 PM
To: Tom Rini <[email protected]>
Cc: Wolfgang Denk <[email protected]>; Felix Brack <[email protected]>; u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexey Brodkin <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableQuoted 21 lines · Tom Rini
Quoted 20 lines · Alexander Graf
Quoted 19 lines · Tom Rini
Quoted 13 lines · Wolfgang Denk
Dear Felix,
Quoted 10 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
Quoted 8 lines
Quoted 3 lines
Quoted 2 lines
What does "differently" mean? Linux tries quite hard to be platform agnostic, so a per-build-target #define surely isn't what they're doing.
Yes, what exactly is the Linux kernel doing here?
Linux has a completely separate driver for omap3 (which is wrong too). But in a nutshell, it basically determines the shift value by the "compatible" string, so we should too.
Here's the driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/omap-serial.c
-Alexey
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 37 lines · Alexey Brodkin
Hi Alexander, Tom,
Quoted 35 lines
-----Original Message-----
From: Alexander Graf [mailto:agraf at suse.de]
Sent: Tuesday, July 17, 2018 4:33 PM
To: Tom Rini <[email protected]>
Cc: Wolfgang Denk <[email protected]>; Felix Brack <[email protected]>; u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexey Brodkin <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableQuoted 24 lines · Tom Rini
Quoted 15 lines · Alexander Graf
Quoted 14 lines · Tom Rini
Quoted 13 lines · Wolfgang Denk
Dear Felix,
Quoted 10 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
What does "differently" mean? Linux tries quite hard to be platform agnostic, so a per-build-target #define surely isn't what they're doing.
Yes, what exactly is the Linux kernel doing here?
Quoted 6 lines
Quoted 3 lines
Linux has a completely separate driver for omap3 (which is wrong too). But in a nutshell, it basically determines the shift value by the "compatible" string, so we should too.
Here's the driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/omap-serial.c
But I would swear that's also not required and the generic ns1655x driver can be used.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180717/31fef243/attachment.sig>
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 37 lines · Tom Rini
Quoted 36 lines · Alexey Brodkin
Hi Alexander, Tom,
Quoted 34 lines
-----Original Message-----
From: Alexander Graf [mailto:agraf at suse.de]
Sent: Tuesday, July 17, 2018 4:33 PM
To: Tom Rini <[email protected]>
Cc: Wolfgang Denk <[email protected]>; Felix Brack <[email protected]>; u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexey Brodkin <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableQuoted 24 lines · Tom Rini
Quoted 15 lines · Alexander Graf
Quoted 14 lines · Tom Rini
Quoted 13 lines · Wolfgang Denk
Dear Felix,
Quoted 10 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
What does "differently" mean? Linux tries quite hard to be platform agnostic, so a per-build-target #define surely isn't what they're doing.
Yes, what exactly is the Linux kernel doing here?
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Linux has a completely separate driver for omap3 (which is wrong too). But in a nutshell, it basically determines the shift value by the "compatible" string, so we should too.
Here's the driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/omap-serial.cBut I would swear that's also not required and the generic ns1655x driver can be used.
I don't see the generic driver matching on the omap compatible string. So you'd have to convert the DT to the generic ns16650 binding (which would mean you also had to include the reg-shift property). I can't find any omap DT that does refer to its internal ns16650 by the generic binding though.
Again, maybe there's some additional functionality in that omap specific driver, like power management, that we don't care about at this point.
Alex
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 53 lines · Alexander Graf
Quoted 42 lines · Tom Rini
Quoted 41 lines · Alexey Brodkin
Hi Alexander, Tom,
Quoted 39 lines
-----Original Message-----
From: Alexander Graf [mailto:agraf at suse.de]
Sent: Tuesday, July 17, 2018 4:33 PM
To: Tom Rini <[email protected]>
Cc: Wolfgang Denk <[email protected]>; Felix Brack <[email protected]>; u-boot at lists.denx.de; Stefan Roese <[email protected]>; Alexey Brodkin <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variableQuoted 27 lines · Tom Rini
Quoted 18 lines · Alexander Graf
Quoted 17 lines · Tom Rini
Quoted 16 lines · Wolfgang Denk
Dear Felix,
In message <[email protected]> you
Quoted 10 lines · wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.Does this mean that U-Boot will not be able to use the same DTB as Linux?
Quoted 7 lines
Quoted 4 lines
To be clear, it's the other way around. We can't use the Linux dtb/dts
files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.What does "differently" mean? Linux tries quite hard to be platform agnostic, so a per-build-target #define surely isn't what they're doing.
Yes, what exactly is the Linux kernel doing here?
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Linux has a completely separate driver for omap3 (which is wrong too). But in a nutshell, it basically determines the shift value by the "compatible" string, so we should too.
Here's the driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/omap-serial.cBut I would swear that's also not required and the generic ns1655x driver can be used.
Quoted 9 lines
I don't see the generic driver matching on the omap compatible string. So you'd have to convert the DT to the generic ns16650 binding (which would mean you also had to include the reg-shift property). I can't find any omap DT that does refer to its internal ns16650 by the generic binding though.
Again, maybe there's some additional functionality in that omap specific driver, like power management, that we don't care about at this point.
omap uart do have the differences with ns16650. Affected to u-boot is the MDR1 register that enables the IP[1]. Right now the difference is guarded with OMAP2PLUS.
Thanks and regards,
Lokesh
PATCH 1/1 serial: ns16550: Add register shift variable
In reply to Alexey BrodkinQuoted 1 line · Alexey Brodkin
Quoted 1 line
Linux has a completely separate driver for omap3 (which is wrong too).
This is not (fully) correct,
Quoted 5 lines
Quoted 2 lines
But in a nutshell, it basically determines the shift value by the "compatible" string, so we should too.
Here's the driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/omap-serial.c
I dunno why it's still in the tree. The modern one is (check compatible strings for both):
https://elixir.bootlin.com/linux/v4.18-rc5/source/drivers/tty/serial/8250/8250_omap.c
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 11 lines · Andy Shevchenko
Quoted 1 line · Alexey Brodkin
Quoted 1 line
Linux has a completely separate driver for omap3 (which is wrong too).
This is not (fully) correct,
Quoted 3 lines
Quoted 2 lines
But in a nutshell, it basically determines the shift value by the "compatible" string, so we should too.
Quoted 9 lines
Quoted 2 lines
Here's the driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/omap-serial.cI dunno why it's still in the tree. The modern one is (check compatible strings for both):
https://elixir.bootlin.com/linux/v4.18-rc5/source/drivers/tty/serial/8250/8250_omap.c
Can we omap2+ users patch the 8250_omap.c code in Linux to make use of the reg-shift, then do the same from u-boot so they are in sync and clean out any #defines that could be replaced by the device tree?
(I am not volunteering anyone or myself, it's more of a hypothetical question)
adam
Quoted 9 lines
--
With Best Regards,
Andy Shevchenko
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot
PATCH 1/1 serial: ns16550: Add register shift variable
Quoted 8 lines · Adam Ford
Quoted 1 line · Andy Shevchenko
https://elixir.bootlin.com/linux/v4.18-rc5/source/drivers/tty/serial/8250/8250_omap.c
Can we omap2+ users patch the
8250_omap.ccode in Linux to make use of the reg-shift, then do the same from u-boot so they are in sync and clean out any #defines that could be replaced by the device tree?
It's possible, but it doesn't mean you now fully on DT, b/c Linux must support old DTBs as I already told in this thread.
So, the code in the driver will look like
ret = of_property_read_u32(..., ®_shift);
if (ret)
reg_shift = 2;
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH 1/1 serial: ns16550: Add register shift variable
In reply to Alexander GrafQuoted 22 lines · Alexander Graf
Quoted 21 lines · Tom Rini
Quoted 20 lines · Alexander Graf
Quoted 19 lines · Tom Rini
Quoted 13 lines · Wolfgang Denk
Dear Felix,
Quoted 10 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.Quoted 2 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
To be clear, it's the other way around. We can't use the Linux dtb/dts files as they've dropped (and in other cases, aren't adding) these properties as it's handled differently.
Quoted 7 lines
Quoted 3 lines
Quoted 2 lines
What does "differently" mean? Linux tries quite hard to be platform agnostic, so a per-build-target #define surely isn't what they're doing.
Yes, what exactly is the Linux kernel doing here?
Linux has a completely separate driver for omap3 (which is wrong too). But in a nutshell, it basically determines the shift value by the "compatible" string, so we should too.
Yes and no. For a number of years now the omap-only (ttyOx) driver has been deprecated and the generic ns1665x driver is valid. What does the ns1655x driver do here?
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180717/8411e0f6/attachment.sig>
PATCH 1/1 serial: ns16550: Add register shift variable
In reply to Wolfgang DenkHello Wolfgang,
Quoted 15 lines · Wolfgang Denk
Dear Felix,
Quoted 11 lines · In message <[email protected]> you wrote:
The motivation for writing this patch originates in the effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs. The current
am33xx.dtsifile from U-Boot defines the <reg-shift> property for all UART nodes. The actual (4.18+)am33xx.dtsifile from Linux does not define <reg-shift> anymore. To prevent (probably difficult) changes in many .dts and .dtsi files once the synchronization is done, one can use this new variable. For the pdu001 board, for example,SYS_NS16550_REG_SHIFTis set to 2; no need to clutter U-Boot and board specific dts files with <reg-shift> properties.
Quoted 3 lines
Does this mean that U-Boot will not be able to use the same DTB as Linux?
This has already been answered very accurately by Tom.
I have also written an RFC dealing with this: "[RFC] arm: dts: am33xx:
Sync DTS with Linux 4.16.11", posted May 24.
Quoted 16 lines
drivers/serial/Kconfig config SYS_NS16550 · line 541
Quoted 9 lines
drivers/serial/Kconfig +9
-0+config SYS_NS16550_REG_SHIFT + int "Number of bytes to shift register offsets" + default 0 + depends on SYS_NS16550 + help + Use this to specify the amount of bytes between discrete + device registers. If, for example, the device registers are + located at 0x00, 0x04, 0x08, 0x0C and so forth than set + this to 2. The default value is 0.This description is inconsistent or misleading. How do you define "space between registers"? The unused gaps? In the example, the registers are spaced at 4 bytes intervals, so a value of 2 would only make sense of we have 16 bit registters and you count the gap bytes.
Quoted 7 lines
But this is a very strange and uncommon way to describe such a situation, especially when you write that you "shift register offsets". Here I think about something like a LSL operation, so shifing by 2 bits would result in a 2^2 = 4 byte spacing.
This needs to be rewritten.
You are right, sorry. It seems I used my brain to mix up two different explanations and that's the result - will be fixed.
Quoted 5 lines
Best regards,
Wolfgang Denk
regards, Felix