| Message ID | 20221214142725.23881-1-hhhawa@amazon.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/1] i2c: designware: set pinctrl recovery information from device pinctrl | expand |
On 12/15/2022 4:06 PM, Linus Walleij wrote: >> Getter with a stub sounds better to me, so you won't access some device core >> fields. >> >> Linus, what do you think about all these (including previous paragraph)? > A getter may be a good solution, it depends, it can also be pushed > somewhere local in the designware i2c driver can it not? > I am thinking that the rest of the code that is using that field is > certainly not going to work without pinctrl either. the compilation failure occurs on platform which not define the CONFIG_PINCTRL , most of the pinctrl APIs are wrapped with PINCTRL config, for example pinctrl_select_state() or devm_pinctrl_get(). In addition if we add the getter in pinctrl/devinfo.h other drivers may access the pins field without re-call devm_pinctrl_get(). Thanks, Hanna
On 12/15/2022 12:27 PM, Andy Shevchenko wrote: > OK, but why that function doesn't use the dev->pins->p if it's defined? > (As a fallback when rinfo->pinctrl is NULL. > The solution will look like the below diff (get_device_pinctrl() is new function that i've added that return the dev->pins->p) diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 7539b0740351..469344d4ee43 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -34,6 +34,7 @@ #include <linux/of.h> #include <linux/of_irq.h> #include <linux/pinctrl/consumer.h> +#include <linux/pinctrl/devinfo.h> #include <linux/pm_domain.h> #include <linux/pm_runtime.h> #include <linux/pm_wakeirq.h> @@ -282,7 +283,11 @@ static void i2c_gpio_init_pinctrl_recovery(struct i2c_adapter *adap) { struct i2c_bus_recovery_info *bri = adap->bus_recovery_info; struct device *dev = &adap->dev; - struct pinctrl *p = bri->pinctrl; + struct pinctrl *p; + + if (!bri->pinctrl) + bri->pinctrl = get_device_pinctrl(dev->parent); + p = bri->pinctrl; > Wolfram? > > Hanna, it seems you missed I²C maintainer to Cc... I based my CC/TO on get_maintainer.pl script. Will make sure that Wolfram on CC next time.
On 12/16/2022 4:46 PM, Andy Shevchenko wrote: > Can be simplified with help of Elvis: > > p = bri->pinctrl ?: dev_pinctrl(dev->parent); Can't use this, as need to set the bri->pinctrl to dev_pinctrl() in case it's not set by the driver. >> I based my CC/TO on get_maintainer.pl script. Will make sure that Wolfram on >> CC next time. > All the same about Linus W., who is pin control subsystem maintainer, and be > sure the respective mailing lists are also included. Sure, thanks Thanks, Hanna
On Mon, Dec 19, 2022 at 09:35:37PM +0200, Hawa, Hanna wrote: > On 12/16/2022 4:46 PM, Andy Shevchenko wrote: > > Can be simplified with help of Elvis: > > > > p = bri->pinctrl ?: dev_pinctrl(dev->parent); > > Can't use this, as need to set the bri->pinctrl to dev_pinctrl() in case > it's not set by the driver. I guess you can. I will answer to the v3.
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c index dc3c5a15a95b..16a4cd68567c 100644 --- a/drivers/i2c/busses/i2c-designware-master.c +++ b/drivers/i2c/busses/i2c-designware-master.c @@ -17,6 +17,7 @@ #include <linux/interrupt.h> #include <linux/io.h> #include <linux/module.h> +#include <linux/pinctrl/devinfo.h> #include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/reset.h> @@ -832,6 +833,9 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev) struct i2c_adapter *adap = &dev->adapter; struct gpio_desc *gpio; + if (dev->dev->pins && dev->dev->pins->p) + rinfo->pinctrl = dev->dev->pins->p; + gpio = devm_gpiod_get_optional(dev->dev, "scl", GPIOD_OUT_HIGH); if (IS_ERR_OR_NULL(gpio)) return PTR_ERR_OR_ZERO(gpio);
The current implementation of designware recovery mechanism fit for specific device (Intel / Altera Cyclone V SOC) which have two separated "wired" GPIOs to the i2c bus via the SOC FPGA for the i2c recovery. Make pinctrl recovery information to points to the device pinctrl by setting the rinfo->pinctrl to dev->pins->p. Change Log v1->v2: - set the rinfo->pinctrl to dev->pins->p instead calling devm_pinctrl_get(). Signed-off-by: Hanna Hawa <hhhawa@amazon.com> --- drivers/i2c/busses/i2c-designware-master.c | 4 ++++ 1 file changed, 4 insertions(+)