[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4468becb-dc03-4832-aa03-5f597023fcb2@linaro.org>
Date: Wed, 8 May 2024 17:12:51 +0100
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Johan Hovold <johan+linaro@...nel.org>, Lee Jones <lee@...nel.org>,
Mark Brown <broonie@...nel.org>, Linus Walleij <linus.walleij@...aro.org>,
Bjorn Andersson <andersson@...nel.org>
Cc: Konrad Dybcio <konrad.dybcio@...aro.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Liam Girdwood <lgirdwood@...il.com>,
Das Srinagesh <quic_gurus@...cinc.com>,
Satya Priya <quic_c_skakit@...cinc.com>, Stephen Boyd <swboyd@...omium.org>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org
Subject: Re: [PATCH 03/13] mfd: pm8008: deassert reset on probe
On 06/05/2024 16:08, Johan Hovold wrote:
> Request and deassert any (optional) reset gpio during probe in case it
> has been left asserted by the boot firmware.
>
> Note the reset line is not asserted to avoid reverting to the default
> I2C address in case the firmware has configured an alternate address.
>
> Signed-off-by: Johan Hovold <johan+linaro@...nel.org>
> ---
> drivers/mfd/qcom-pm8008.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c
> index d53c987b0d49..d0f190c2ea2b 100644
> --- a/drivers/mfd/qcom-pm8008.c
> +++ b/drivers/mfd/qcom-pm8008.c
> @@ -4,6 +4,7 @@
> */
>
> #include <linux/bitops.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> @@ -158,6 +159,7 @@ static struct regmap_config qcom_mfd_regmap_cfg = {
> static int pm8008_probe(struct i2c_client *client)
> {
> struct regmap_irq_chip_data *irq_data;
> + struct gpio_desc *reset;
> int rc;
> struct device *dev;
> struct regmap *regmap;
> @@ -169,6 +171,10 @@ static int pm8008_probe(struct i2c_client *client)
>
> i2c_set_clientdata(client, regmap);
>
> + reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(reset))
> + return PTR_ERR(reset);
> +
> if (of_property_read_bool(dev->of_node, "interrupt-controller")) {
> rc = devm_regmap_add_irq_chip(dev, regmap, client->irq,
> IRQF_SHARED, 0, &pm8008_irq_chip, &irq_data);
So not resetting is fine and I understand you want to retain the address
given by the firmware, I think that's the right thing to do.
You can add this now
Tested-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
In addition to adding a small delay suggested by Andy - a few
microseconds pick a number, I think you should verify the chip is out of
reset as we would do with many other i2c devices.
A common pattern with i2c devices is take the device out of reset then
read back an identity register as a smoke test.
Take drivers/media/i2c/ov8856.c::ov8856_identify_module() as an example.
In this case, suggest reading REVID_PERPH_TYPE @ 0x104 and
REVID_PERPH_SUBTYPE @ 0x105
REVID_PERPH_TYPE @ 0x104 == 0x51 (PMIC)
REVID_PERPH_SUBYTE @ 0x105 == 0x2C (PM8008)
You can then add my
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
---
bod
Powered by blists - more mailing lists