[PATCH v2 1/2] clk: cdce9xx: Convert .of_xlate to .request

[PATCH v2 1/2] clk: cdce9xx: Convert .of_xlate to .request

[Sean Anderson]

This xlate function just performs some checking. We can do this in
request() instead and use the default xlate.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

Changes in v2:
- Fix build error caused by mismatched function name

 drivers/clk/clk-cdce9xx.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/clk-cdce9xx.c b/drivers/clk/clk-cdce9xx.c
index 6634b7b799..f23465d7e1 100644
--- a/drivers/clk/clk-cdce9xx.c
+++ b/drivers/clk/clk-cdce9xx.c
@@ -86,19 +86,13 @@ static int cdce9xx_reg_write(struct udevice *dev, u8 addr, u8 val)
 	return ret;
 }
 
-static int cdce9xx_clk_of_xlate(struct clk *clk,
-				struct ofnode_phandle_args *args)
+static int cdce9xx_clk_request(struct clk *clk)
 {
 	struct cdce9xx_clk_data *data = dev_get_priv(clk->dev);
 
-	if (args->args_count != 1)
+	if (clk->id > data->chip->num_outputs)
 		return -EINVAL;
 
-	if (args->args[0] > data->chip->num_outputs)
-		return -EINVAL;
-
-	clk->id = args->args[0];
-
 	return 0;
 }
 
@@ -241,7 +235,7 @@ static const struct udevice_id cdce9xx_clk_of_match[] = {
 };
 
 static const struct clk_ops cdce9xx_clk_ops = {
-	.of_xlate = cdce9xx_clk_of_xlate,
+	.request = cdce9xx_clk_request,
 	.get_rate = cdce9xx_clk_get_rate,
 	.set_rate = cdce9xx_clk_set_rate,
 };
-- 
2.33.0

[PATCH v2 2/2] clk: cdce9xxx: Add maintainer

[Sean Anderson]

This adds an entry in MAINTAINERS for the cdce9xx driver, since it was not
added when the driver was submitted. This will help future submitters
figure out who to CC.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
Tero, if you don't want to maintain this I'll resubmit this patch with the
orphaned status. Alternatively, perhaps Tom wants to maintain this driver since
it is used exclusively by boards he maintains.

(no changes since v1)

 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6db5354322..44f4b846e0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -658,6 +658,12 @@ M:	Simon Glass <sjg@chromium.org>
 S:	Maintained
 F:	tools/buildman/
 
+CDCE9XX CLOCK:
+M:	Tero Kristo <kristo@kernel.org>
+S:	Maintained
+F:	doc/device-tree-bindings/clock/ti,cdce9xx.txt
+F:	drivers/clk/clk-cdce9xx.c
+
 CFI FLASH
 M:	Stefan Roese <sr@denx.de>
 S:	Maintained
-- 
2.33.0

Re: [PATCH v2 1/2] clk: cdce9xx: Convert .of_xlate to .request

[Tero Kristo]

On 15/12/2021 18:47, Sean Anderson wrote:
> This xlate function just performs some checking. We can do this in
> request() instead and use the default xlate.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>

Looks good to me now.

Reviewed-by: Tero Kristo <kristo@kernel.org>

> ---
> 
> Changes in v2:
> - Fix build error caused by mismatched function name
> 
>   drivers/clk/clk-cdce9xx.c | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/clk/clk-cdce9xx.c b/drivers/clk/clk-cdce9xx.c
> index 6634b7b799..f23465d7e1 100644
> --- a/drivers/clk/clk-cdce9xx.c
> +++ b/drivers/clk/clk-cdce9xx.c
> @@ -86,19 +86,13 @@ static int cdce9xx_reg_write(struct udevice *dev, u8 addr, u8 val)
>   	return ret;
>   }
>   
> -static int cdce9xx_clk_of_xlate(struct clk *clk,
> -				struct ofnode_phandle_args *args)
> +static int cdce9xx_clk_request(struct clk *clk)
>   {
>   	struct cdce9xx_clk_data *data = dev_get_priv(clk->dev);
>   
> -	if (args->args_count != 1)
> +	if (clk->id > data->chip->num_outputs)
>   		return -EINVAL;
>   
> -	if (args->args[0] > data->chip->num_outputs)
> -		return -EINVAL;
> -
> -	clk->id = args->args[0];
> -
>   	return 0;
>   }
>   
> @@ -241,7 +235,7 @@ static const struct udevice_id cdce9xx_clk_of_match[] = {
>   };
>   
>   static const struct clk_ops cdce9xx_clk_ops = {
> -	.of_xlate = cdce9xx_clk_of_xlate,
> +	.request = cdce9xx_clk_request,
>   	.get_rate = cdce9xx_clk_get_rate,
>   	.set_rate = cdce9xx_clk_set_rate,
>   };
> 

