Re: [PATCH 1/4] spi: xcomm: add gpiochip support

[Nuno Sá <noname.nuno@xxxxxxxxx>]

On Thu, 2024-07-04 at 16:45 +0100, Mark Brown wrote:
> On Thu, Jul 04, 2024 at 04:00:15PM +0200, Nuno Sá wrote:
> > On Thu, 2024-07-04 at 14:53 +0100, Mark Brown wrote:
> > > On Thu, Jul 04, 2024 at 03:49:12PM +0200, Nuno Sa wrote:
> > > 
> > > > +static int spi_xcomm_gpio_get_value(struct gpio_chip *chip, unsigned int
> > > > offset)
> > > > +{
> > > > +	struct spi_xcomm *spi_xcomm = gpiochip_get_data(chip);
> > > > +
> > > > +	return spi_xcomm->gpio_val;
> > > > +}
> 
> > > It seems like the hardware doesn't support input at all so should there
> > > even be a get operation?  gpiolib appears to cope fine with omitting it.
> 
> > Just following recommendations :)
> 
> > https://elixir.bootlin.com/linux/v6.10-rc6/source/include/linux/gpio/driver.h#L336
> 
> That comment is for get_direction(), not get().

Oh right, sorry. For some reason I assumed get_direction()... Well, get() was mainly
to not get an error when reading the GPIO value from userspace (eg:
/sys/clagg/gpio). 

That said, we're just caching the value in the driver in case the i2c transfer does
not fail so I guess yes, we can make this even simpler and remove get() and
'gpio_val'. Userspace apps can very well cache the value themselves.

- Nuno Sá