mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/3] arm: rockchip: minor fixes as preparation for mainline dts
@ 2021-11-11 14:03 Michael Riesch
  2021-11-11 14:03 ` [PATCH 1/3] arm: rockchip: rk3568: fix mmc boot source instances Michael Riesch
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Michael Riesch @ 2021-11-11 14:03 UTC (permalink / raw)
  To: barebox; +Cc: Michael Riesch

Hi all,

These patches fix bugs that are bound to pop up once the mainline
device tree for the Rockchip RK356x are pulled into barebox.

Tested on a Pine64 Quartz64 board, further tests on a RK3568 EVB
will follow tomorrow.

Best regards,
Michael

Michael Riesch (3):
  arm: rockchip: rk3568: fix mmc boot source instances
  net: designware: rockchip: remove unnecessary clock pclk_xpcs
  pinctrl: rockchip: use alias rather than full of name

 arch/arm/mach-rockchip/rk3568.c    | 4 ++--
 drivers/net/designware_rockchip.c  | 2 --
 drivers/pinctrl/pinctrl-rockchip.c | 2 +-
 3 files changed, 3 insertions(+), 5 deletions(-)

-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] arm: rockchip: rk3568: fix mmc boot source instances
  2021-11-11 14:03 [PATCH 0/3] arm: rockchip: minor fixes as preparation for mainline dts Michael Riesch
@ 2021-11-11 14:03 ` Michael Riesch
  2021-11-15  7:51   ` Sascha Hauer
  2021-11-11 14:03 ` [PATCH 2/3] net: designware: rockchip: remove unnecessary clock pclk_xpcs Michael Riesch
  2021-11-11 14:03 ` [PATCH 3/3] pinctrl: rockchip: use alias rather than full of name Michael Riesch
  2 siblings, 1 reply; 14+ messages in thread
From: Michael Riesch @ 2021-11-11 14:03 UTC (permalink / raw)
  To: barebox; +Cc: Michael Riesch

The mainline DTS for the RK3568 EVB1 introduces mmc aliases sorted
by the addresses of the corresponding controller. This commit
fixes the instance number and aligns it with these aliases.

Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
 arch/arm/mach-rockchip/rk3568.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-rockchip/rk3568.c b/arch/arm/mach-rockchip/rk3568.c
index 234c6d22d..95f462eca 100644
--- a/arch/arm/mach-rockchip/rk3568.c
+++ b/arch/arm/mach-rockchip/rk3568.c
@@ -144,10 +144,10 @@ struct rk_bootsource {
 
 static struct rk_bootsource bootdev_map[] = {
 	[0x1] = { .src = BOOTSOURCE_NAND, .instance = 0 },
-	[0x2] = { .src = BOOTSOURCE_MMC, .instance = 0 },
+	[0x2] = { .src = BOOTSOURCE_MMC, .instance = 1 },
 	[0x3] = { .src = BOOTSOURCE_SPI_NOR, .instance = 0 },
 	[0x4] = { .src = BOOTSOURCE_SPI_NAND, .instance = 0 },
-	[0x5] = { .src = BOOTSOURCE_MMC, .instance = 1 },
+	[0x5] = { .src = BOOTSOURCE_MMC, .instance = 0 },
 	[0xa] = { .src = BOOTSOURCE_USB, .instance = 0 },
 };
 
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/3] net: designware: rockchip: remove unnecessary clock pclk_xpcs
  2021-11-11 14:03 [PATCH 0/3] arm: rockchip: minor fixes as preparation for mainline dts Michael Riesch
  2021-11-11 14:03 ` [PATCH 1/3] arm: rockchip: rk3568: fix mmc boot source instances Michael Riesch