Re: [PATCH v2 2/2] clk: cdce9xxx: Add maintainer

[Tero Kristo]

On 15/12/2021 18:47, Sean Anderson wrote:
> This adds an entry in MAINTAINERS for the cdce9xx driver, since it was not
> added when the driver was submitted. This will help future submitters
> figure out who to CC.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
> Tero, if you don't want to maintain this I'll resubmit this patch with the
> orphaned status. Alternatively, perhaps Tom wants to maintain this driver since
> it is used exclusively by boards he maintains.

Well, I am not actively maintaining this, as I am not employed by TI 
anymore. :)

That said, I don't see how much work there is needed for this driver 
anyways, it is very simple, and it can be considered "completed". That's 
the reason I didn't initially put any maintainer on it. Marking it 
"orphaned" would be a bit too harsh status for it imho, as it is still 
used by TI platforms, and it gets actively tested by them.

-Tero

> 
> (no changes since v1)
> 
>   MAINTAINERS | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6db5354322..44f4b846e0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -658,6 +658,12 @@ M:	Simon Glass <sjg@chromium.org>
>   S:	Maintained
>   F:	tools/buildman/
>   
> +CDCE9XX CLOCK:
> +M:	Tero Kristo <kristo@kernel.org>
> +S:	Maintained
> +F:	doc/device-tree-bindings/clock/ti,cdce9xx.txt
> +F:	drivers/clk/clk-cdce9xx.c
> +
>   CFI FLASH
>   M:	Stefan Roese <sr@denx.de>
>   S:	Maintained
> 

Re: [PATCH v2 2/2] clk: cdce9xxx: Add maintainer

[Sean Anderson]

On 12/16/21 8:17 AM, Tero Kristo wrote:
> On 15/12/2021 18:47, Sean Anderson wrote:
>> This adds an entry in MAINTAINERS for the cdce9xx driver, since it was not
>> added when the driver was submitted. This will help future submitters
>> figure out who to CC.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>> Tero, if you don't want to maintain this I'll resubmit this patch with the
>> orphaned status. Alternatively, perhaps Tom wants to maintain this driver since
>> it is used exclusively by boards he maintains.
> 
> Well, I am not actively maintaining this, as I am not employed by TI anymore. :)
> 
> That said, I don't see how much work there is needed for this driver anyways, it is very simple, and it can be considered "completed". That's the reason I didn't initially put any maintainer on it. Marking it "orphaned" would be a bit too harsh status for it imho, as it is still used by TI platforms, and it gets actively tested by them.

Well, the issue that I would like to resolve is that in order to CC you
I had to look up who did the initial commit, and even then the email was
wrong (since it had your TI email). So for the benefit of future
hackers, I would like to record your current email. I suppose your
Reviewed-By on the first patch will do.

--Sean

Re: [PATCH v2 2/2] clk: cdce9xxx: Add maintainer

[Tero Kristo]

