Messages in this thread | | | Date | Sat, 2 Sep 2023 00:10:39 +0300 | From | Andy Shevchenko <> | Subject | Re: [PATCH] gpio: sim: don't fiddle with GPIOLIB private members |
| |
On Fri, Sep 01, 2023 at 08:32:40PM +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 leverage the fact that > once created for a software node, struct device is accessible from that > fwnode_handle. We don't need to dereference gpio_device. > > While at it: fix one line break and remove the untrue part about > configfs callbacks using dev_get_drvdata() from a comment.
...
> -static void gpio_sim_free(struct gpio_chip *gc, unsigned int offset)
Why is this?
> +static int gpio_sim_request(struct gpio_chip *gc, unsigned int offset) > { > struct gpio_sim_chip *chip = gpiochip_get_data(gc); > > scoped_guard(mutex, &chip->lock) > + __set_bit(offset, chip->request_map); > + > + return 0; > +} > + > +static void gpio_sim_free(struct gpio_chip *gc, unsigned int offset) > +{ > + struct gpio_sim_chip *chip = gpiochip_get_data(gc); > + > + scoped_guard(mutex, &chip->lock) { > __assign_bit(offset, chip->value_map, > !!test_bit(offset, chip->pull_map)); > + __clear_bit(offset, chip->request_map); > + } > }
Seems to me like you. shuffled the order of the two functions. Can you leave _free() at the same location in the file?
...
> - /* Used by sysfs and configfs callbacks. */ > - dev_set_drvdata(&gc->gpiodev->dev, chip); > + /* Used by sysfs callbacks. */ > + dev_set_drvdata(swnode->dev, chip);
dev pointer of firmware node is solely for dev links. Is it the case here? Seems to me you luckily abuse it.
-- With Best Regards, Andy Shevchenko
| |