>Am 2023-04-24 17:39, schrieb Sahin, Okan: >>> Am 2023-04-09 16:25, schrieb Sahin, Okan: >>>>> Fri, Apr 07, 2023 at 03:48:25PM +0200, Linus Walleij kirjoitti: >>>>>> On Wed, Apr 5, 2023 at 3:57 PM Michael Walle <michael@xxxxxxxx> >>>>>> wrote: >>>>>> >>>>>> > OTOH I'm not sure the driver is doing it correctly, because it also >>>>>> > seems to switch the pullup resisters together with the direction. >>>>>> > I'm not sure that is correct. So there might be just one register >>>>>> > involved after all and the GPIO_REGMAP should work again. >>>>>> >>>>>> I'm pretty sure that should be in the .set_config() callback. >>>>>> >>>>>> > Also, according to the datasheet this has some nv memory (to set the >>>>>> > initial state of the GPIOs [?]). So it should really be a >>>>>> > multi-function device. I'm not sure if this has to be considered >>>>>> > right from the beginning or if the device support can start with >>>>>> > GPIO only and later be transitioned to a full featured MFD (probably with >>> nvmem >>>>> support). >>>>>> >>>>>> That's a bit of a soft definition. >>>>>> >>>>>> If the chip is *only* doing GPIO and nvram it can be a GPIO-only >>>>>> device I think. >>>>>> >>>>>> The precedent is a ton of ethernet drivers with nvram for storing >>>>>> e.g. >>>>>> the MAC address. We don't make all of those into MFDs, as the nvram >>>>>> is >>>>>> closely tied to the one and only function of the block. >>>>> >>>>> I agree with Linus. This should be part of the actual (main) driver >>>>> for the chip as many >>>>> do (like USB to serial adapters that have GPIO capability). >>> >>> You mean the gpio driver is calling nvmem_register()? Yeah I agree, >>> that >>> should work. >>> >>>> I think gpio_regmap is not suitable for this driver as Michael >>>> stated. >>>> https://www.analog.com/media/en/technical-documentation/data- >>> sheets/ds4520.pdf >>>> Please check block diagram. There are two input registers that >>>> control >>>> gpio state >>>> so gpio_regmap does not look ok for this. Am I missing something? >>> >>> You mean F8/F9? That will work as they are for different GPIOs. What >>> doesn't work with gpio-regmap is when you need to modify two different >>> registers for one GPIO. Have a look at gpio_regmap_get() and >>> gpio_regmap_set(). If the default gpio_regmap_simple_xlate() doesn't >>> work >>> you can use your own .xlate() op. >>> >> >> Actually, I checked the functions that you suggested, but as far as I >> understand >> they might work if there would be one bit to set direction or value. >> However, >> this is not the case for ds4520. In other words, if I want to set the >> gpio direction >> as output, I need to set a corresponding bit for both F0 and F1 >> registers. > >I can't follow. F0/F1 is for the pull-up. That was actually my initial >question and Linus said, that should probably be done in a seperate >.set_config operation not together with a direction change. I think I understand what you are trying to say so far. I did not have too much experience related to gpio. I will set pull_up register in .set_config However, I did not understand where its parameters come from. set_config(struct gpio_chip *chip, unsigned int offset, unsigned long config) It might be trivial question, but Where does config come from? At the end, I should rewrite the code using regmap_gpio, right? So if I rewrite code using regmap_gpio, how can I replace set_config(...)? > >> In the document, you can see block diagram. I do not know why, but >> design is >> not standard that’s why I think I can not use gpio-regmap. >> >>>> Also, at this point I am not planning to add nvmem support. >>> >>> That is a pity, because that is the whole use case for this gpio >>> expander, >>> no? "Programmable Replacement for Mechanical Jumpers and Switches" >> >> I can set "SEE" bit as "0" in the Configuration Register to write >> EEPROM so it might solve >> issue. > >If you do that unconditionally, that might wear out the EEPROM, >though. > >-michael Hi Michael, Thank you for your support. Regards, Okan Sahin
