On Wed, 29 Jan 2025 16:29:01 +0200 Alisa-Dariana Roman <alisadariana@xxxxxxxxx> wrote: > Thank you all for your feedback! Here is the updated series of patches! > > I addressed all the replies' points, except for the one about the size of the > avail array being 1 when the pga/odr pins are pin-strapped. David raised a very > good point, but, for now, I left the size fixed to 4, since the functions for > setting the values return error anyway when they are pin-strapped. Shouldn't have them pretending they can be changed when they can't (which is what I think you mean about size fixed to 4). So this needs resolving. > > I thought of 3 approaches: > - dynamic allocation for the avail arrays Probably best avoided. > - different avail array for the 2 different cases (pin-strap or gpios) That works for me. > - different channels array for the 2 different cases (probably too much) It is a bit odd to list avail when only one value, but not wrong as such so I think no need to go this far, though maybe there is a userspace library this might confuse? > > If the current setup if not good enough, which approach would be the best? > > Kind regards, > Alisa-Dariana Roman. > > --- > > v2: https://lore.kernel.org/all/20250122132821.126600-1-alisa.roman@xxxxxxxxxx/ > > v2 -> v3: > - correct binding title > - remove clksel_state and clksel_gpio, assume the clksel pin is always > pinstrapped > - rephrase clocks description accordingly > - simplify binding constraints > - specify in binding description that PDOWN must be connected to SPI's > controller's CS > - add minItems for gpios in bindings > - make scope explicit for mutex guard > - remove spi irq check > - add id_table to spi_driver struct > - changed comments as suggested > - use spi_message_init_with_transfers() > - default returns an error in ad7191_set_mode() > - replace hard-coded 2 with st->pga_gpios->ndescs > - use gpiod_set_array_value_cansleep() > - change .storagebits to 32 > - check return value for ad_sd_init() > - change to adi,odr-value and adi,pga-value, which now accepts the value as > suggested > - modify variables names and refactor the setup of odr and pga gpios, > indexes and available arrays into ad7191_config_setup(), since they are all > related > - add ad7191.rst > > v1: https://lore.kernel.org/all/20241221155926.81954-1-alisa.roman@xxxxxxxxxx/ > > v1 -> v2: > - removed patch adding function in ad_sigma_delta.h/.c > - added a function set_cs() for asserting/deasserting the cs > - handle pinstrapping cases > - refactored all clock handling > - updated bindings: corrected and added new things > - -> address of the channels is used in set_channel() > - addressed all the other changes >
