* [PATCH 0/2] i2c: mv64xxx: reset-gpios
@ 2023-10-12 3:58 Chris Packham
2023-10-12 3:58 ` [PATCH 1/2] dt-bindings: i2c: mv64xxx: add reset-gpios property Chris Packham
2023-10-12 3:58 ` [PATCH 2/2] i2c: mv64xxx: add an optional " Chris Packham
0 siblings, 2 replies; 12+ messages in thread
From: Chris Packham @ 2023-10-12 3:58 UTC (permalink / raw)
To: gregory.clement, andi.shyti, robh+dt, krzysztof.kozlowski+dt, conor+dt
Cc: linux-i2c, devicetree, linux-kernel, Chris Packham
This series adds the ability to associate a gpio with an I2C bus so that
downstream devices can be brought out of reset when the host controller is
probed.
Chris Packham (2):
dt-bindings: i2c: mv64xxx: add reset-gpios property
i2c: mv64xxx: add an optional reset-gpios property
.../devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml | 3 +++
drivers/i2c/busses/i2c-mv64xxx.c | 11 +++++++++++
2 files changed, 14 insertions(+)
--
2.42.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] dt-bindings: i2c: mv64xxx: add reset-gpios property
2023-10-12 3:58 [PATCH 0/2] i2c: mv64xxx: reset-gpios Chris Packham
@ 2023-10-12 3:58 ` Chris Packham
2023-10-12 7:42 ` Krzysztof Kozlowski
2023-10-12 3:58 ` [PATCH 2/2] i2c: mv64xxx: add an optional " Chris Packham
1 sibling, 1 reply; 12+ messages in thread
From: Chris Packham @ 2023-10-12 3:58 UTC (permalink / raw)
To: gregory.clement, andi.shyti, robh+dt, krzysztof.kozlowski+dt, conor+dt
Cc: linux-i2c, devicetree, linux-kernel, Chris Packham
Add a reset-gpios property to the marvell,mv64xxx-i2c binding.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Documentation/devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml b/Documentation/devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml
index 461d1c9ee3f7..ac8859aa2545 100644
--- a/Documentation/devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/marvell,mv64xxx-i2c.yaml
@@ -70,6 +70,9 @@ properties:
resets:
maxItems: 1
+ reset-gpios:
+ maxItems: 1
+
dmas:
items:
- description: RX DMA Channel
--
2.42.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] i2c: mv64xxx: add an optional reset-gpios property
2023-10-12 3:58 [PATCH 0/2] i2c: mv64xxx: reset-gpios Chris Packham
2023-10-12 3:58 ` [PATCH 1/2] dt-bindings: i2c: mv64xxx: add reset-gpios property Chris Packham
@ 2023-10-12 3:58 ` Chris Packham
2023-10-12 10:21 ` Andi Shyti
2023-10-21 7:18 ` kernel test robot
1 sibling, 2 replies; 12+ messages in thread
From: Chris Packham @ 2023-10-12 3:58 UTC (permalink / raw)
To: gregory.clement, andi.shyti, robh+dt, krzysztof.kozlowski+dt, conor+dt
Cc: linux-i2c, devicetree, linux-kernel, Chris Packham
Some hardware designs have a GPIO used to control the reset of all the
devices on and I2C bus. It's not possible for every child node to
declare a reset-gpios property as only the first device probed would be
able to successfully request it (the others will get -EBUSY). Represent
this kind of hardware design by associating the reset-gpios with the
parent I2C bus. The reset line will be released prior to the child I2C
devices being probed.
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
drivers/i2c/busses/i2c-mv64xxx.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index efd28bbecf61..b2ca31857cbd 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -160,6 +160,7 @@ struct mv64xxx_i2c_data {
bool clk_n_base_0;
struct i2c_bus_recovery_info rinfo;
bool atomic;
+ struct gpio_desc *reset_gpio;
};
static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
@@ -1083,6 +1084,10 @@ mv64xxx_i2c_probe(struct platform_device *pd)
if (drv_data->irq < 0)
return drv_data->irq;
+ drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(drv_data->reset_gpio))
+ return PTR_ERR(drv_data->reset_gpio);
+
if (pdata) {
drv_data->freq_m = pdata->freq_m;
drv_data->freq_n = pdata->freq_n;
@@ -1121,6 +1126,12 @@ mv64xxx_i2c_probe(struct platform_device *pd)
goto exit_disable_pm;
}
+ if (drv_data->reset_gpio) {
+ udelay(1);
+ gpiod_set_value_cansleep(drv_data->reset_gpio, 0);
+ udelay(1);
+ }
+
rc = request_irq(drv_data->irq, mv64xxx_i2c_intr, 0,
MV64XXX_I2C_CTLR_NAME, drv_data);
if (rc) {
--
2.42.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] dt-bindings: i2c: mv64xxx: add reset-gpios property
2023-10-12 3:58 ` [PATCH 1/2] dt-bindings: i2c: mv64xxx: add reset-gpios property Chris Packham
@ 2023-10-12 7:42 ` Krzysztof Kozlowski
2023-10-12 20:55 ` Chris Packham
0 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-12 7:42 UTC (permalink / raw)
To: Chris Packham, gregory.clement, andi.shyti, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-i2c, devicetree, linux-kernel
On 12/10/2023 05:58, Chris Packham wrote:
> Add a reset-gpios property to the marvell,mv64xxx-i2c binding.
Why?
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] i2c: mv64xxx: add an optional reset-gpios property
2023-10-12 3:58 ` [PATCH 2/2] i2c: mv64xxx: add an optional " Chris Packham
@ 2023-10-12 10:21 ` Andi Shyti
2023-10-12 10:49 ` Peter Rosin
2023-10-21 7:18 ` kernel test robot
1 sibling, 1 reply; 12+ messages in thread
From: Andi Shyti @ 2023-10-12 10:21 UTC (permalink / raw)
To: Chris Packham
Cc: gregory.clement, robh+dt, krzysztof.kozlowski+dt, conor+dt,
linux-i2c, devicetree, linux-kernel
Hi Chris,
...
> static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
> @@ -1083,6 +1084,10 @@ mv64xxx_i2c_probe(struct platform_device *pd)
> if (drv_data->irq < 0)
> return drv_data->irq;
>
> + drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(drv_data->reset_gpio))
> + return PTR_ERR(drv_data->reset_gpio);
if this optional why are we returning in case of error?
> +
> if (pdata) {
> drv_data->freq_m = pdata->freq_m;
> drv_data->freq_n = pdata->freq_n;
> @@ -1121,6 +1126,12 @@ mv64xxx_i2c_probe(struct platform_device *pd)
> goto exit_disable_pm;
> }
>
> + if (drv_data->reset_gpio) {
> + udelay(1);
> + gpiod_set_value_cansleep(drv_data->reset_gpio, 0);
> + udelay(1);
you like busy waiting :-)
What is the reason behind these waits? Is there anything
specified by the datasheet?
If not I would do a more relaxed sleeping like an usleep_range...
what do you think?
Andi
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] i2c: mv64xxx: add an optional reset-gpios property
2023-10-12 10:21 ` Andi Shyti
@ 2023-10-12 10:49 ` Peter Rosin
2023-10-12 20:54 ` Chris Packham
[not found] ` <812dd506-c61b-4967-9b0b-ea35a111bc7f@alliedtelesis.co.nz>
0 siblings, 2 replies; 12+ messages in thread
From: Peter Rosin @ 2023-10-12 10:49 UTC (permalink / raw)
To: Andi Shyti, Chris Packham
Cc: gregory.clement, robh+dt, krzysztof.kozlowski+dt, conor+dt,
linux-i2c, devicetree, linux-kernel
Hi!
2023-10-12 at 12:21, Andi Shyti wrote:
> Hi Chris,
>
> ...
>
>> static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
>> @@ -1083,6 +1084,10 @@ mv64xxx_i2c_probe(struct platform_device *pd)
>> if (drv_data->irq < 0)
>> return drv_data->irq;
>>
>> + drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset", GPIOD_OUT_HIGH);
>> + if (IS_ERR(drv_data->reset_gpio))
>> + return PTR_ERR(drv_data->reset_gpio);
>
> if this optional why are we returning in case of error?
>
>> +
>> if (pdata) {
>> drv_data->freq_m = pdata->freq_m;
>> drv_data->freq_n = pdata->freq_n;
>> @@ -1121,6 +1126,12 @@ mv64xxx_i2c_probe(struct platform_device *pd)
>> goto exit_disable_pm;
>> }
>>
>> + if (drv_data->reset_gpio) {
>> + udelay(1);
>> + gpiod_set_value_cansleep(drv_data->reset_gpio, 0);
>> + udelay(1);
>
> you like busy waiting :-)
>
> What is the reason behind these waits? Is there anything
> specified by the datasheet?
>
> If not I would do a more relaxed sleeping like an usleep_range...
> what do you think?
Since this is apparently not intended to reset the bus driver itself,
but instead various clients connected to the bus, there is not telling
which datasheet to examine. It is simply impossible to hard-code a
correct reset pulse here, when the targets of the pulse are unspecified
and unknown.
I find the reset-gpios naming extremely misleading.
Cheers,
Peter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] i2c: mv64xxx: add an optional reset-gpios property
2023-10-12 10:49 ` Peter Rosin
@ 2023-10-12 20:54 ` Chris Packham
[not found] ` <812dd506-c61b-4967-9b0b-ea35a111bc7f@alliedtelesis.co.nz>
1 sibling, 0 replies; 12+ messages in thread
From: Chris Packham @ 2023-10-12 20:54 UTC (permalink / raw)
To: Peter Rosin, Andi Shyti
Cc: gregory.clement, robh+dt, krzysztof.kozlowski+dt, conor+dt,
linux-i2c, devicetree, linux-kernel
Hi Andi, Peter,
(resend as plain text, sorry to those that get duplicates)
On 12/10/23 23:49, Peter Rosin wrote:
> Hi!
>
> 2023-10-12 at 12:21, Andi Shyti wrote:
>> Hi Chris,
>>
>> ...
>>
>>> static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
>>> @@ -1083,6 +1084,10 @@ mv64xxx_i2c_probe(struct platform_device *pd)
>>> if (drv_data->irq < 0)
>>> return drv_data->irq;
>>>
>>> + drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset", GPIOD_OUT_HIGH);
>>> + if (IS_ERR(drv_data->reset_gpio))
>>> + return PTR_ERR(drv_data->reset_gpio);
>> if this optional why are we returning in case of error?
gpiod_get_optional() will return NULL if the property is not present.
The main error I care about here is -EPROBE_DEFER but I figure other
errors are also relevant. This same kind of pattern is used in other
drivers.
>>> +
>>> if (pdata) {
>>> drv_data->freq_m = pdata->freq_m;
>>> drv_data->freq_n = pdata->freq_n;
>>> @@ -1121,6 +1126,12 @@ mv64xxx_i2c_probe(struct platform_device *pd)
>>> goto exit_disable_pm;
>>> }
>>>
>>> + if (drv_data->reset_gpio) {
>>> + udelay(1);
>>> + gpiod_set_value_cansleep(drv_data->reset_gpio, 0);
>>> + udelay(1);
>> you like busy waiting :-)
sure do.
>> What is the reason behind these waits? Is there anything
>> specified by the datasheet?
Those particular times were lifted from the pca954x mux but they are
fairly arbitrary.
>> If not I would do a more relaxed sleeping like an usleep_range...
>> what do you think?
> Since this is apparently not intended to reset the bus driver itself,
> but instead various clients connected to the bus, there is not telling
> which datasheet to examine. It is simply impossible to hard-code a
> correct reset pulse here, when the targets of the pulse are unspecified
> and unknown.
I could probably follow what similar code does in the pci-mvebu.c driver
and make the delay a property as well. As you're highlighting I can't
possibly pick a value that's right for everyone. We really need to be
told that the hardware design requires X us of delay after reset.
> I find the reset-gpios naming extremely misleading.
I picked that mainly because that's the name of the property for
pci-mvebu.c and a few other end-point devices. The crux of the problem
I'm trying to solve is that I have multiple i2c muxes that share a
common reset GPIO in hardware. I can't associate the GPIO with multiple
devices as the ones that are probed after the first will get -EBUSY. I
can cheat and not have a reset-gpios property on the other muxes but
then if the GPIO is deferred (because the controller driver hasn't been
loaded) the muxes don't get reset at all.
> Cheers,
> Peter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] dt-bindings: i2c: mv64xxx: add reset-gpios property
2023-10-12 7:42 ` Krzysztof Kozlowski
@ 2023-10-12 20:55 ` Chris Packham
0 siblings, 0 replies; 12+ messages in thread
From: Chris Packham @ 2023-10-12 20:55 UTC (permalink / raw)
To: Krzysztof Kozlowski, gregory.clement, andi.shyti, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-i2c, devicetree, linux-kernel
Hi Krysztof,
(resend as plain text)
On 12/10/23 20:42, Krzysztof Kozlowski wrote:
> On 12/10/2023 05:58, Chris Packham wrote:
>> Add a reset-gpios property to the marvell,mv64xxx-i2c binding.
> Why?
Sorry about that. I put a better explanation in the corresponding driver
change but then over-edited when doing the device-tree change. I'll try
and explain things a bit better in v2.
>> Signed-off-by: Chris Packham<chris.packham@alliedtelesis.co.nz>
>> ---
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] i2c: mv64xxx: add an optional reset-gpios property
[not found] ` <812dd506-c61b-4967-9b0b-ea35a111bc7f@alliedtelesis.co.nz>
@ 2023-10-13 9:34 ` Andi Shyti
2023-10-15 20:20 ` Chris Packham
0 siblings, 1 reply; 12+ messages in thread
From: Andi Shyti @ 2023-10-13 9:34 UTC (permalink / raw)
To: Chris Packham
Cc: Peter Rosin, gregory.clement, robh+dt, krzysztof.kozlowski+dt,
conor+dt, linux-i2c, devicetree, linux-kernel
Hi Chris,
...
> static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
> @@ -1083,6 +1084,10 @@ mv64xxx_i2c_probe(struct platform_device *pd)
> if (drv_data->irq < 0)
> return drv_data->irq;
>
> + drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(drv_data->reset_gpio))
> + return PTR_ERR(drv_data->reset_gpio);
>
> if this optional why are we returning in case of error?
>
> gpiod_get_optional() will return NULL if the property is not present. The main
> error I care about here is -EPROBE_DEFER but I figure other errors are also
> relevant. This same kind of pattern is used in other drivers.
we already discussed about this, I don't have a strong opinion,
you can leave it as it is... I recon this is a matter of pure
taste.
Would you just mind adding an error message using
dev_err_probe()?
Thanks,
Andi
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] i2c: mv64xxx: add an optional reset-gpios property
2023-10-13 9:34 ` Andi Shyti
@ 2023-10-15 20:20 ` Chris Packham
2023-10-16 7:31 ` Peter Rosin
0 siblings, 1 reply; 12+ messages in thread
From: Chris Packham @ 2023-10-15 20:20 UTC (permalink / raw)
To: Andi Shyti
Cc: Peter Rosin, gregory.clement, robh+dt, krzysztof.kozlowski+dt,
conor+dt, linux-i2c, devicetree, linux-kernel
On 13/10/23 22:34, Andi Shyti wrote:
> Hi Chris,
>
> ...
>
>> static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
>> @@ -1083,6 +1084,10 @@ mv64xxx_i2c_probe(struct platform_device *pd)
>> if (drv_data->irq < 0)
>> return drv_data->irq;
>>
>> + drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset", GPIOD_OUT_HIGH);
>> + if (IS_ERR(drv_data->reset_gpio))
>> + return PTR_ERR(drv_data->reset_gpio);
>>
>> if this optional why are we returning in case of error?
>>
>> gpiod_get_optional() will return NULL if the property is not present. The main
>> error I care about here is -EPROBE_DEFER but I figure other errors are also
>> relevant. This same kind of pattern is used in other drivers.
> we already discussed about this, I don't have a strong opinion,
> you can leave it as it is... I recon this is a matter of pure
> taste.
I think in this case it would actually make things uglier because I'd
have to check for -EPROBE_DEFER. So something like
drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset",
GPIOD_OUT_HIGH);
if (IS_ERR(drv_data->reset_gpio) && PTR_ERR(drv_data->reset_gpio)
== -EPROBE_DEFER)
return PTR_ERR(drv_data->reset_gpio);
else
drv_data->reset_gpio = NULL;
I could probably come up with something less ugly with a local variable
or two but nothing as tidy as just returning on error.
>
> Would you just mind adding an error message using
> dev_err_probe()?
Yep sure. Will include in the next round.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] i2c: mv64xxx: add an optional reset-gpios property
2023-10-15 20:20 ` Chris Packham
@ 2023-10-16 7:31 ` Peter Rosin
0 siblings, 0 replies; 12+ messages in thread
From: Peter Rosin @ 2023-10-16 7:31 UTC (permalink / raw)
To: Chris Packham, Andi Shyti
Cc: gregory.clement, robh+dt, krzysztof.kozlowski+dt, conor+dt,
linux-i2c, devicetree, linux-kernel
Hi!
2023-10-15 at 22:20, Chris Packham wrote:
>
> On 13/10/23 22:34, Andi Shyti wrote:
>> Hi Chris,
>>
>> ...
>>
>>> static struct mv64xxx_i2c_regs mv64xxx_i2c_regs_mv64xxx = {
>>> @@ -1083,6 +1084,10 @@ mv64xxx_i2c_probe(struct platform_device *pd)
>>> if (drv_data->irq < 0)
>>> return drv_data->irq;
>>>
>>> + drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset", GPIOD_OUT_HIGH);
>>> + if (IS_ERR(drv_data->reset_gpio))
>>> + return PTR_ERR(drv_data->reset_gpio);
>>>
>>> if this optional why are we returning in case of error?
>>>
>>> gpiod_get_optional() will return NULL if the property is not present. The main
>>> error I care about here is -EPROBE_DEFER but I figure other errors are also
>>> relevant. This same kind of pattern is used in other drivers.
>> we already discussed about this, I don't have a strong opinion,
>> you can leave it as it is... I recon this is a matter of pure
>> taste.
>
> I think in this case it would actually make things uglier because I'd
> have to check for -EPROBE_DEFER. So something like
>
> drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset",
> GPIOD_OUT_HIGH);
> if (IS_ERR(drv_data->reset_gpio) && PTR_ERR(drv_data->reset_gpio)
> == -EPROBE_DEFER)
> return PTR_ERR(drv_data->reset_gpio);
> else
> drv_data->reset_gpio = NULL;
>
> I could probably come up with something less ugly with a local variable
> or two but nothing as tidy as just returning on error.
I disagree with this, in my opinion it should just be:
drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(drv_data->reset_gpio))
return dev_err_probe(&pd->dev, PTR_ERR(drv_data->reset_gpio), ...);
My take is that optional gpios (or whatever) are optional because it is
optional to specify them in the devicetree (or whereever), but if the
optional item is indeed specified, it is just like any other error if
it is not working.
$0.02
Cheers,
Peter
>>
>> Would you just mind adding an error message using
>> dev_err_probe()?
>
> Yep sure. Will include in the next round.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] i2c: mv64xxx: add an optional reset-gpios property
2023-10-12 3:58 ` [PATCH 2/2] i2c: mv64xxx: add an optional " Chris Packham
2023-10-12 10:21 ` Andi Shyti
@ 2023-10-21 7:18 ` kernel test robot
1 sibling, 0 replies; 12+ messages in thread
From: kernel test robot @ 2023-10-21 7:18 UTC (permalink / raw)
To: Chris Packham, gregory.clement, andi.shyti, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: oe-kbuild-all, linux-i2c, devicetree, linux-kernel, Chris Packham
Hi Chris,
kernel test robot noticed the following build errors:
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on linus/master v6.6-rc6 next-20231020]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Chris-Packham/dt-bindings-i2c-mv64xxx-add-reset-gpios-property/20231017-102540
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
patch link: https://lore.kernel.org/r/20231012035838.2804064-3-chris.packham%40alliedtelesis.co.nz
patch subject: [PATCH 2/2] i2c: mv64xxx: add an optional reset-gpios property
config: i386-buildonly-randconfig-001-20231021 (https://download.01.org/0day-ci/archive/20231021/202310211508.57kpcEto-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231021/202310211508.57kpcEto-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310211508.57kpcEto-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/i2c/busses/i2c-mv64xxx.c: In function 'mv64xxx_i2c_probe':
>> drivers/i2c/busses/i2c-mv64xxx.c:1028:32: error: implicit declaration of function 'devm_gpiod_get_optional'; did you mean 'devm_clk_get_optional'? [-Werror=implicit-function-declaration]
1028 | drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset", GPIOD_OUT_HIGH);
| ^~~~~~~~~~~~~~~~~~~~~~~
| devm_clk_get_optional
>> drivers/i2c/busses/i2c-mv64xxx.c:1028:75: error: 'GPIOD_OUT_HIGH' undeclared (first use in this function)
1028 | drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset", GPIOD_OUT_HIGH);
| ^~~~~~~~~~~~~~
drivers/i2c/busses/i2c-mv64xxx.c:1028:75: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/i2c/busses/i2c-mv64xxx.c:1068:17: error: implicit declaration of function 'gpiod_set_value_cansleep' [-Werror=implicit-function-declaration]
1068 | gpiod_set_value_cansleep(drv_data->reset_gpio, 0);
| ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +1028 drivers/i2c/busses/i2c-mv64xxx.c
983
984 static int
985 mv64xxx_i2c_probe(struct platform_device *pd)
986 {
987 struct mv64xxx_i2c_data *drv_data;
988 struct mv64xxx_i2c_pdata *pdata = dev_get_platdata(&pd->dev);
989 int rc;
990
991 if ((!pdata && !pd->dev.of_node))
992 return -ENODEV;
993
994 drv_data = devm_kzalloc(&pd->dev, sizeof(struct mv64xxx_i2c_data),
995 GFP_KERNEL);
996 if (!drv_data)
997 return -ENOMEM;
998
999 drv_data->reg_base = devm_platform_ioremap_resource(pd, 0);
1000 if (IS_ERR(drv_data->reg_base))
1001 return PTR_ERR(drv_data->reg_base);
1002
1003 strscpy(drv_data->adapter.name, MV64XXX_I2C_CTLR_NAME " adapter",
1004 sizeof(drv_data->adapter.name));
1005
1006 init_waitqueue_head(&drv_data->waitq);
1007 spin_lock_init(&drv_data->lock);
1008
1009 /* Not all platforms have clocks */
1010 drv_data->clk = devm_clk_get(&pd->dev, NULL);
1011 if (IS_ERR(drv_data->clk)) {
1012 if (PTR_ERR(drv_data->clk) == -EPROBE_DEFER)
1013 return -EPROBE_DEFER;
1014 drv_data->clk = NULL;
1015 }
1016
1017 drv_data->reg_clk = devm_clk_get(&pd->dev, "reg");
1018 if (IS_ERR(drv_data->reg_clk)) {
1019 if (PTR_ERR(drv_data->reg_clk) == -EPROBE_DEFER)
1020 return -EPROBE_DEFER;
1021 drv_data->reg_clk = NULL;
1022 }
1023
1024 drv_data->irq = platform_get_irq(pd, 0);
1025 if (drv_data->irq < 0)
1026 return drv_data->irq;
1027
> 1028 drv_data->reset_gpio = devm_gpiod_get_optional(&pd->dev, "reset", GPIOD_OUT_HIGH);
1029 if (IS_ERR(drv_data->reset_gpio))
1030 return PTR_ERR(drv_data->reset_gpio);
1031
1032 if (pdata) {
1033 drv_data->freq_m = pdata->freq_m;
1034 drv_data->freq_n = pdata->freq_n;
1035 drv_data->adapter.timeout = msecs_to_jiffies(pdata->timeout);
1036 drv_data->offload_enabled = false;
1037 memcpy(&drv_data->reg_offsets, &mv64xxx_i2c_regs_mv64xxx, sizeof(drv_data->reg_offsets));
1038 } else if (pd->dev.of_node) {
1039 rc = mv64xxx_of_config(drv_data, &pd->dev);
1040 if (rc)
1041 return rc;
1042 }
1043
1044 rc = mv64xxx_i2c_init_recovery_info(drv_data, &pd->dev);
1045 if (rc == -EPROBE_DEFER)
1046 return rc;
1047
1048 drv_data->adapter.dev.parent = &pd->dev;
1049 drv_data->adapter.algo = &mv64xxx_i2c_algo;
1050 drv_data->adapter.owner = THIS_MODULE;
1051 drv_data->adapter.class = I2C_CLASS_DEPRECATED;
1052 drv_data->adapter.nr = pd->id;
1053 drv_data->adapter.dev.of_node = pd->dev.of_node;
1054 platform_set_drvdata(pd, drv_data);
1055 i2c_set_adapdata(&drv_data->adapter, drv_data);
1056
1057 pm_runtime_set_autosuspend_delay(&pd->dev, MSEC_PER_SEC);
1058 pm_runtime_use_autosuspend(&pd->dev);
1059 pm_runtime_enable(&pd->dev);
1060 if (!pm_runtime_enabled(&pd->dev)) {
1061 rc = mv64xxx_i2c_runtime_resume(&pd->dev);
1062 if (rc)
1063 goto exit_disable_pm;
1064 }
1065
1066 if (drv_data->reset_gpio) {
1067 udelay(1);
> 1068 gpiod_set_value_cansleep(drv_data->reset_gpio, 0);
1069 udelay(1);
1070 }
1071
1072 rc = request_irq(drv_data->irq, mv64xxx_i2c_intr, 0,
1073 MV64XXX_I2C_CTLR_NAME, drv_data);
1074 if (rc) {
1075 dev_err(&drv_data->adapter.dev,
1076 "mv64xxx: Can't register intr handler irq%d: %d\n",
1077 drv_data->irq, rc);
1078 goto exit_disable_pm;
1079 } else if ((rc = i2c_add_numbered_adapter(&drv_data->adapter)) != 0) {
1080 dev_err(&drv_data->adapter.dev,
1081 "mv64xxx: Can't add i2c adapter, rc: %d\n", -rc);
1082 goto exit_free_irq;
1083 }
1084
1085 return 0;
1086
1087 exit_free_irq:
1088 free_irq(drv_data->irq, drv_data);
1089 exit_disable_pm:
1090 pm_runtime_disable(&pd->dev);
1091 if (!pm_runtime_status_suspended(&pd->dev))
1092 mv64xxx_i2c_runtime_suspend(&pd->dev);
1093
1094 return rc;
1095 }
1096
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-10-21 7:19 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-12 3:58 [PATCH 0/2] i2c: mv64xxx: reset-gpios Chris Packham
2023-10-12 3:58 ` [PATCH 1/2] dt-bindings: i2c: mv64xxx: add reset-gpios property Chris Packham
2023-10-12 7:42 ` Krzysztof Kozlowski
2023-10-12 20:55 ` Chris Packham
2023-10-12 3:58 ` [PATCH 2/2] i2c: mv64xxx: add an optional " Chris Packham
2023-10-12 10:21 ` Andi Shyti
2023-10-12 10:49 ` Peter Rosin
2023-10-12 20:54 ` Chris Packham
[not found] ` <812dd506-c61b-4967-9b0b-ea35a111bc7f@alliedtelesis.co.nz>
2023-10-13 9:34 ` Andi Shyti
2023-10-15 20:20 ` Chris Packham
2023-10-16 7:31 ` Peter Rosin
2023-10-21 7:18 ` kernel test robot
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).