u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection
@ 2025-08-28 13:51 Naresh Kumar Ravulapalli
  2025-08-29  4:58 ` Chee, Tien Fong
  2025-08-29 23:58 ` Marek Vasut
  0 siblings, 2 replies; 6+ messages in thread
From: Naresh Kumar Ravulapalli @ 2025-08-28 13:51 UTC (permalink / raw)
  To: u-boot
  Cc: Marek Vasut, Simon Goldschmidt, Tien Fong Chee, Tom Rini,
	Naresh Kumar Ravulapalli

Fix the incorrect bit masking and bit shift used to compute EMAC
control which in turn is used to select EMAC clock from EMAC
source A or B.

Signed-off-by: Naresh Kumar Ravulapalli <nareshkumar.ravulapalli@altera.com>
---
 drivers/clk/altera/clk-agilex.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/altera/clk-agilex.c b/drivers/clk/altera/clk-agilex.c
index e1ddd02f356..2ad8cfd8856 100644
--- a/drivers/clk/altera/clk-agilex.c
+++ b/drivers/clk/altera/clk-agilex.c
@@ -1,6 +1,7 @@
-// SPDX-License-Identifier: GPL-2.0
+// SPDX-License-Identifier: GPL-2.0+
 /*
  * Copyright (C) 2019 Intel Corporation <www.intel.com>
+ * Copyright (C) 2025 Altera Corporation <www.altera.com>
  */
 
 #include <log.h>
@@ -11,6 +12,7 @@
 #include <dm/lists.h>
 #include <dm/util.h>
 #include <dt-bindings/clock/agilex-clock.h>
+#include <linux/bitfield.h>
 #include <linux/bitops.h>
 
 #include <asm/arch/clock_manager.h>
@@ -515,14 +517,11 @@ static u32 clk_get_emac_clk_hz(struct socfpga_clk_plat *plat, u32 emac_id)
 	/* Get EMAC clock source */
 	ctl = CM_REG_READL(plat, CLKMGR_PERPLL_EMACCTL);
 	if (emac_id == AGILEX_EMAC0_CLK)
-		ctl = (ctl >> CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_OFFSET) &
-		       CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_MASK;
+		ctl = FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_MASK, ctl);
 	else if (emac_id == AGILEX_EMAC1_CLK)
-		ctl = (ctl >> CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_OFFSET) &
-		       CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_MASK;
+		ctl = FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_MASK, ctl);
 	else if (emac_id == AGILEX_EMAC2_CLK)
-		ctl = (ctl >> CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_OFFSET) &
-		       CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_MASK;
+		ctl = FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_MASK, ctl);
 	else
 		return 0;
 
-- 
2.35.3


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

