* [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset
@ 2024-02-23 12:45 Quentin Schulz
2024-02-23 12:45 ` [PATCH 1/3] iio: adc: rockchip_saradc: fix bitmask for channels on SARADCv2 Quentin Schulz
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Quentin Schulz @ 2024-02-23 12:45 UTC (permalink / raw
To: Jonathan Cameron, Lars-Peter Clausen, Heiko Stuebner,
AngeloGioacchino Del Regno, Andy Shevchenko, Shreeya Patel,
Simon Xue, Philipp Zabel
Cc: Jonathan Cameron, linux-iio, linux-arm-kernel, linux-rockchip,
linux-kernel, Quentin Schulz, Quentin Schulz
The mask for the channel selection is incorrect as it's specified to be
16b wide by is actually only 4.
Also, the 16 lower bits in the SARADC_CONV_CON register are write
protected. Whatever their value is can only be written to the hardware
block if their associated bit in the higher 16 bits is set. Considering
that the channel bitmask is 4b wide but that we can write e.g. 0 in
there, we shouldn't use the value shifted by 16 as a mask but rather the
bitmask for that value shifted by 16. This is currently NOT an issue
because the only SoC with SARADCv2 IP is the RK3588 which has a reset
defined in the SoC DTSI. When that is the case, the reset is asserted
before every channel conversion is started. This means the registers are
reset so effectively, we do not need to write zeros so the wrong mask
still works because where we should be writing zeroes, there are already
zeroes. However, let's fix this in case there comes a day there's an SoC
which doesn't require to reset the controller before every channel
conversion is started.
Lastly, let's use the appropriate function from the reset subsystem
for getting an optional exclusive reset instead of rolling out our own
logic.
Those three patches should not be changing any behavior.
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
Quentin Schulz (3):
iio: adc: rockchip_saradc: fix bitmask for channels on SARADCv2
iio: adc: rockchip_saradc: use mask for write_enable bitfield
iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive
drivers/iio/adc/rockchip_saradc.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
---
base-commit: 39133352cbed6626956d38ed72012f49b0421e7b
change-id: 20240222-saradcv2-chan-mask-593585865256
Best regards,
--
Quentin Schulz <quentin.schulz@theobroma-systems.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] iio: adc: rockchip_saradc: fix bitmask for channels on SARADCv2
2024-02-23 12:45 [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset Quentin Schulz
@ 2024-02-23 12:45 ` Quentin Schulz
2024-02-23 13:40 ` Heiko Stübner
2024-02-23 12:45 ` [PATCH 2/3] iio: adc: rockchip_saradc: use mask for write_enable bitfield Quentin Schulz
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Quentin Schulz @ 2024-02-23 12:45 UTC (permalink / raw
To: Jonathan Cameron, Lars-Peter Clausen, Heiko Stuebner,
AngeloGioacchino Del Regno, Andy Shevchenko, Shreeya Patel,
Simon Xue, Philipp Zabel
Cc: Jonathan Cameron, linux-iio, linux-arm-kernel, linux-rockchip,
linux-kernel, Quentin Schulz, Quentin Schulz
From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
The SARADCv2 on RK3588 (the only SoC currently supported that has an
SARADCv2) selects the channel through the channel_sel bitfield which is
the 4 lowest bits, therefore the mask should be GENMASK(3, 0) and not
GENMASK(15, 0).
Fixes: 757953f8ec69 ("iio: adc: rockchip_saradc: Add support for RK3588")
Cc: Quentin Schulz <foss+kernel@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
drivers/iio/adc/rockchip_saradc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
index dd94667a623b..2da8d6f3241a 100644
--- a/drivers/iio/adc/rockchip_saradc.c
+++ b/drivers/iio/adc/rockchip_saradc.c
@@ -52,7 +52,7 @@
#define SARADC2_START BIT(4)
#define SARADC2_SINGLE_MODE BIT(5)
-#define SARADC2_CONV_CHANNELS GENMASK(15, 0)
+#define SARADC2_CONV_CHANNELS GENMASK(3, 0)
struct rockchip_saradc;
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] iio: adc: rockchip_saradc: use mask for write_enable bitfield
2024-02-23 12:45 [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset Quentin Schulz
2024-02-23 12:45 ` [PATCH 1/3] iio: adc: rockchip_saradc: fix bitmask for channels on SARADCv2 Quentin Schulz
@ 2024-02-23 12:45 ` Quentin Schulz
2024-02-23 13:43 ` Heiko Stübner
2024-02-23 12:45 ` [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive Quentin Schulz
2024-02-23 13:01 ` [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset Andy Shevchenko
3 siblings, 1 reply; 15+ messages in thread
From: Quentin Schulz @ 2024-02-23 12:45 UTC (permalink / raw
To: Jonathan Cameron, Lars-Peter Clausen, Heiko Stuebner,
AngeloGioacchino Del Regno, Andy Shevchenko, Shreeya Patel,
Simon Xue, Philipp Zabel
Cc: Jonathan Cameron, linux-iio, linux-arm-kernel, linux-rockchip,
linux-kernel, Quentin Schulz, Quentin Schulz
From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
Some of the registers on the SARADCv2 have bits write protected except
if another bit is set. This is usually done by having the lowest 16 bits
store the data to write and the highest 16 bits specify which of the 16
lowest bits should have their value written to the hardware block.
The write_enable mask for the channel selection was incorrect because it
was just the value shifted by 16 bits, which means it would only ever
write bits and never clear them. So e.g. if someone starts a conversion
on channel 5, the lowest 4 bits would be 0x5, then starts a conversion
on channel 0, it would still be 5.
Instead of shifting the value by 16 as the mask, let's use the OR'ing of
the appropriate masks shifted by 16.
Note that this is not an issue currently because the only SARADCv2
currently supported has a reset defined in its Device Tree, that reset
resets the SARADC controller before starting a conversion on a channel.
However, this reset is handled as optional by the probe function and
thus proper masking should be used in the event an SARADCv2 without a
reset ever makes it upstream.
Fixes: 757953f8ec69 ("iio: adc: rockchip_saradc: Add support for RK3588")
Cc: Quentin Schulz <foss+kernel@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
drivers/iio/adc/rockchip_saradc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
index 2da8d6f3241a..1c0042fbbb54 100644
--- a/drivers/iio/adc/rockchip_saradc.c
+++ b/drivers/iio/adc/rockchip_saradc.c
@@ -102,12 +102,12 @@ static void rockchip_saradc_start_v2(struct rockchip_saradc *info, int chn)
writel_relaxed(0xc, info->regs + SARADC_T_DAS_SOC);
writel_relaxed(0x20, info->regs + SARADC_T_PD_SOC);
val = FIELD_PREP(SARADC2_EN_END_INT, 1);
- val |= val << 16;
+ val |= SARADC2_EN_END_INT << 16;
writel_relaxed(val, info->regs + SARADC2_END_INT_EN);
val = FIELD_PREP(SARADC2_START, 1) |
FIELD_PREP(SARADC2_SINGLE_MODE, 1) |
FIELD_PREP(SARADC2_CONV_CHANNELS, chn);
- val |= val << 16;
+ val |= (SARADC2_START | SARADC2_SINGLE_MODE | SARADC2_CONV_CHANNELS) << 16;
writel(val, info->regs + SARADC2_CONV_CON);
}
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive
2024-02-23 12:45 [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset Quentin Schulz
2024-02-23 12:45 ` [PATCH 1/3] iio: adc: rockchip_saradc: fix bitmask for channels on SARADCv2 Quentin Schulz
2024-02-23 12:45 ` [PATCH 2/3] iio: adc: rockchip_saradc: use mask for write_enable bitfield Quentin Schulz
@ 2024-02-23 12:45 ` Quentin Schulz
2024-02-23 13:00 ` Andy Shevchenko
2024-02-23 13:44 ` Heiko Stübner
2024-02-23 13:01 ` [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset Andy Shevchenko
3 siblings, 2 replies; 15+ messages in thread
From: Quentin Schulz @ 2024-02-23 12:45 UTC (permalink / raw
To: Jonathan Cameron, Lars-Peter Clausen, Heiko Stuebner,
AngeloGioacchino Del Regno, Andy Shevchenko, Shreeya Patel,
Simon Xue, Philipp Zabel
Cc: Jonathan Cameron, linux-iio, linux-arm-kernel, linux-rockchip,
linux-kernel, Quentin Schulz, Quentin Schulz
From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
devm_reset_control_get_optional_exclusive does what this driver is
trying to do in its probe function, therefore let's switch over to that
subsystem function.
Cc: Quentin Schulz <foss+kernel@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
drivers/iio/adc/rockchip_saradc.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
index 1c0042fbbb54..bbe954a738c7 100644
--- a/drivers/iio/adc/rockchip_saradc.c
+++ b/drivers/iio/adc/rockchip_saradc.c
@@ -450,16 +450,11 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
* The reset should be an optional property, as it should work
* with old devicetrees as well
*/
- info->reset = devm_reset_control_get_exclusive(&pdev->dev,
- "saradc-apb");
+ info->reset = devm_reset_control_get_optional_exclusive(&pdev->dev,
+ "saradc-apb");
if (IS_ERR(info->reset)) {
ret = PTR_ERR(info->reset);
- if (ret != -ENOENT)
- return dev_err_probe(&pdev->dev, ret,
- "failed to get saradc-apb\n");
-
- dev_dbg(&pdev->dev, "no reset control found\n");
- info->reset = NULL;
+ return dev_err_probe(&pdev->dev, ret, "failed to get saradc-apb\n");
}
init_completion(&info->completion);
--
2.43.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive
2024-02-23 12:45 ` [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive Quentin Schulz
@ 2024-02-23 13:00 ` Andy Shevchenko
2024-02-23 13:10 ` Quentin Schulz
2024-02-23 13:44 ` Heiko Stübner
1 sibling, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2024-02-23 13:00 UTC (permalink / raw
To: Quentin Schulz
Cc: Jonathan Cameron, Lars-Peter Clausen, Heiko Stuebner,
AngeloGioacchino Del Regno, Shreeya Patel, Simon Xue,
Philipp Zabel, Jonathan Cameron, linux-iio, linux-arm-kernel,
linux-rockchip, linux-kernel, Quentin Schulz
On Fri, Feb 23, 2024 at 2:46 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> devm_reset_control_get_optional_exclusive does what this driver is
devm_reset_control_get_optional_exclusive()
> trying to do in its probe function, therefore let's switch over to that
do it in
> subsystem function.
> Cc: Quentin Schulz <foss+kernel@0leil.net>
You may use the --cc option to `git send-email` instead of polluting
commit messages, or move this after the '---' cutter line.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset
2024-02-23 12:45 [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset Quentin Schulz
` (2 preceding siblings ...)
2024-02-23 12:45 ` [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive Quentin Schulz
@ 2024-02-23 13:01 ` Andy Shevchenko
2024-02-24 19:25 ` Jonathan Cameron
3 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2024-02-23 13:01 UTC (permalink / raw
To: Quentin Schulz
Cc: Jonathan Cameron, Lars-Peter Clausen, Heiko Stuebner,
AngeloGioacchino Del Regno, Shreeya Patel, Simon Xue,
Philipp Zabel, Jonathan Cameron, linux-iio, linux-arm-kernel,
linux-rockchip, linux-kernel, Quentin Schulz
On Fri, Feb 23, 2024 at 2:46 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
>
> The mask for the channel selection is incorrect as it's specified to be
> 16b wide by is actually only 4.
>
> Also, the 16 lower bits in the SARADC_CONV_CON register are write
> protected. Whatever their value is can only be written to the hardware
> block if their associated bit in the higher 16 bits is set. Considering
> that the channel bitmask is 4b wide but that we can write e.g. 0 in
> there, we shouldn't use the value shifted by 16 as a mask but rather the
> bitmask for that value shifted by 16. This is currently NOT an issue
> because the only SoC with SARADCv2 IP is the RK3588 which has a reset
> defined in the SoC DTSI. When that is the case, the reset is asserted
> before every channel conversion is started. This means the registers are
> reset so effectively, we do not need to write zeros so the wrong mask
> still works because where we should be writing zeroes, there are already
> zeroes. However, let's fix this in case there comes a day there's an SoC
> which doesn't require to reset the controller before every channel
> conversion is started.
>
> Lastly, let's use the appropriate function from the reset subsystem
> for getting an optional exclusive reset instead of rolling out our own
> logic.
>
> Those three patches should not be changing any behavior.
Nice series, I have the comments in patch 3, but no need to resend
until Jonathan asks for. He might address that whilst applying.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive
2024-02-23 13:00 ` Andy Shevchenko
@ 2024-02-23 13:10 ` Quentin Schulz
2024-02-23 14:39 ` Andy Shevchenko
0 siblings, 1 reply; 15+ messages in thread
From: Quentin Schulz @ 2024-02-23 13:10 UTC (permalink / raw
To: Andy Shevchenko, Quentin Schulz
Cc: Jonathan Cameron, Lars-Peter Clausen, Heiko Stuebner,
AngeloGioacchino Del Regno, Shreeya Patel, Simon Xue,
Philipp Zabel, Jonathan Cameron, linux-iio, linux-arm-kernel,
linux-rockchip, linux-kernel
Hi Andy,
Thanks for the prompt feedback on the whole series.
On 2/23/24 14:00, Andy Shevchenko wrote:
> On Fri, Feb 23, 2024 at 2:46 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
>>
>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>
>> devm_reset_control_get_optional_exclusive does what this driver is
>
> devm_reset_control_get_optional_exclusive()
>
>> trying to do in its probe function, therefore let's switch over to that
>
> do it in
>
>> subsystem function.
>
>> Cc: Quentin Schulz <foss+kernel@0leil.net>
>
> You may use the --cc option to `git send-email` instead of polluting
> commit messages, or move this after the '---' cutter line.
>
The whole point is that my SoB and authorship is from my professional
mail address which is likely to change over time, the Cc is my personal
one for development. Basically, in the event that I change my employer,
I would still be reachable at that Cc address without having to modify
the .mailmap after the fact (which won't make it to an earlier version
of the kernel for example). Some maintainers don't really like this,
some don't mind, we'll see in which category the IIO maintainer(s) fall
in :) (I don't mind either way just to be clear).
Cheers,
Quentin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] iio: adc: rockchip_saradc: fix bitmask for channels on SARADCv2
2024-02-23 12:45 ` [PATCH 1/3] iio: adc: rockchip_saradc: fix bitmask for channels on SARADCv2 Quentin Schulz
@ 2024-02-23 13:40 ` Heiko Stübner
0 siblings, 0 replies; 15+ messages in thread
From: Heiko Stübner @ 2024-02-23 13:40 UTC (permalink / raw
To: Jonathan Cameron, Lars-Peter Clausen, AngeloGioacchino Del Regno,
Andy Shevchenko, Shreeya Patel, Simon Xue, Philipp Zabel,
Quentin Schulz
Cc: Jonathan Cameron, linux-iio, linux-arm-kernel, linux-rockchip,
linux-kernel, Quentin Schulz, Quentin Schulz
Am Freitag, 23. Februar 2024, 13:45:21 CET schrieb Quentin Schulz:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> The SARADCv2 on RK3588 (the only SoC currently supported that has an
> SARADCv2) selects the channel through the channel_sel bitfield which is
> the 4 lowest bits, therefore the mask should be GENMASK(3, 0) and not
> GENMASK(15, 0).
>
> Fixes: 757953f8ec69 ("iio: adc: rockchip_saradc: Add support for RK3588")
> Cc: Quentin Schulz <foss+kernel@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
after checking against the TRM
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] iio: adc: rockchip_saradc: use mask for write_enable bitfield
2024-02-23 12:45 ` [PATCH 2/3] iio: adc: rockchip_saradc: use mask for write_enable bitfield Quentin Schulz
@ 2024-02-23 13:43 ` Heiko Stübner
0 siblings, 0 replies; 15+ messages in thread
From: Heiko Stübner @ 2024-02-23 13:43 UTC (permalink / raw
To: Jonathan Cameron, Lars-Peter Clausen, AngeloGioacchino Del Regno,
Andy Shevchenko, Shreeya Patel, Simon Xue, Philipp Zabel,
Quentin Schulz
Cc: Jonathan Cameron, linux-iio, linux-arm-kernel, linux-rockchip,
linux-kernel, Quentin Schulz, Quentin Schulz
Am Freitag, 23. Februar 2024, 13:45:22 CET schrieb Quentin Schulz:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> Some of the registers on the SARADCv2 have bits write protected except
> if another bit is set. This is usually done by having the lowest 16 bits
> store the data to write and the highest 16 bits specify which of the 16
> lowest bits should have their value written to the hardware block.
>
> The write_enable mask for the channel selection was incorrect because it
> was just the value shifted by 16 bits, which means it would only ever
> write bits and never clear them. So e.g. if someone starts a conversion
> on channel 5, the lowest 4 bits would be 0x5, then starts a conversion
> on channel 0, it would still be 5.
>
> Instead of shifting the value by 16 as the mask, let's use the OR'ing of
> the appropriate masks shifted by 16.
>
> Note that this is not an issue currently because the only SARADCv2
> currently supported has a reset defined in its Device Tree, that reset
> resets the SARADC controller before starting a conversion on a channel.
> However, this reset is handled as optional by the probe function and
> thus proper masking should be used in the event an SARADCv2 without a
> reset ever makes it upstream.
>
> Fixes: 757953f8ec69 ("iio: adc: rockchip_saradc: Add support for RK3588")
> Cc: Quentin Schulz <foss+kernel@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive
2024-02-23 12:45 ` [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive Quentin Schulz
2024-02-23 13:00 ` Andy Shevchenko
@ 2024-02-23 13:44 ` Heiko Stübner
1 sibling, 0 replies; 15+ messages in thread
From: Heiko Stübner @ 2024-02-23 13:44 UTC (permalink / raw
To: Jonathan Cameron, Lars-Peter Clausen, AngeloGioacchino Del Regno,
Andy Shevchenko, Shreeya Patel, Simon Xue, Philipp Zabel,
Quentin Schulz
Cc: Jonathan Cameron, linux-iio, linux-arm-kernel, linux-rockchip,
linux-kernel, Quentin Schulz, Quentin Schulz
Am Freitag, 23. Februar 2024, 13:45:23 CET schrieb Quentin Schulz:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> devm_reset_control_get_optional_exclusive does what this driver is
> trying to do in its probe function, therefore let's switch over to that
> subsystem function.
>
> Cc: Quentin Schulz <foss+kernel@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive
2024-02-23 13:10 ` Quentin Schulz
@ 2024-02-23 14:39 ` Andy Shevchenko
2024-02-26 20:31 ` Dragan Simic
0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2024-02-23 14:39 UTC (permalink / raw
To: Quentin Schulz
Cc: Quentin Schulz, Jonathan Cameron, Lars-Peter Clausen,
Heiko Stuebner, AngeloGioacchino Del Regno, Shreeya Patel,
Simon Xue, Philipp Zabel, Jonathan Cameron, linux-iio,
linux-arm-kernel, linux-rockchip, linux-kernel
On Fri, Feb 23, 2024 at 3:10 PM Quentin Schulz
<quentin.schulz@theobroma-systems.com> wrote:
> On 2/23/24 14:00, Andy Shevchenko wrote:
> > On Fri, Feb 23, 2024 at 2:46 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
...
> >> Cc: Quentin Schulz <foss+kernel@0leil.net>
> >
> > You may use the --cc option to `git send-email` instead of polluting
> > commit messages, or move this after the '---' cutter line.
>
> The whole point is that my SoB and authorship is from my professional
> mail address which is likely to change over time, the Cc is my personal
> one for development. Basically, in the event that I change my employer,
> I would still be reachable at that Cc address without having to modify
> the .mailmap after the fact (which won't make it to an earlier version
> of the kernel for example). Some maintainers don't really like this,
> some don't mind, we'll see in which category the IIO maintainer(s) fall
> in :) (I don't mind either way just to be clear).
My point is that Cc and other similar (non-real-tags) stuff is
polluting commit messages. It means that this will be copied to the
Git index to all kernel git repositories in the world from now and
then, This is at bare minimum makes additional burden on git log (and
parsing and so on) and moreover, wastes resources becoming less
environment friendly (no jokes). Using --cc or moving to the behind
the commit message will keep email copied with cleaner commit
messages. Yet, all email tags are available in lore archive
(lore.kernel.org). Please, really reconsider the commit messages
content in the Linux kernel project and elsewhere, it will help to
make the world more friendly.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset
2024-02-23 13:01 ` [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset Andy Shevchenko
@ 2024-02-24 19:25 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2024-02-24 19:25 UTC (permalink / raw
To: Andy Shevchenko
Cc: Quentin Schulz, Lars-Peter Clausen, Heiko Stuebner,
AngeloGioacchino Del Regno, Shreeya Patel, Simon Xue,
Philipp Zabel, Jonathan Cameron, linux-iio, linux-arm-kernel,
linux-rockchip, linux-kernel, Quentin Schulz
On Fri, 23 Feb 2024 15:01:30 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Fri, Feb 23, 2024 at 2:46 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
> >
> > The mask for the channel selection is incorrect as it's specified to be
> > 16b wide by is actually only 4.
> >
> > Also, the 16 lower bits in the SARADC_CONV_CON register are write
> > protected. Whatever their value is can only be written to the hardware
> > block if their associated bit in the higher 16 bits is set. Considering
> > that the channel bitmask is 4b wide but that we can write e.g. 0 in
> > there, we shouldn't use the value shifted by 16 as a mask but rather the
> > bitmask for that value shifted by 16. This is currently NOT an issue
> > because the only SoC with SARADCv2 IP is the RK3588 which has a reset
> > defined in the SoC DTSI. When that is the case, the reset is asserted
> > before every channel conversion is started. This means the registers are
> > reset so effectively, we do not need to write zeros so the wrong mask
> > still works because where we should be writing zeroes, there are already
> > zeroes. However, let's fix this in case there comes a day there's an SoC
> > which doesn't require to reset the controller before every channel
> > conversion is started.
> >
> > Lastly, let's use the appropriate function from the reset subsystem
> > for getting an optional exclusive reset instead of rolling out our own
> > logic.
> >
> > Those three patches should not be changing any behavior.
>
> Nice series, I have the comments in patch 3, but no need to resend
> until Jonathan asks for. He might address that whilst applying.
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
I've take the series through the togreg branch as we are so near the
merge windows. Note (despite Linus not liking it :)) I use links
in my git tags so anyone really searching for Quentin can find
the discussion.
I'm usually a bit lazy on cleaning out what I would consider unnecessary
CC tags but given Andy comment on these I've dropped the extra
one.
Patches 1 and 2 marked for stable.
Jonathan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive
2024-02-23 14:39 ` Andy Shevchenko
@ 2024-02-26 20:31 ` Dragan Simic
2024-02-27 12:48 ` Andy Shevchenko
0 siblings, 1 reply; 15+ messages in thread
From: Dragan Simic @ 2024-02-26 20:31 UTC (permalink / raw
To: Andy Shevchenko
Cc: Quentin Schulz, Quentin Schulz, Jonathan Cameron,
Lars-Peter Clausen, Heiko Stuebner, AngeloGioacchino Del Regno,
Shreeya Patel, Simon Xue, Philipp Zabel, Jonathan Cameron,
linux-iio, linux-arm-kernel, linux-rockchip, linux-kernel
Hello Andy and Quentin,
On 2024-02-23 15:39, Andy Shevchenko wrote:
> On Fri, Feb 23, 2024 at 3:10 PM Quentin Schulz
> <quentin.schulz@theobroma-systems.com> wrote:
>> On 2/23/24 14:00, Andy Shevchenko wrote:
>> > On Fri, Feb 23, 2024 at 2:46 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
>
> ...
>
>> >> Cc: Quentin Schulz <foss+kernel@0leil.net>
>> >
>> > You may use the --cc option to `git send-email` instead of polluting
>> > commit messages, or move this after the '---' cutter line.
>>
>> The whole point is that my SoB and authorship is from my professional
>> mail address which is likely to change over time, the Cc is my
>> personal
>> one for development. Basically, in the event that I change my
>> employer,
>> I would still be reachable at that Cc address without having to modify
>> the .mailmap after the fact (which won't make it to an earlier version
>> of the kernel for example). Some maintainers don't really like this,
>> some don't mind, we'll see in which category the IIO maintainer(s)
>> fall
>> in :) (I don't mind either way just to be clear).
>
> My point is that Cc and other similar (non-real-tags) stuff is
> polluting commit messages. It means that this will be copied to the
> Git index to all kernel git repositories in the world from now and
> then, This is at bare minimum makes additional burden on git log (and
> parsing and so on) and moreover, wastes resources becoming less
> environment friendly (no jokes). Using --cc or moving to the behind
> the commit message will keep email copied with cleaner commit
> messages. Yet, all email tags are available in lore archive
> (lore.kernel.org). Please, really reconsider the commit messages
> content in the Linux kernel project and elsewhere, it will help to
> make the world more friendly.
Believe it or not, I'm working on some patches for Git that, I believe,
should help a lot when it comes to handling Cc: addresses. Would you
like to be included in the list of recipients for those Git patches, so
you could, hopefully, provide some feeback?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive
2024-02-26 20:31 ` Dragan Simic
@ 2024-02-27 12:48 ` Andy Shevchenko
2024-02-27 14:55 ` Dragan Simic
0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2024-02-27 12:48 UTC (permalink / raw
To: Dragan Simic
Cc: Quentin Schulz, Quentin Schulz, Jonathan Cameron,
Lars-Peter Clausen, Heiko Stuebner, AngeloGioacchino Del Regno,
Shreeya Patel, Simon Xue, Philipp Zabel, Jonathan Cameron,
linux-iio, linux-arm-kernel, linux-rockchip, linux-kernel
On Mon, Feb 26, 2024 at 10:31 PM Dragan Simic <dsimic@manjaro.org> wrote:
> On 2024-02-23 15:39, Andy Shevchenko wrote:
> > On Fri, Feb 23, 2024 at 3:10 PM Quentin Schulz
> > <quentin.schulz@theobroma-systems.com> wrote:
> >> On 2/23/24 14:00, Andy Shevchenko wrote:
...
> >> I would still be reachable at that Cc address without having to modify
> >> the .mailmap after the fact (which won't make it to an earlier version
> >> of the kernel for example). Some maintainers don't really like this,
> >> some don't mind, we'll see in which category the IIO maintainer(s)
> >> fall
> >> in :) (I don't mind either way just to be clear).
> >
> > My point is that Cc and other similar (non-real-tags) stuff is
> > polluting commit messages. It means that this will be copied to the
> > Git index to all kernel git repositories in the world from now and
> > then, This is at bare minimum makes additional burden on git log (and
> > parsing and so on) and moreover, wastes resources becoming less
> > environment friendly (no jokes). Using --cc or moving to the behind
> > the commit message will keep email copied with cleaner commit
> > messages. Yet, all email tags are available in lore archive
> > (lore.kernel.org). Please, really reconsider the commit messages
> > content in the Linux kernel project and elsewhere, it will help to
> > make the world more friendly.
>
> Believe it or not, I'm working on some patches for Git that, I believe,
> should help a lot when it comes to handling Cc: addresses. Would you
> like to be included in the list of recipients for those Git patches, so
> you could, hopefully, provide some feeback?
You may Cc me if you want to, but I can't guarantee I have time or
valuable input to that.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive
2024-02-27 12:48 ` Andy Shevchenko
@ 2024-02-27 14:55 ` Dragan Simic
0 siblings, 0 replies; 15+ messages in thread
From: Dragan Simic @ 2024-02-27 14:55 UTC (permalink / raw
To: Andy Shevchenko
Cc: Quentin Schulz, Quentin Schulz, Jonathan Cameron,
Lars-Peter Clausen, Heiko Stuebner, AngeloGioacchino Del Regno,
Shreeya Patel, Simon Xue, Philipp Zabel, Jonathan Cameron,
linux-iio, linux-arm-kernel, linux-rockchip, linux-kernel
On 2024-02-27 13:48, Andy Shevchenko wrote:
> On Mon, Feb 26, 2024 at 10:31 PM Dragan Simic <dsimic@manjaro.org>
> wrote:
>> On 2024-02-23 15:39, Andy Shevchenko wrote:
>> > On Fri, Feb 23, 2024 at 3:10 PM Quentin Schulz
>> > <quentin.schulz@theobroma-systems.com> wrote:
>> >> On 2/23/24 14:00, Andy Shevchenko wrote:
>
> ...
>
>> >> I would still be reachable at that Cc address without having to modify
>> >> the .mailmap after the fact (which won't make it to an earlier version
>> >> of the kernel for example). Some maintainers don't really like this,
>> >> some don't mind, we'll see in which category the IIO maintainer(s)
>> >> fall
>> >> in :) (I don't mind either way just to be clear).
>> >
>> > My point is that Cc and other similar (non-real-tags) stuff is
>> > polluting commit messages. It means that this will be copied to the
>> > Git index to all kernel git repositories in the world from now and
>> > then, This is at bare minimum makes additional burden on git log (and
>> > parsing and so on) and moreover, wastes resources becoming less
>> > environment friendly (no jokes). Using --cc or moving to the behind
>> > the commit message will keep email copied with cleaner commit
>> > messages. Yet, all email tags are available in lore archive
>> > (lore.kernel.org). Please, really reconsider the commit messages
>> > content in the Linux kernel project and elsewhere, it will help to
>> > make the world more friendly.
>>
>> Believe it or not, I'm working on some patches for Git that, I
>> believe,
>> should help a lot when it comes to handling Cc: addresses. Would you
>> like to be included in the list of recipients for those Git patches,
>> so
>> you could, hopefully, provide some feeback?
>
> You may Cc me if you want to, but I can't guarantee I have time or
> valuable input to that.
Thanks, I'll be happy to have another set of eyes on those Git patches.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-02-27 14:55 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-23 12:45 [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset Quentin Schulz
2024-02-23 12:45 ` [PATCH 1/3] iio: adc: rockchip_saradc: fix bitmask for channels on SARADCv2 Quentin Schulz
2024-02-23 13:40 ` Heiko Stübner
2024-02-23 12:45 ` [PATCH 2/3] iio: adc: rockchip_saradc: use mask for write_enable bitfield Quentin Schulz
2024-02-23 13:43 ` Heiko Stübner
2024-02-23 12:45 ` [PATCH 3/3] iio: adc: rockchip_saradc: replace custom logic with devm_reset_control_get_optional_exclusive Quentin Schulz
2024-02-23 13:00 ` Andy Shevchenko
2024-02-23 13:10 ` Quentin Schulz
2024-02-23 14:39 ` Andy Shevchenko
2024-02-26 20:31 ` Dragan Simic
2024-02-27 12:48 ` Andy Shevchenko
2024-02-27 14:55 ` Dragan Simic
2024-02-23 13:44 ` Heiko Stübner
2024-02-23 13:01 ` [PATCH 0/3] iio: adc: rockchip_saradc: fix bitmasking and remove custom logic for getting reset Andy Shevchenko
2024-02-24 19:25 ` Jonathan Cameron
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 read-only IMAP folder(s) and NNTP newsgroup(s).