[U-Boot] [PATCH] dm: pinctrl: Prevent (re-)configuring pins when already done before relocation
This commit prevents from re-configuring pins if those were configured before relocation.
Some pins - like UART or DDR must be setup before relocation (as they have 'u-boot,dm-pre-reloc' property set in DTS). Without this change, those pins are re-configured after relocation (pre_reloc_only = 0, so we do not "continue"). Such behavior may be a problem for DDR PAD configuration, as they might be already leveled/tuned with original setup).
Signed-off-by: Lukasz Majewski lukma@denx.de
---
drivers/pinctrl/pinctrl-uclass.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c index 6db0445067..29c910c55f 100644 --- a/drivers/pinctrl/pinctrl-uclass.c +++ b/drivers/pinctrl/pinctrl-uclass.c @@ -117,9 +117,9 @@ static int pinconfig_post_bind(struct udevice *dev) int ret;
dev_for_each_subnode(node, dev) { - if (pre_reloc_only && - !ofnode_pre_reloc(node)) + if (pre_reloc_only ^ ofnode_pre_reloc(node)) continue; + /* * If this node has "compatible" property, this is not * a pin configuration node, but a normal device. skip.
On Tue, Dec 18, 2018 at 12:30:50PM +0100, Lukasz Majewski wrote:
This commit prevents from re-configuring pins if those were configured before relocation.
Some pins - like UART or DDR must be setup before relocation (as they have 'u-boot,dm-pre-reloc' property set in DTS). Without this change, those pins are re-configured after relocation (pre_reloc_only = 0, so we do not "continue"). Such behavior may be a problem for DDR PAD configuration, as they might be already leveled/tuned with original setup).
Signed-off-by: Lukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!
On Thu, Dec 27, 2018 at 3:49 PM Tom Rini trini@konsulko.com wrote:
On Tue, Dec 18, 2018 at 12:30:50PM +0100, Lukasz Majewski wrote:
This commit prevents from re-configuring pins if those were configured before relocation.
Some pins - like UART or DDR must be setup before relocation (as they have 'u-boot,dm-pre-reloc' property set in DTS). Without this change, those pins are re-configured after relocation (pre_reloc_only = 0, so we do not "continue"). Such behavior may be a problem for DDR PAD configuration, as they might be already leveled/tuned with original setup).
Signed-off-by: Lukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!
I've bisected out to this commit and it's slightly broken things for me on an AM3352. It all works fine so long as I boot MLO from MMC (so the MMC is probed, pinctrl setup), but if I boot from UART then I get to full U-Boot, MMC hasn't been probed and the pinmuxing isn't set up for the MMC.
Hi Alex,
On Thu, Dec 27, 2018 at 3:49 PM Tom Rini trini@konsulko.com wrote:
On Tue, Dec 18, 2018 at 12:30:50PM +0100, Lukasz Majewski wrote:
This commit prevents from re-configuring pins if those were configured before relocation.
Some pins - like UART or DDR must be setup before relocation (as they have 'u-boot,dm-pre-reloc' property set in DTS). Without this change, those pins are re-configured after relocation (pre_reloc_only = 0, so we do not "continue"). Such behavior may be a problem for DDR PAD configuration, as they might be already leveled/tuned with original setup).
Signed-off-by: Lukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!
I've bisected out to this commit and it's slightly broken things for me on an AM3352. It all works fine so long as I boot MLO from MMC (so the MMC is probed, pinctrl setup), but if I boot from UART then I get to full U-Boot, MMC hasn't been probed and the pinmuxing isn't set up for the MMC.
I suppose that the pinmux node have set "u-boot,dm-pre-reloc" property?
The problem is not with lack of eMMC probing - it is with pinctrl nodes having "u-boot,dm-pre-reloc" set in DTS and the eMMC is probed after MLO/SPL. It looks like your use case implicitly depends on pinmux being reconfigured no matter if we are pre-relocated (MLO) or afterwards.
As stated in the commit message above - for DDR pads it is dangerous to re-configure them.
I'm wondering as in the device_probe() @ drivers/core/device.c the DM_FLAG_ACTIVATED is checked. This should be enough to prevent re-checking (of the DDR pins).
Anyway, this will not fix the issue you mentioned.
I've put Simon to CC, maybe he would have some input?
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Tue, Jan 8, 2019 at 10:52 PM Lukasz Majewski lukma@denx.de wrote:
Hi Alex,
On Thu, Dec 27, 2018 at 3:49 PM Tom Rini trini@konsulko.com wrote:
On Tue, Dec 18, 2018 at 12:30:50PM +0100, Lukasz Majewski wrote:
This commit prevents from re-configuring pins if those were configured before relocation.
Some pins - like UART or DDR must be setup before relocation (as they have 'u-boot,dm-pre-reloc' property set in DTS). Without this change, those pins are re-configured after relocation (pre_reloc_only = 0, so we do not "continue"). Such behavior may be a problem for DDR PAD configuration, as they might be already leveled/tuned with original setup).
Signed-off-by: Lukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!
I've bisected out to this commit and it's slightly broken things for me on an AM3352. It all works fine so long as I boot MLO from MMC (so the MMC is probed, pinctrl setup), but if I boot from UART then I get to full U-Boot, MMC hasn't been probed and the pinmuxing isn't set up for the MMC.
I suppose that the pinmux node have set "u-boot,dm-pre-reloc" property?
Yeah, well "u-boot,dm-spl", though now I re-read the docs I guess that's equivalent to "u-boot,dm-pre-reloc" as I've no TPL.
The problem is not with lack of eMMC probing - it is with pinctrl nodes having "u-boot,dm-pre-reloc" set in DTS and the eMMC is probed after MLO/SPL. It looks like your use case implicitly depends on pinmux being reconfigured no matter if we are pre-relocated (MLO) or afterwards.
Yes, that's a much clearer description...
Hi Alex,
On Tue, Jan 8, 2019 at 10:52 PM Lukasz Majewski lukma@denx.de wrote:
Hi Alex,
On Thu, Dec 27, 2018 at 3:49 PM Tom Rini trini@konsulko.com wrote:
On Tue, Dec 18, 2018 at 12:30:50PM +0100, Lukasz Majewski wrote:
This commit prevents from re-configuring pins if those were configured before relocation.
Some pins - like UART or DDR must be setup before relocation (as they have 'u-boot,dm-pre-reloc' property set in DTS). Without this change, those pins are re-configured after relocation (pre_reloc_only = 0, so we do not "continue"). Such behavior may be a problem for DDR PAD configuration, as they might be already leveled/tuned with original setup).
Signed-off-by: Lukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!
I've bisected out to this commit and it's slightly broken things for me on an AM3352. It all works fine so long as I boot MLO from MMC (so the MMC is probed, pinctrl setup), but if I boot from UART then I get to full U-Boot, MMC hasn't been probed and the pinmuxing isn't set up for the MMC.
I suppose that the pinmux node have set "u-boot,dm-pre-reloc" property?
Yeah, well "u-boot,dm-spl", though now I re-read the docs I guess that's equivalent to "u-boot,dm-pre-reloc" as I've no TPL.
I do need to check this -> as in my case I do have "u-boot,dm-pre-reloc" property. And yes, in my case I do not have TPL too (just SPL).
The problem is not with lack of eMMC probing - it is with pinctrl nodes having "u-boot,dm-pre-reloc" set in DTS and the eMMC is probed after MLO/SPL. It looks like your use case implicitly depends on pinmux being reconfigured no matter if we are pre-relocated (MLO) or afterwards.
Yes, that's a much clearer description...
Frankly, your mail opened my eyes to a use case which was handled (I assume on purpose) with the old code - the "cross" boot with pinmux and devices.
For example boot from UART (MLO/SPL) and then continue with eMMC (which require pinmux).
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Tue, Jan 8, 2019 at 11:24 PM Lukasz Majewski lukma@denx.de wrote:
Hi Alex,
On Tue, Jan 8, 2019 at 10:52 PM Lukasz Majewski lukma@denx.de wrote:
Hi Alex,
On Thu, Dec 27, 2018 at 3:49 PM Tom Rini trini@konsulko.com wrote:
On Tue, Dec 18, 2018 at 12:30:50PM +0100, Lukasz Majewski wrote:
This commit prevents from re-configuring pins if those were configured before relocation.
Some pins - like UART or DDR must be setup before relocation (as they have 'u-boot,dm-pre-reloc' property set in DTS). Without this change, those pins are re-configured after relocation (pre_reloc_only = 0, so we do not "continue"). Such behavior may be a problem for DDR PAD configuration, as they might be already leveled/tuned with original setup).
Signed-off-by: Lukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!
I've bisected out to this commit and it's slightly broken things for me on an AM3352. It all works fine so long as I boot MLO from MMC (so the MMC is probed, pinctrl setup), but if I boot from UART then I get to full U-Boot, MMC hasn't been probed and the pinmuxing isn't set up for the MMC.
I suppose that the pinmux node have set "u-boot,dm-pre-reloc" property?
Yeah, well "u-boot,dm-spl", though now I re-read the docs I guess that's equivalent to "u-boot,dm-pre-reloc" as I've no TPL.
I do need to check this -> as in my case I do have "u-boot,dm-pre-reloc" property. And yes, in my case I do not have TPL too (just SPL).
The problem is not with lack of eMMC probing - it is with pinctrl nodes having "u-boot,dm-pre-reloc" set in DTS and the eMMC is probed after MLO/SPL. It looks like your use case implicitly depends on pinmux being reconfigured no matter if we are pre-relocated (MLO) or afterwards.
Yes, that's a much clearer description...
Frankly, your mail opened my eyes to a use case which was handled (I assume on purpose) with the old code - the "cross" boot with pinmux and devices.
For example boot from UART (MLO/SPL) and then continue with eMMC (which require pinmux).
So I guess this needs something like a bloblist to communicate the initialisation state between MLO/SPL and u-boot, which I've not the faintest idea where to start on.
Horrible as they are, is a CONFIG symbol a possibility whilst we figure out how to solve this properly?
I guess another option would be an additional two properties that give the two behaviours, but that feels like something we don't want long term.
-- Alex Kiernan
Hi Alex,
On Tue, Jan 8, 2019 at 11:24 PM Lukasz Majewski lukma@denx.de wrote:
Hi Alex,
On Tue, Jan 8, 2019 at 10:52 PM Lukasz Majewski lukma@denx.de wrote:
Hi Alex,
On Thu, Dec 27, 2018 at 3:49 PM Tom Rini trini@konsulko.com wrote:
On Tue, Dec 18, 2018 at 12:30:50PM +0100, Lukasz Majewski wrote: > This commit prevents from re-configuring pins if those > were configured before relocation. > > Some pins - like UART or DDR must be setup before > relocation (as they have 'u-boot,dm-pre-reloc' property > set in DTS). Without this change, those pins are > re-configured after relocation (pre_reloc_only = 0, so we > do not "continue"). Such behavior may be a problem for > DDR PAD configuration, as they might be already > leveled/tuned with original setup). > > Signed-off-by: Lukasz Majewski lukma@denx.de
Applied to u-boot/master, thanks!
I've bisected out to this commit and it's slightly broken things for me on an AM3352. It all works fine so long as I boot MLO from MMC (so the MMC is probed, pinctrl setup), but if I boot from UART then I get to full U-Boot, MMC hasn't been probed and the pinmuxing isn't set up for the MMC.
I suppose that the pinmux node have set "u-boot,dm-pre-reloc" property?
Yeah, well "u-boot,dm-spl", though now I re-read the docs I guess that's equivalent to "u-boot,dm-pre-reloc" as I've no TPL.
I do need to check this -> as in my case I do have "u-boot,dm-pre-reloc" property. And yes, in my case I do not have TPL too (just SPL).
The problem is not with lack of eMMC probing - it is with pinctrl nodes having "u-boot,dm-pre-reloc" set in DTS and the eMMC is probed after MLO/SPL. It looks like your use case implicitly depends on pinmux being reconfigured no matter if we are pre-relocated (MLO) or afterwards.
Yes, that's a much clearer description...
Frankly, your mail opened my eyes to a use case which was handled (I assume on purpose) with the old code - the "cross" boot with pinmux and devices.
For example boot from UART (MLO/SPL) and then continue with eMMC (which require pinmux).
So I guess this needs something like a bloblist to communicate the initialisation state between MLO/SPL and u-boot,
Yes. IMHO this is one way to do it. There is already a DM_FLAG_ACTIVATED, which indicates if the device was probed. However, it is not "dragged" from MLO / SPL to u-boot.img
However, I think that we shall introduce a new flag - DM_FLAG_CONFIGURE_ONCE [*], which would indicate that the device need single configuration.
This flag could be used with DM_PINMUX driver, to avoid re-configuration.
The idea would be to use bloblist (only when [*] is set) to pass tuples: (driver_name, config_status)
and in the core of DM just check if we shall re-configure the driver or not.
which I've not the faintest idea where to start on.
Horrible as they are, is a CONFIG symbol a possibility whilst we figure out how to solve this properly?
The bloblist can be activated with CONFIG_BLOBLIST
I guess another option would be an additional two properties that give the two behaviours, but that feels like something we don't want long term.
As we strive to use as little "u-boot,XXXXXX" extra properties to avoid problems with Linux re-sync, I think that we shall use bloblist.
-- Alex Kiernan
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi Lukasz,
On Sat, 12 Jan 2019 at 14:46, Lukasz Majewski lukma@denx.de wrote:
Hi Alex,
On Tue, Jan 8, 2019 at 11:24 PM Lukasz Majewski lukma@denx.de wrote:
Hi Alex,
On Tue, Jan 8, 2019 at 10:52 PM Lukasz Majewski lukma@denx.de wrote:
Hi Alex,
On Thu, Dec 27, 2018 at 3:49 PM Tom Rini trini@konsulko.com wrote: > > On Tue, Dec 18, 2018 at 12:30:50PM +0100, Lukasz Majewski > wrote: > > This commit prevents from re-configuring pins if those > > were configured before relocation. > > > > Some pins - like UART or DDR must be setup before > > relocation (as they have 'u-boot,dm-pre-reloc' property > > set in DTS). Without this change, those pins are > > re-configured after relocation (pre_reloc_only = 0, so we > > do not "continue"). Such behavior may be a problem for > > DDR PAD configuration, as they might be already > > leveled/tuned with original setup). > > > > Signed-off-by: Lukasz Majewski lukma@denx.de > > Applied to u-boot/master, thanks! >
I've bisected out to this commit and it's slightly broken things for me on an AM3352. It all works fine so long as I boot MLO from MMC (so the MMC is probed, pinctrl setup), but if I boot from UART then I get to full U-Boot, MMC hasn't been probed and the pinmuxing isn't set up for the MMC.
I suppose that the pinmux node have set "u-boot,dm-pre-reloc" property?
Yeah, well "u-boot,dm-spl", though now I re-read the docs I guess that's equivalent to "u-boot,dm-pre-reloc" as I've no TPL.
I do need to check this -> as in my case I do have "u-boot,dm-pre-reloc" property. And yes, in my case I do not have TPL too (just SPL).
The problem is not with lack of eMMC probing - it is with pinctrl nodes having "u-boot,dm-pre-reloc" set in DTS and the eMMC is probed after MLO/SPL. It looks like your use case implicitly depends on pinmux being reconfigured no matter if we are pre-relocated (MLO) or afterwards.
Yes, that's a much clearer description...
Frankly, your mail opened my eyes to a use case which was handled (I assume on purpose) with the old code - the "cross" boot with pinmux and devices.
For example boot from UART (MLO/SPL) and then continue with eMMC (which require pinmux).
So I guess this needs something like a bloblist to communicate the initialisation state between MLO/SPL and u-boot,
Yes. IMHO this is one way to do it. There is already a DM_FLAG_ACTIVATED, which indicates if the device was probed. However, it is not "dragged" from MLO / SPL to u-boot.img
However, I think that we shall introduce a new flag - DM_FLAG_CONFIGURE_ONCE [*], which would indicate that the device need single configuration.
This flag could be used with DM_PINMUX driver, to avoid re-configuration.
The idea would be to use bloblist (only when [*] is set) to pass tuples: (driver_name, config_status)
That seems reasonable, but I don't like the idea of using a name. Could we have some sort of numeric value instead?
and in the core of DM just check if we shall re-configure the driver or not.
which I've not the faintest idea where to start on.
Horrible as they are, is a CONFIG symbol a possibility whilst we figure out how to solve this properly?
The bloblist can be activated with CONFIG_BLOBLIST
I guess another option would be an additional two properties that give the two behaviours, but that feels like something we don't want long term.
As we strive to use as little "u-boot,XXXXXX" extra properties to avoid problems with Linux re-sync, I think that we shall use bloblist.
Regards, Simon
participants (4)
-
Alex Kiernan -
Lukasz Majewski -
Simon Glass -
Tom Rini