* RE: [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection
  2025-08-28 13:51 [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection Naresh Kumar Ravulapalli
@ 2025-08-29  4:58 ` Chee, Tien Fong
  2025-08-29 14:26   ` Tom Rini
  2025-08-29 23:58 ` Marek Vasut
  1 sibling, 1 reply; 6+ messages in thread
From: Chee, Tien Fong @ 2025-08-29  4:58 UTC (permalink / raw)
  To: Ravulapalli, Naresh Kumar, u-boot@lists.denx.de
  Cc: Marek Vasut, Simon Goldschmidt, Tom Rini



> -----Original Message-----
> From: Ravulapalli, Naresh Kumar <naresh.kumar.ravulapalli@altera.com>
> Sent: Thursday, August 28, 2025 9:52 PM
> To: u-boot@lists.denx.de
> Cc: Marek Vasut <marek.vasut@mailbox.org>; Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com>; Chee, Tien Fong
> <tien.fong.chee@altera.com>; Tom Rini <trini@konsulko.com>; Ravulapalli,
> Naresh Kumar <naresh.kumar.ravulapalli@altera.com>
> Subject: [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection
> 
> Fix the incorrect bit masking and bit shift used to compute EMAC control
> which in turn is used to select EMAC clock from EMAC source A or B.
> 
> Signed-off-by: Naresh Kumar Ravulapalli
> <nareshkumar.ravulapalli@altera.com>
> ---
>  drivers/clk/altera/clk-agilex.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clk/altera/clk-agilex.c b/drivers/clk/altera/clk-agilex.c
> index e1ddd02f356..2ad8cfd8856 100644
> --- a/drivers/clk/altera/clk-agilex.c
> +++ b/drivers/clk/altera/clk-agilex.c
> @@ -1,6 +1,7 @@
> -// SPDX-License-Identifier: GPL-2.0
> +// SPDX-License-Identifier: GPL-2.0+
>  /*
>   * Copyright (C) 2019 Intel Corporation <www.intel.com>
> + * Copyright (C) 2025 Altera Corporation <www.altera.com>
>   */
> 
>  #include <log.h>
> @@ -11,6 +12,7 @@
>  #include <dm/lists.h>
>  #include <dm/util.h>
>  #include <dt-bindings/clock/agilex-clock.h>
> +#include <linux/bitfield.h>
>  #include <linux/bitops.h>
> 
>  #include <asm/arch/clock_manager.h>
> @@ -515,14 +517,11 @@ static u32 clk_get_emac_clk_hz(struct
> socfpga_clk_plat *plat, u32 emac_id)
>  	/* Get EMAC clock source */
>  	ctl = CM_REG_READL(plat, CLKMGR_PERPLL_EMACCTL);
>  	if (emac_id == AGILEX_EMAC0_CLK)
> -		ctl = (ctl >>
> CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_OFFSET) &
> -		       CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_MASK;
> +		ctl =
> FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_MASK, ctl);
>  	else if (emac_id == AGILEX_EMAC1_CLK)
> -		ctl = (ctl >>
> CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_OFFSET) &
> -		       CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_MASK;
> +		ctl =
> FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_MASK, ctl);
>  	else if (emac_id == AGILEX_EMAC2_CLK)
> -		ctl = (ctl >>
> CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_OFFSET) &
> -		       CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_MASK;
> +		ctl =
> FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_MASK, ctl);
>  	else
>  		return 0;
> 
> --
> 2.35.3

Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong


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

* Re: [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection
  2025-08-29  4:58 ` Chee, Tien Fong
@ 2025-08-29 14:26   ` Tom Rini
  2025-08-29 16:00     ` Ravulapalli, Naresh Kumar
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Rini @ 2025-08-29 14:26 UTC (permalink / raw)
  To: Chee, Tien Fong
  Cc: Ravulapalli, Naresh Kumar, u-boot@lists.denx.de, Marek Vasut,
	Simon Goldschmidt

[-- Attachment #1: Type: text/plain, Size: 1828 bytes --]

On Fri, Aug 29, 2025 at 04:58:07AM +0000, Chee, Tien Fong wrote:
> 
> 
> > -----Original Message-----
> > From: Ravulapalli, Naresh Kumar <naresh.kumar.ravulapalli@altera.com>
> > Sent: Thursday, August 28, 2025 9:52 PM
> > To: u-boot@lists.denx.de
> > Cc: Marek Vasut <marek.vasut@mailbox.org>; Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com>; Chee, Tien Fong
> > <tien.fong.chee@altera.com>; Tom Rini <trini@konsulko.com>; Ravulapalli,
> > Naresh Kumar <naresh.kumar.ravulapalli@altera.com>
> > Subject: [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection
> > 
> > Fix the incorrect bit masking and bit shift used to compute EMAC control
> > which in turn is used to select EMAC clock from EMAC source A or B.
> > 
> > Signed-off-by: Naresh Kumar Ravulapalli
> > <nareshkumar.ravulapalli@altera.com>
> > ---
> >  drivers/clk/altera/clk-agilex.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/clk/altera/clk-agilex.c b/drivers/clk/altera/clk-agilex.c
> > index e1ddd02f356..2ad8cfd8856 100644
> > --- a/drivers/clk/altera/clk-agilex.c
> > +++ b/drivers/clk/altera/clk-agilex.c
> > @@ -1,6 +1,7 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > +// SPDX-License-Identifier: GPL-2.0+
> >  /*
> >   * Copyright (C) 2019 Intel Corporation <www.intel.com>
> > + * Copyright (C) 2025 Altera Corporation <www.altera.com>
> >   */
> > 
> >  #include <log.h>

This is a NAK here. You can't just change the license, especially
without calling it out in the commit message. And I'm not pleased with
"make a small change, add/extend copyright". I'm not going to run around
NAK'ing everything else you've sent but I'm going to ask you to stop
doing that for changes that are not significant in size or scope.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection
  2025-08-29 14:26   ` Tom Rini
@ 2025-08-29 16:00     ` Ravulapalli, Naresh Kumar
  0 siblings, 0 replies; 6+ messages in thread
From: Ravulapalli, Naresh Kumar @ 2025-08-29 16:00 UTC (permalink / raw)
  To: Tom Rini, Chee, Tien Fong
  Cc: Ravulapalli, Naresh Kumar, u-boot@lists.denx.de, Marek Vasut,
	Simon Goldschmidt

Hi Tom

On 29-Aug-25 7:56 PM, Tom Rini wrote:
> On Fri, Aug 29, 2025 at 04:58:07AM +0000, Chee, Tien Fong wrote:
>>
>>> -----Original Message-----
>>> From: Ravulapalli, Naresh Kumar<naresh.kumar.ravulapalli@altera.com>
>>> Sent: Thursday, August 28, 2025 9:52 PM
>>> To:u-boot@lists.denx.de
>>> Cc: Marek Vasut<marek.vasut@mailbox.org>; Simon Goldschmidt
>>> <simon.k.r.goldschmidt@gmail.com>; Chee, Tien Fong
>>> <tien.fong.chee@altera.com>; Tom Rini<trini@konsulko.com>; Ravulapalli,
>>> Naresh Kumar<naresh.kumar.ravulapalli@altera.com>
>>> Subject: [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection
>>>
>>> Fix the incorrect bit masking and bit shift used to compute EMAC control
>>> which in turn is used to select EMAC clock from EMAC source A or B.
>>>
>>> Signed-off-by: Naresh Kumar Ravulapalli
>>> <nareshkumar.ravulapalli@altera.com>
>>> ---
>>>   drivers/clk/altera/clk-agilex.c | 13 ++++++-------
>>>   1 file changed, 6 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/clk/altera/clk-agilex.c b/drivers/clk/altera/clk-agilex.c
>>> index e1ddd02f356..2ad8cfd8856 100644
>>> --- a/drivers/clk/altera/clk-agilex.c
>>> +++ b/drivers/clk/altera/clk-agilex.c
>>> @@ -1,6 +1,7 @@
>>> -// SPDX-License-Identifier: GPL-2.0
>>> +// SPDX-License-Identifier: GPL-2.0+
>>>   /*
>>>    * Copyright (C) 2019 Intel Corporation <www.intel.com>
>>> + * Copyright (C) 2025 Altera Corporation <www.altera.com>
>>>    */
>>>
>>>   #include <log.h>
> This is a NAK here. You can't just change the license, especially
> without calling it out in the commit message. And I'm not pleased with
> "make a small change, add/extend copyright". I'm not going to run around
> NAK'ing everything else you've sent but I'm going to ask you to stop
> doing that for changes that are not significant in size or scope.

Patch is about an important bug fix but I agree with your thoughts on 
license extension. I will revert the changes.

Kind Regards
Naresh


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

* Re: [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection
  2025-08-28 13:51 [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection Naresh Kumar Ravulapalli
  2025-08-29  4:58 ` Chee, Tien Fong
@ 2025-08-29 23:58 ` Marek Vasut
  2025-09-02  8:08   ` Ravulapalli, Naresh Kumar
  1 sibling, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2025-08-29 23:58 UTC (permalink / raw)
  To: Naresh Kumar Ravulapalli, u-boot
  Cc: Simon Goldschmidt, Tien Fong Chee, Tom Rini

On 8/28/25 3:51 PM, Naresh Kumar Ravulapalli wrote:
> Fix the incorrect bit masking and bit shift used to compute EMAC
> control which in turn is used to select EMAC clock from EMAC
> source A or B.
> 
> Signed-off-by: Naresh Kumar Ravulapalli <nareshkumar.ravulapalli@altera.com>
> ---
>   drivers/clk/altera/clk-agilex.c | 13 ++++++-------
>   1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clk/altera/clk-agilex.c b/drivers/clk/altera/clk-agilex.c
> index e1ddd02f356..2ad8cfd8856 100644
> --- a/drivers/clk/altera/clk-agilex.c
> +++ b/drivers/clk/altera/clk-agilex.c
> @@ -1,6 +1,7 @@
> -// SPDX-License-Identifier: GPL-2.0
> +// SPDX-License-Identifier: GPL-2.0+
>   /*
>    * Copyright (C) 2019 Intel Corporation <www.intel.com>
> + * Copyright (C) 2025 Altera Corporation <www.altera.com>
>    */
>   
>   #include <log.h>
> @@ -11,6 +12,7 @@
>   #include <dm/lists.h>
>   #include <dm/util.h>
>   #include <dt-bindings/clock/agilex-clock.h>
> +#include <linux/bitfield.h>
>   #include <linux/bitops.h>
>   
>   #include <asm/arch/clock_manager.h>
> @@ -515,14 +517,11 @@ static u32 clk_get_emac_clk_hz(struct socfpga_clk_plat *plat, u32 emac_id)
>   	/* Get EMAC clock source */
>   	ctl = CM_REG_READL(plat, CLKMGR_PERPLL_EMACCTL);
>   	if (emac_id == AGILEX_EMAC0_CLK)
> -		ctl = (ctl >> CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_OFFSET) &
> -		       CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_MASK;
> +		ctl = FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_MASK, ctl);
>   	else if (emac_id == AGILEX_EMAC1_CLK)
> -		ctl = (ctl >> CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_OFFSET) &
> -		       CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_MASK;
> +		ctl = FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_MASK, ctl);
>   	else if (emac_id == AGILEX_EMAC2_CLK)
> -		ctl = (ctl >> CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_OFFSET) &
> -		       CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_MASK;
> +		ctl = FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_MASK, ctl);
Please do the actual fix separately from FIELD_GET() conversion.

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

* Re: [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection
  2025-08-29 23:58 ` Marek Vasut
@ 2025-09-02  8:08   ` Ravulapalli, Naresh Kumar
  0 siblings, 0 replies; 6+ messages in thread
From: Ravulapalli, Naresh Kumar @ 2025-09-02  8:08 UTC (permalink / raw)
  To: Marek Vasut, u-boot; +Cc: Simon Goldschmidt, Tien Fong Chee, Tom Rini

Hi Marek

On 30-Aug-25 5:28 AM, Marek Vasut wrote:
> 
> On 8/28/25 3:51 PM, Naresh Kumar Ravulapalli wrote:
>> Fix the incorrect bit masking and bit shift used to compute EMAC
>> control which in turn is used to select EMAC clock from EMAC
>> source A or B.
>>
>> Signed-off-by: Naresh Kumar Ravulapalli 
>> <nareshkumar.ravulapalli@altera.com>
>> ---
>>   drivers/clk/altera/clk-agilex.c | 13 ++++++-------
>>   1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/clk/altera/clk-agilex.c b/drivers/clk/altera/clk- 
>> agilex.c
>> index e1ddd02f356..2ad8cfd8856 100644
>> --- a/drivers/clk/altera/clk-agilex.c
>> +++ b/drivers/clk/altera/clk-agilex.c
>> @@ -1,6 +1,7 @@
>> -// SPDX-License-Identifier: GPL-2.0
>> +// SPDX-License-Identifier: GPL-2.0+
>>   /*
>>    * Copyright (C) 2019 Intel Corporation <www.intel.com>
>> + * Copyright (C) 2025 Altera Corporation <www.altera.com>
>>    */
>>
>>   #include <log.h>
>> @@ -11,6 +12,7 @@
>>   #include <dm/lists.h>
>>   #include <dm/util.h>
>>   #include <dt-bindings/clock/agilex-clock.h>
>> +#include <linux/bitfield.h>
>>   #include <linux/bitops.h>
>>
>>   #include <asm/arch/clock_manager.h>
>> @@ -515,14 +517,11 @@ static u32 clk_get_emac_clk_hz(struct 
>> socfpga_clk_plat *plat, u32 emac_id)
>>       /* Get EMAC clock source */
>>       ctl = CM_REG_READL(plat, CLKMGR_PERPLL_EMACCTL);
>>       if (emac_id == AGILEX_EMAC0_CLK)
>> -             ctl = (ctl >> CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_OFFSET) &
>> -                    CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_MASK;
>> +             ctl = FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC0SELB_MASK, 
>> ctl);
>>       else if (emac_id == AGILEX_EMAC1_CLK)
>> -             ctl = (ctl >> CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_OFFSET) &
>> -                    CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_MASK;
>> +             ctl = FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC1SELB_MASK, 
>> ctl);
>>       else if (emac_id == AGILEX_EMAC2_CLK)
>> -             ctl = (ctl >> CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_OFFSET) &
>> -                    CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_MASK;
>> +             ctl = FIELD_GET(CLKMGR_PERPLLGRP_EMACCTL_EMAC2SELB_MASK, 
>> ctl);
> Please do the actual fix separately from FIELD_GET() conversion.

Sure, will make the change.

Kind Regards
Naresh


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

end of thread, other threads:[~2025-09-02  8:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-28 13:51 [PATCH v2] drivers: clk: agilex: Correct EMAC clock source selection Naresh Kumar Ravulapalli
2025-08-29  4:58 ` Chee, Tien Fong
2025-08-29 14:26   ` Tom Rini
2025-08-29 16:00     ` Ravulapalli, Naresh Kumar
2025-08-29 23:58 ` Marek Vasut
2025-09-02  8:08   ` Ravulapalli, Naresh Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).