@ 2021-11-11 14:03 ` Michael Riesch
  2022-01-14  5:24   ` Ahmad Fatoum
  2021-11-11 14:03 ` [PATCH 3/3] pinctrl: rockchip: use alias rather than full of name Michael Riesch
  2 siblings, 1 reply; 14+ messages in thread
From: Michael Riesch @ 2021-11-11 14:03 UTC (permalink / raw)
  To: barebox; +Cc: Michael Riesch

The pclk_xpcs clock is not used in the mainline Linux driver and
only rarely specified in the mainline dts files. Drop it in order
to avoid errors when the mainline dts files are used.

Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
 drivers/net/designware_rockchip.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/designware_rockchip.c b/drivers/net/designware_rockchip.c
index e4f74f646..7acbd723b 100644
--- a/drivers/net/designware_rockchip.c
+++ b/drivers/net/designware_rockchip.c
@@ -44,7 +44,6 @@ enum {
 	CLK_MAC_PCLK,
 	CLK_MAC_SPEED,
 	CLK_PTP_REF,
-	CLK_XPCS_PCLK,
 };
 
 static const struct clk_bulk_data rk_gmac_clks[] = {
@@ -56,7 +55,6 @@ static const struct clk_bulk_data rk_gmac_clks[] = {
 	[CLK_MAC_PCLK]    = { .id = "pclk_mac" },
 	[CLK_MAC_SPEED]   = { .id = "clk_mac_speed" },
 	[CLK_PTP_REF]     = { .id = "ptp_ref" },
-	[CLK_XPCS_PCLK]   = { .id = "pclk_xpcs" },
 };
 
 static inline struct eqos_rk_gmac *to_rk_gmac(struct eqos *eqos)
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/3] pinctrl: rockchip: use alias rather than full of name
  2021-11-11 14:03 [PATCH 0/3] arm: rockchip: minor fixes as preparation for mainline dts Michael Riesch
  2021-11-11 14:03 ` [PATCH 1/3] arm: rockchip: rk3568: fix mmc boot source instances Michael Riesch
  2021-11-11 14:03 ` [PATCH 2/3] net: designware: rockchip: remove unnecessary clock pclk_xpcs Michael Riesch
@ 2021-11-11 14:03 ` Michael Riesch
  2021-11-15  7:54   ` Sascha Hauer
  2 siblings, 1 reply; 14+ messages in thread
From: Michael Riesch @ 2021-11-11 14:03 UTC (permalink / raw)
  To: barebox; +Cc: Michael Riesch

Do not rely on GPIO controllers being named "gpioX". Instead, use the
alias to match the controllers to the GPIO banks.

Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
 drivers/pinctrl/pinctrl-rockchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 1fdb9a913..0e706c51d 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -888,7 +888,7 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data(
 		bank = ctrl->pin_banks;
 		for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
 			name = bank->name;
-			if (!strncmp(name, np->name, strlen(name))) {
+			if (!strncmp(name, of_alias_get(np), strlen(name))) {
 				bank->of_node = np;
 				if (!rockchip_get_bank_data(bank, dev))
 					bank->valid = true;
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] arm: rockchip: rk3568: fix mmc boot source instances
  2021-11-11 14:03 ` [PATCH 1/3] arm: rockchip: rk3568: fix mmc boot source instances Michael Riesch
@ 2021-11-15  7:51   ` Sascha Hauer
  2021-11-15  8:06     ` Sascha Hauer
  0 siblings, 1 reply; 14+ messages in thread
From: Sascha Hauer @ 2021-11-15  7:51 UTC (permalink / raw)
  To: Michael Riesch; +Cc: barebox

On Thu, Nov 11, 2021 at 03:03:14PM +0100, Michael Riesch wrote:
> The mainline DTS for the RK3568 EVB1 introduces mmc aliases sorted
> by the addresses of the corresponding controller. This commit
> fixes the instance number and aligns it with these aliases.

The board dts sorts them differently, but the file is a SoC specific
one. We have a problem here.

Sascha

> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  arch/arm/mach-rockchip/rk3568.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/rk3568.c b/arch/arm/mach-rockchip/rk3568.c
> index 234c6d22d..95f462eca 100644
> --- a/arch/arm/mach-rockchip/rk3568.c
> +++ b/arch/arm/mach-rockchip/rk3568.c
> @@ -144,10 +144,10 @@ struct rk_bootsource {
>  
>  static struct rk_bootsource bootdev_map[] = {
>  	[0x1] = { .src = BOOTSOURCE_NAND, .instance = 0 },
> -	[0x2] = { .src = BOOTSOURCE_MMC, .instance = 0 },
> +	[0x2] = { .src = BOOTSOURCE_MMC, .instance = 1 },
>  	[0x3] = { .src = BOOTSOURCE_SPI_NOR, .instance = 0 },
>  	[0x4] = { .src = BOOTSOURCE_SPI_NAND, .instance = 0 },
> -	[0x5] = { .src = BOOTSOURCE_MMC, .instance = 1 },
> +	[0x5] = { .src = BOOTSOURCE_MMC, .instance = 0 },
>  	[0xa] = { .src = BOOTSOURCE_USB, .instance = 0 },
>  };
>  
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] pinctrl: rockchip: use alias rather than full of name
  2021-11-11 14:03 ` [PATCH 3/3] pinctrl: rockchip: use alias rather than full of name Michael Riesch
@ 2021-11-15  7:54   ` Sascha Hauer
  2021-11-17 12:42     ` Michael Riesch
  0 siblings, 1 reply; 14+ messages in thread
From: Sascha Hauer @ 2021-11-15  7:54 UTC (permalink / raw)
  To: Michael Riesch; +Cc: barebox

On Thu, Nov 11, 2021 at 03:03:16PM +0100, Michael Riesch wrote:
> Do not rely on GPIO controllers being named "gpioX". Instead, use the
> alias to match the controllers to the GPIO banks.
> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  drivers/pinctrl/pinctrl-rockchip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 1fdb9a913..0e706c51d 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -888,7 +888,7 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data(
>  		bank = ctrl->pin_banks;
>  		for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
>  			name = bank->name;
> -			if (!strncmp(name, np->name, strlen(name))) {
> +			if (!strncmp(name, of_alias_get(np), strlen(name))) {

Please use of_alias_get_id(np, "gpio");

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] arm: rockchip: rk3568: fix mmc boot source instances
  2021-11-15  7:51   ` Sascha Hauer
@ 2021-11-15  8:06     ` Sascha Hauer
  2021-11-15  9:24       ` Michael Riesch
  0 siblings, 1 reply; 14+ messages in thread
From: Sascha Hauer @ 2021-11-15  8:06 UTC (permalink / raw)
  To: Michael Riesch; +Cc: barebox