On 16/12/2021 15:57, Sean Anderson wrote:
> 
> On 12/16/21 8:17 AM, Tero Kristo wrote:
>> On 15/12/2021 18:47, Sean Anderson wrote:
>>> This adds an entry in MAINTAINERS for the cdce9xx driver, since it 
>>> was not
>>> added when the driver was submitted. This will help future submitters
>>> figure out who to CC.
>>>
>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>> ---
>>> Tero, if you don't want to maintain this I'll resubmit this patch 
>>> with the
>>> orphaned status. Alternatively, perhaps Tom wants to maintain this 
>>> driver since
>>> it is used exclusively by boards he maintains.
>>
>> Well, I am not actively maintaining this, as I am not employed by TI 
>> anymore. :)
>>
>> That said, I don't see how much work there is needed for this driver 
>> anyways, it is very simple, and it can be considered "completed". 
>> That's the reason I didn't initially put any maintainer on it. Marking 
>> it "orphaned" would be a bit too harsh status for it imho, as it is 
>> still used by TI platforms, and it gets actively tested by them.
> 
> Well, the issue that I would like to resolve is that in order to CC you
> I had to look up who did the initial commit, and even then the email was
> wrong (since it had your TI email). So for the benefit of future
> hackers, I would like to record your current email. I suppose your
> Reviewed-By on the first patch will do.

Yes, I understand your point, however adding a maintainer entry for 
every tiny driver is a bit of an overkill. And well, it would also need 
an ack from the subsystem maintainer itself, whether he/she wants to 
share the workload on it or not.

U-boot contains a script called get_maintainer, that can be used to 
fetch the valid maintainer entry for a file. In this case, it reports 
Lukasz, which, imho, is just fine. cdce9xxx is not going to face many 
changes, and if any, those are just generic framework changes/fixes 
which can be handled just fine by the subsystem maintainers.

-Tero

Re: [PATCH v2 2/2] clk: cdce9xxx: Add maintainer

[Sean Anderson]

On 12/16/21 9:07 AM, Tero Kristo wrote:
> On 16/12/2021 15:57, Sean Anderson wrote:
>>
>> On 12/16/21 8:17 AM, Tero Kristo wrote:
>>> On 15/12/2021 18:47, Sean Anderson wrote:
>>>> This adds an entry in MAINTAINERS for the cdce9xx driver, since it was not
>>>> added when the driver was submitted. This will help future submitters
>>>> figure out who to CC.
>>>>
>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>> ---
>>>> Tero, if you don't want to maintain this I'll resubmit this patch with the
>>>> orphaned status. Alternatively, perhaps Tom wants to maintain this driver since
>>>> it is used exclusively by boards he maintains.
>>>
>>> Well, I am not actively maintaining this, as I am not employed by TI anymore. :)
>>>
>>> That said, I don't see how much work there is needed for this driver anyways, it is very simple, and it can be considered "completed". That's the reason I didn't initially put any maintainer on it. Marking it "orphaned" would be a bit too harsh status for it imho, as it is still used by TI platforms, and it gets actively tested by them.
>>
>> Well, the issue that I would like to resolve is that in order to CC you
>> I had to look up who did the initial commit, and even then the email was
>> wrong (since it had your TI email). So for the benefit of future
>> hackers, I would like to record your current email. I suppose your
>> Reviewed-By on the first patch will do.
> 
> Yes, I understand your point, however adding a maintainer entry for every tiny driver is a bit of an overkill.

I think it's great to have. It helps submitters know who to CC, and it
helps me know who I need a review/ack from. It also helps to know who
has hardware if a change needs testing, though perhaps you don't have
access to this hardware any more.

> And well, it would also need an ack from the subsystem maintainer itself, whether he/she wants to share the workload on it or not.

Because Lukasz has not been very active lately, I volunteered to be
clock subsystem maintainer.

> U-boot contains a script called get_maintainer, that can be used to fetch the valid maintainer entry for a file. In this case, it reports Lukasz, which, imho, is just fine. cdce9xxx is not going to face many changes, and if any, those are just generic framework changes/fixes which can be handled just fine by the subsystem maintainers.

And that is why I wanted to have your email in MAINTAINERS. But since
you reviewed the other patch, I think it will pick up your new email. So
I don't think this patch is necessary.

--Sean

Re: (subset) [PATCH v2 1/2] clk: cdce9xx: Convert .of_xlate to .request

[Sean Anderson]

On Wed, 15 Dec 2021 11:47:17 -0500, Sean Anderson wrote:
> This xlate function just performs some checking. We can do this in
> request() instead and use the default xlate.
> 
> 

Applied, thanks!

[1/2] clk: cdce9xx: Convert .of_xlate to .request
      commit: f9489580750af51ef7214bb1937d4b1d3c00a333

Best regards,
-- 
Sean Anderson <seanga2@gmail.com>