Messages in this thread | | | Date | Tue, 5 Sep 2023 15:05:17 +0300 | From | Andy Shevchenko <> | Subject | Re: [PATCH v3] gpio: sim: don't fiddle with GPIOLIB private members |
| |
On Tue, Sep 05, 2023 at 10:24:13AM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > We access internals of struct gpio_device and struct gpio_desc because > it's easier but it can actually be avoided and we're working towards a > better encapsulation of GPIO data structures across the kernel so let's > start at home. > > Instead of checking gpio_desc flags, let's just track the requests of > GPIOs in the driver. We also already store the information about > direction of simulated lines. > > For kobjects needed by sysfs callbacks: we can iterate over the children > devices of the top-level platform device and compare their fwnodes > against the one passed to the init function from probe. > > While at it: fix one line break and remove the untrue part about > configfs callbacks using dev_get_drvdata() from a comment.
...
> v2 -> v3: > - don't use fwnode internal fields, instead: iterate over the platform > device's children and locate the GPIO device
Thank you!
...
> @@ -181,7 +178,7 @@ static int gpio_sim_get_direction(struct gpio_chip *gc, unsigned int offset)
> static int gpio_sim_set_config(struct gpio_chip *gc, > - unsigned int offset, unsigned long config) > + unsigned int offset, unsigned long config)
Looking at other prototypes, it can be
static int gpio_sim_set_config(struct gpio_chip *gc, unsigned int offset, unsigned long config)
...
> +static int gpio_sim_chip_set_device(struct device *dev, void *data) > +{ > + struct gpio_sim_chip *chip = data;
> + if (chip->swnode == dev->fwnode) {
Please do not dereference fwnode from the struct device, we have an API! device_match_fwnode()
> + chip->dev = dev; > + return 1; > + } > + > + return 0; > +}
...
> + chip->swnode = swnode; > + ret = device_for_each_child(dev, chip, gpio_sim_chip_set_device); > + if (!ret) > + return -ENODEV;
Can bus_find_device_by_fwnode() be used here?
-- With Best Regards, Andy Shevchenko
| |