On Mon, Nov 15, 2021 at 08:51:16AM +0100, Sascha Hauer wrote:
> On Thu, Nov 11, 2021 at 03:03:14PM +0100, Michael Riesch wrote:
> > The mainline DTS for the RK3568 EVB1 introduces mmc aliases sorted
> > by the addresses of the corresponding controller. This commit
> > fixes the instance number and aligns it with these aliases.
> 
> The board dts sorts them differently, but the file is a SoC specific
> one. We have a problem here.
> 
> Sascha
> 
> > 
> > Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> > ---
> >  arch/arm/mach-rockchip/rk3568.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/mach-rockchip/rk3568.c b/arch/arm/mach-rockchip/rk3568.c
> > index 234c6d22d..95f462eca 100644
> > --- a/arch/arm/mach-rockchip/rk3568.c
> > +++ b/arch/arm/mach-rockchip/rk3568.c
> > @@ -144,10 +144,10 @@ struct rk_bootsource {
> >  
> >  static struct rk_bootsource bootdev_map[] = {
> >  	[0x1] = { .src = BOOTSOURCE_NAND, .instance = 0 },
> > -	[0x2] = { .src = BOOTSOURCE_MMC, .instance = 0 },
> > +	[0x2] = { .src = BOOTSOURCE_MMC, .instance = 1 },
> >  	[0x3] = { .src = BOOTSOURCE_SPI_NOR, .instance = 0 },
> >  	[0x4] = { .src = BOOTSOURCE_SPI_NAND, .instance = 0 },
> > -	[0x5] = { .src = BOOTSOURCE_MMC, .instance = 1 },
> > +	[0x5] = { .src = BOOTSOURCE_MMC, .instance = 0 },

Instead of storing the .src and .instance directly here we could store
the base address of the peripheral here. Then search in the device tree
for the node with that address and get the corresponding alias.

We would then have to translate this into our BOOTSOURCE_ defines and
instance numbers. Or maybe it was a bad idea to have defines for these
and we should have used strings for the bootsources in the first place.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] arm: rockchip: rk3568: fix mmc boot source instances
  2021-11-15  8:06     ` Sascha Hauer
@ 2021-11-15  9:24       ` Michael Riesch
  2021-11-17 14:24         ` [RFC PATCH] bootsource: add helper to set instance by name Michael Riesch
  0 siblings, 1 reply; 14+ messages in thread
From: Michael Riesch @ 2021-11-15  9:24 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hello Sascha,

On 11/15/21 9:06 AM, Sascha Hauer wrote:
> On Mon, Nov 15, 2021 at 08:51:16AM +0100, Sascha Hauer wrote:
>> On Thu, Nov 11, 2021 at 03:03:14PM +0100, Michael Riesch wrote:
>>> The mainline DTS for the RK3568 EVB1 introduces mmc aliases sorted
>>> by the addresses of the corresponding controller. This commit
>>> fixes the instance number and aligns it with these aliases.
>>
>> The board dts sorts them differently, but the file is a SoC specific
>> one. We have a problem here.

Indeed. FWIW Ahmad and I had a discussion about this topic in which
having the mmc aliases in the SoC dtsi file was suggested as nice
solution. However, the ARM SoC community is about to move the aliases to
the board files and the corresponding patch was declined [0].
Hence, currently we rely on the board dts files to be consistent. At the
moment this is the case, but I take it that you are not too fond of this
approach.
BTW if I am not mistaken, this is an issue on other platforms as well,
e.g., on the ZynqMP machines.

>> Sascha
>>
>>>
>>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
>>> ---
>>>  arch/arm/mach-rockchip/rk3568.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-rockchip/rk3568.c b/arch/arm/mach-rockchip/rk3568.c
>>> index 234c6d22d..95f462eca 100644
>>> --- a/arch/arm/mach-rockchip/rk3568.c
>>> +++ b/arch/arm/mach-rockchip/rk3568.c
>>> @@ -144,10 +144,10 @@ struct rk_bootsource {
>>>  
>>>  static struct rk_bootsource bootdev_map[] = {
>>>  	[0x1] = { .src = BOOTSOURCE_NAND, .instance = 0 },
>>> -	[0x2] = { .src = BOOTSOURCE_MMC, .instance = 0 },
>>> +	[0x2] = { .src = BOOTSOURCE_MMC, .instance = 1 },
>>>  	[0x3] = { .src = BOOTSOURCE_SPI_NOR, .instance = 0 },
>>>  	[0x4] = { .src = BOOTSOURCE_SPI_NAND, .instance = 0 },
>>> -	[0x5] = { .src = BOOTSOURCE_MMC, .instance = 1 },
>>> +	[0x5] = { .src = BOOTSOURCE_MMC, .instance = 0 },
> 
> Instead of storing the .src and .instance directly here we could store
> the base address of the peripheral here. Then search in the device tree
> for the node with that address and get the corresponding alias.

Sounds good to me.

> We would then have to translate this into our BOOTSOURCE_ defines and
> instance numbers. Or maybe it was a bad idea to have defines for these
> and we should have used strings for the bootsources in the first place.

AFAIC having a clear classification of the boot sources with enums is
helpful, and it is just the link to the actual device that might need a
revision.

I might give this a spin and come up with something.

Best regards,
Michael

[0]
https://lore.kernel.org/all/20210917110528.24454-1-michael.riesch@wolfvision.net/

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] pinctrl: rockchip: use alias rather than full of name
  2021-11-15  7:54   ` Sascha Hauer
@ 2021-11-17 12:42     ` Michael Riesch
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Riesch @ 2021-11-17 12:42 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hello Sascha,

On 11/15/21 8:54 AM, Sascha Hauer wrote:
> On Thu, Nov 11, 2021 at 03:03:16PM +0100, Michael Riesch wrote:
>> Do not rely on GPIO controllers being named "gpioX". Instead, use the
>> alias to match the controllers to the GPIO banks.
>>
>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
>> ---
>>   drivers/pinctrl/pinctrl-rockchip.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
>> index 1fdb9a913..0e706c51d 100644
>> --- a/drivers/pinctrl/pinctrl-rockchip.c
>> +++ b/drivers/pinctrl/pinctrl-rockchip.c
>> @@ -888,7 +888,7 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data(
>>   		bank = ctrl->pin_banks;
>>   		for (i = 0; i < ctrl->nr_banks; ++i, ++bank) {
>>   			name = bank->name;
>> -			if (!strncmp(name, np->name, strlen(name))) {
>> +			if (!strncmp(name, of_alias_get(np), strlen(name))) {
> 
> Please use of_alias_get_id(np, "gpio");

OK! v2 in preparation.

Best regards,
Michael

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH] bootsource: add helper to set instance by name
  2021-11-15  9:24       ` Michael Riesch
@ 2021-11-17 14:24         ` Michael Riesch
  2022-01-14  8:25           ` Michael Riesch
  2022-01-14  9:15           ` Sascha Hauer
  0 siblings, 2 replies; 14+ messages in thread
From: Michael Riesch @ 2021-11-17 14:24 UTC (permalink / raw)
  To: barebox; +Cc: sha, Michael Riesch

Instance numbers should be related to device tree aliases, which may be
board-specific. In order to establish a board-independent link between
the boot source and the actual alias, introduce a helper that sets the
instance by the OF name.

Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
 common/bootsource.c  | 74 ++++++++++++++++++++++++++++----------------
 include/bootsource.h |  1 +
 2 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/common/bootsource.c b/common/bootsource.c
index 1f8d053a8..1b45956a5 100644
--- a/common/bootsource.c
+++ b/common/bootsource.c
@@ -9,6 +9,7 @@
 #include <environment.h>
 #include <magicvar.h>
 #include <init.h>
+#include <of.h>
 
 static const char *bootsource_str[] = {
 	[BOOTSOURCE_UNKNOWN] = "unknown",
@@ -33,31 +34,8 @@ static enum bootsource bootsource = BOOTSOURCE_UNKNOWN;
 static int bootsource_instance = BOOTSOURCE_INSTANCE_UNKNOWN;
 const char *bootsource_alias_name = NULL;
 
-/**
- * bootsource_get_alias_name() - Get the name of the bootsource alias
- *
- * This function will return newly allocated string containing name of
- * the alias that is expected to point to DTB node corresponding to
- * detected bootsource
- *
- * NOTE: Caller is expected to free() the string allocated by this
- * function
- */
-char *bootsource_get_alias_name(void)
+static const char *bootsource_get_of_stem(void)
 {
-	const char *stem;
-
-	/*
-	 * If alias name was overridden via
-	 * bootsource_set_alias_name() return that value without
-	 * asking any questions.
-	 *
-	 * Note that we have to strdup() the result to make it
-	 * free-able.
-	 */
-	if (bootsource_alias_name)
-		return strdup(bootsource_alias_name);
-
 	switch (bootsource) {
 		/*
 		 * For I2C and SPI EEPROMs we set the stem to be 'i2c'
@@ -69,22 +47,50 @@ char *bootsource_get_alias_name(void)
 		 * controller
 		 */
 	case BOOTSOURCE_I2C_EEPROM:
-		stem = bootsource_str[BOOTSOURCE_I2C];
+		return bootsource_str[BOOTSOURCE_I2C];
 		break;
 	case BOOTSOURCE_SPI_EEPROM:
 	case BOOTSOURCE_SPI_NOR:
-		stem = bootsource_str[BOOTSOURCE_SPI];
+		return bootsource_str[BOOTSOURCE_SPI];
 		break;
 	case BOOTSOURCE_SERIAL:	/* FALLTHROUGH */
 	case BOOTSOURCE_I2C:	/* FALLTHROUGH */
 	case BOOTSOURCE_MMC:	/* FALLTHROUGH */
 	case BOOTSOURCE_SPI:	/* FALLTHROUGH */
 	case BOOTSOURCE_CAN:
-		stem = bootsource_str[bootsource];
+		return bootsource_str[bootsource];
 		break;
 	default:
 		return NULL;
 	}
+}
+
+/**
+ * bootsource_get_alias_name() - Get the name of the bootsource alias
+ *
+ * This function will return newly allocated string containing name of
+ * the alias that is expected to point to DTB node corresponding to
+ * detected bootsource
+ *
+ * NOTE: Caller is expected to free() the string allocated by this
+ * function
+ */
+char *bootsource_get_alias_name(void)
+{
+	const char *stem;
+
+	/*
+	 * If alias name was overridden via
+	 * bootsource_set_alias_name() return that value without
+	 * asking any questions.
+	 *
+	 * Note that we have to strdup() the result to make it
+	 * free-able.
+	 */
+	if (bootsource_alias_name)
+		return strdup(bootsource_alias_name);
+
+	stem = bootsource_get_of_stem();
 
 	/*
 	 * We expect SoC specific bootsource detection code to properly
@@ -125,6 +131,20 @@ void bootsource_set_instance(int instance)
 	setenv("bootsource_instance", buf);
 }
 
+void bootsource_set_instance_by_of_name(const char *name)
+{
+	int instance = BOOTSOURCE_UNKNOWN;
+	struct device_node *node;
+
+	node = of_find_node_by_name(of_get_root_node(), name);
+	if (node) {
+		instance = of_alias_get_id(node, bootsource_get_of_stem());
+		if (instance < 0)
+			instance = BOOTSOURCE_UNKNOWN;
+	}
+	bootsource_set_instance(instance);
+}
+
 enum bootsource bootsource_get(void)
 {
 	return bootsource;
diff --git a/include/bootsource.h b/include/bootsource.h
index 646b0e91c..4dcb969ac 100644
--- a/include/bootsource.h
+++ b/include/bootsource.h
@@ -28,6 +28,7 @@ enum bootsource bootsource_get(void);
 int bootsource_get_instance(void);
 void bootsource_set(enum bootsource src);
 void bootsource_set_instance(int instance);
+void bootsource_set_instance_by_of_name(const char *name);
 void bootsource_set_alias_name(const char *name);
 char *bootsource_get_alias_name(void);
 
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] net: designware: rockchip: remove unnecessary clock pclk_xpcs
  2021-11-11 14:03 ` [PATCH 2/3] net: designware: rockchip: remove unnecessary clock pclk_xpcs Michael Riesch
@ 2022-01-14  5:24   ` Ahmad Fatoum
  2022-01-14  8:16     ` Sascha Hauer
  0 siblings, 1 reply; 14+ messages in thread
From: Ahmad Fatoum @ 2022-01-14  5:24 UTC (permalink / raw)
  To: Michael Riesch, barebox, Sascha Hauer

Hello Sascha,

On 11.11.21 15:03, Michael Riesch wrote:
> The pclk_xpcs clock is not used in the mainline Linux driver and
> only rarely specified in the mainline dts files. Drop it in order
> to avoid errors when the mainline dts files are used.
> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>

Can you cherry-pick this one patch? I have code depending on it.

> ---
>  drivers/net/designware_rockchip.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/designware_rockchip.c b/drivers/net/designware_rockchip.c
> index e4f74f646..7acbd723b 100644
> --- a/drivers/net/designware_rockchip.c
> +++ b/drivers/net/designware_rockchip.c
> @@ -44,7 +44,6 @@ enum {
>  	CLK_MAC_PCLK,
>  	CLK_MAC_SPEED,
>  	CLK_PTP_REF,
> -	CLK_XPCS_PCLK,
>  };
>  
>  static const struct clk_bulk_data rk_gmac_clks[] = {
> @@ -56,7 +55,6 @@ static const struct clk_bulk_data rk_gmac_clks[] = {
>  	[CLK_MAC_PCLK]    = { .id = "pclk_mac" },
>  	[CLK_MAC_SPEED]   = { .id = "clk_mac_speed" },
>  	[CLK_PTP_REF]     = { .id = "ptp_ref" },
> -	[CLK_XPCS_PCLK]   = { .id = "pclk_xpcs" },
>  };
>  
>  static inline struct eqos_rk_gmac *to_rk_gmac(struct eqos *eqos)
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] net: designware: rockchip: remove unnecessary clock pclk_xpcs
  2022-01-14  5:24   ` Ahmad Fatoum
@ 2022-01-14  8:16     ` Sascha Hauer
  0 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2022-01-14  8:16 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Michael Riesch, barebox

On Fri, Jan 14, 2022 at 06:24:00AM +0100, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 11.11.21 15:03, Michael Riesch wrote:
> > The pclk_xpcs clock is not used in the mainline Linux driver and
> > only rarely specified in the mainline dts files. Drop it in order
> > to avoid errors when the mainline dts files are used.
> > 
> > Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> 
> Can you cherry-pick this one patch? I have code depending on it.

Did that.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] bootsource: add helper to set instance by name
  2021-11-17 14:24         ` [RFC PATCH] bootsource: add helper to set instance by name Michael Riesch
@ 2022-01-14  8:25           ` Michael Riesch
  2022-01-14  9:15           ` Sascha Hauer
  1 sibling, 0 replies; 14+ messages in thread
From: Michael Riesch @ 2022-01-14  8:25 UTC (permalink / raw)
  To: barebox; +Cc: sha, a.fatoum

Hi Sascha and Ahmad,

On 11/17/21 15:24, Michael Riesch wrote:
> Instance numbers should be related to device tree aliases, which may be
> board-specific. In order to establish a board-independent link between
> the boot source and the actual alias, introduce a helper that sets the
> instance by the OF name.

Gentle ping here. A rough indication whether or not this is the correct
path would be great.

Since patch 2/3 has just been cherry-picked, I could prepare a v2 of the
remaining two patches as next step.

Best regards,
Michael

> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  common/bootsource.c  | 74 ++++++++++++++++++++++++++++----------------
>  include/bootsource.h |  1 +
>  2 files changed, 48 insertions(+), 27 deletions(-)
> 
> diff --git a/common/bootsource.c b/common/bootsource.c
> index 1f8d053a8..1b45956a5 100644
> --- a/common/bootsource.c
> +++ b/common/bootsource.c
> @@ -9,6 +9,7 @@
>  #include <environment.h>
>  #include <magicvar.h>
>  #include <init.h>
> +#include <of.h>
>  
>  static const char *bootsource_str[] = {
>  	[BOOTSOURCE_UNKNOWN] = "unknown",
> @@ -33,31 +34,8 @@ static enum bootsource bootsource = BOOTSOURCE_UNKNOWN;
>  static int bootsource_instance = BOOTSOURCE_INSTANCE_UNKNOWN;
>  const char *bootsource_alias_name = NULL;
>  
> -/**
> - * bootsource_get_alias_name() - Get the name of the bootsource alias
> - *
> - * This function will return newly allocated string containing name of
> - * the alias that is expected to point to DTB node corresponding to
> - * detected bootsource
> - *
> - * NOTE: Caller is expected to free() the string allocated by this
> - * function
> - */
> -char *bootsource_get_alias_name(void)
> +static const char *bootsource_get_of_stem(void)
>  {
> -	const char *stem;
> -
> -	/*
> -	 * If alias name was overridden via
> -	 * bootsource_set_alias_name() return that value without
> -	 * asking any questions.
> -	 *
> -	 * Note that we have to strdup() the result to make it
> -	 * free-able.
> -	 */
> -	if (bootsource_alias_name)
> -		return strdup(bootsource_alias_name);
> -
>  	switch (bootsource) {
>  		/*
>  		 * For I2C and SPI EEPROMs we set the stem to be 'i2c'
> @@ -69,22 +47,50 @@ char *bootsource_get_alias_name(void)
>  		 * controller
>  		 */
>  	case BOOTSOURCE_I2C_EEPROM:
> -		stem = bootsource_str[BOOTSOURCE_I2C];
> +		return bootsource_str[BOOTSOURCE_I2C];
>  		break;
>  	case BOOTSOURCE_SPI_EEPROM:
>  	case BOOTSOURCE_SPI_NOR:
> -		stem = bootsource_str[BOOTSOURCE_SPI];
> +		return bootsource_str[BOOTSOURCE_SPI];
>  		break;
>  	case BOOTSOURCE_SERIAL:	/* FALLTHROUGH */
>  	case BOOTSOURCE_I2C:	/* FALLTHROUGH */
>  	case BOOTSOURCE_MMC:	/* FALLTHROUGH */
>  	case BOOTSOURCE_SPI:	/* FALLTHROUGH */
>  	case BOOTSOURCE_CAN:
> -		stem = bootsource_str[bootsource];
> +		return bootsource_str[bootsource];
>  		break;
>  	default:
>  		return NULL;
>  	}
> +}
> +
> +/**
> + * bootsource_get_alias_name() - Get the name of the bootsource alias
> + *
> + * This function will return newly allocated string containing name of
> + * the alias that is expected to point to DTB node corresponding to
> + * detected bootsource
> + *
> + * NOTE: Caller is expected to free() the string allocated by this
> + * function
> + */
> +char *bootsource_get_alias_name(void)
> +{
> +	const char *stem;
> +
> +	/*
> +	 * If alias name was overridden via
> +	 * bootsource_set_alias_name() return that value without
> +	 * asking any questions.
> +	 *
> +	 * Note that we have to strdup() the result to make it
> +	 * free-able.
> +	 */
> +	if (bootsource_alias_name)
> +		return strdup(bootsource_alias_name);
> +
> +	stem = bootsource_get_of_stem();
>  
>  	/*
>  	 * We expect SoC specific bootsource detection code to properly
> @@ -125,6 +131,20 @@ void bootsource_set_instance(int instance)
>  	setenv("bootsource_instance", buf);
>  }
>  
> +void bootsource_set_instance_by_of_name(const char *name)
> +{
> +	int instance = BOOTSOURCE_UNKNOWN;
> +	struct device_node *node;
> +
> +	node = of_find_node_by_name(of_get_root_node(), name);
> +	if (node) {
> +		instance = of_alias_get_id(node, bootsource_get_of_stem());
> +		if (instance < 0)
> +			instance = BOOTSOURCE_UNKNOWN;
> +	}
> +	bootsource_set_instance(instance);
> +}
> +
>  enum bootsource bootsource_get(void)
>  {
>  	return bootsource;
> diff --git a/include/bootsource.h b/include/bootsource.h
> index 646b0e91c..4dcb969ac 100644
> --- a/include/bootsource.h
> +++ b/include/bootsource.h
> @@ -28,6 +28,7 @@ enum bootsource bootsource_get(void);
>  int bootsource_get_instance(void);
>  void bootsource_set(enum bootsource src);
>  void bootsource_set_instance(int instance);
> +void bootsource_set_instance_by_of_name(const char *name);
>  void bootsource_set_alias_name(const char *name);
>  char *bootsource_get_alias_name(void);
>  

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] bootsource: add helper to set instance by name
  2021-11-17 14:24         ` [RFC PATCH] bootsource: add helper to set instance by name Michael Riesch
  2022-01-14  8:25           ` Michael Riesch
@ 2022-01-14  9:15           ` Sascha Hauer
  1 sibling, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2022-01-14  9:15 UTC (permalink / raw)
  To: Michael Riesch; +Cc: barebox

