On Mon, Feb 04, 2019 at 12:29:53PM +0100, Linus Walleij wrote: > On Thu, Jan 31, 2019 at 1:08 PM Matti Vaittinen > <matti.vaittinen@xxxxxxxxxxxxxxxxx> wrote: > > > ROHM BD70528 PMIC has 4 GPIO pins. Allow them to be > > controlled by GPIO framework. > > > > IRQs are handled by regmap-irq and GPIO driver is not > > aware of the irq usage. > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx> > (...) > > I dropped the review-by from Linus Walleij because I would like to > > get a comment on if locking is required when we check the direction > > in order to detect the correct register for getting the pin state. > > I don't know that. You isn't regmap locking inherently? This direction fetching and then the value reading are two separate operations at regmap-level. I guess the regmap can't be holding any locks during two indpendent operations. > > > My initial feeling is that locking makes no sense. > > Mine too. Yep. I change the comment to explain why I think this race condition is Ok. We can fix it if it ever turns out to be an issue. > > > + bdgpio->gpio.get_direction = &bd70528_get_direction; > > + bdgpio->gpio.direction_input = &bd70528_direction_input; > > + bdgpio->gpio.direction_output = &bd70528_direction_output; > > + bdgpio->gpio.set_config = &bd70528_gpio_set_config; > > + bdgpio->gpio.can_sleep = true; > > + bdgpio->gpio.get = &bd70528_gpio_get; > > + bdgpio->gpio.set = &bd70528_gpio_set; > > Drop the &ersand in from of the functions. All functions > are pointers. Yes they are. This & in front of functions when initializing a pointer with their addresss is my old habit. I liked to consistently use & when taking address of any beast, whether it was a variable or function. Getting rid of old habits is not so easy :/ I'll clean thisi at v5 and put back your Reviewed-by =) Thanks > > With that: > Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx> > > Yours, > Linus Walleij -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