On Wed, Nov 17, 2021 at 03:24:48PM +0100, Michael Riesch wrote:
> Instance numbers should be related to device tree aliases, which may be
> board-specific.

When instance numbers were introduced they were meant to be SoC
specific, i.e. they should correspond to the numbers in the manual.
When starting with device tree aliases the alias numbers were assumed to
match the numbers in the manual. On i.MX this is still mostly true, but
on (most?) other SoCs it is not. In barebox we have several users of
bootsource_get_instance() which rely on the instance being the hardware
instance number, so we can't just change that.

If we have to keep track of both hardware numbers and alias numbers we
have to store both separately and not change the meaning of the existing
instance number.

Thinking this further I don't think the alias is worth anything because
it may differ between the barebox internal device tree and the one the
kernel is booted with. We should rather store the device node providing
the bootsource. From that we can retrieve the alias matching a given
device tree if we still need it.

That said, I don't know a good way how we get there from our existing
codebase.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-01-14  9:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-11 14:03 [PATCH 0/3] arm: rockchip: minor fixes as preparation for mainline dts Michael Riesch
2021-11-11 14:03 ` [PATCH 1/3] arm: rockchip: rk3568: fix mmc boot source instances Michael Riesch
2021-11-15  7:51   ` Sascha Hauer
2021-11-15  8:06     ` Sascha Hauer
2021-11-15  9:24       ` Michael Riesch
2021-11-17 14:24         ` [RFC PATCH] bootsource: add helper to set instance by name Michael Riesch
2022-01-14  8:25           ` Michael Riesch
2022-01-14  9:15           ` Sascha Hauer
2021-11-11 14:03 ` [PATCH 2/3] net: designware: rockchip: remove unnecessary clock pclk_xpcs Michael Riesch
2022-01-14  5:24   ` Ahmad Fatoum
2022-01-14  8:16     ` Sascha Hauer
2021-11-11 14:03 ` [PATCH 3/3] pinctrl: rockchip: use alias rather than full of name Michael Riesch
2021-11-15  7:54   ` Sascha Hauer
2021-11-17 12:42     ` Michael Riesch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox