On Wed, Mar 20, 2024 at 10:37 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > > On 19/03/2024 22:27, Lothar Rubusch wrote: > > Adds the spi-3wire feature and adds general refactoring to the > > Add > > > iio driver. > > > > The patch moves driver wide constants and fields into the > > Please do not use "This commit/patch/change", but imperative mood. See > longer explanation here: > https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > > > header. Thereby reduces redundant info struct definitions. > > Allows to pass a function pointer from SPI/I2C specific probe, > > and smaller refactorings. A regmap_update_bits() in the core > > file replaces the regmap_write() to format_data. > > > > Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx> > > --- Agree > > static int adxl345_spi_probe(struct spi_device *spi) > > { > > + const struct adxl345_chip_info *chip_data; > > struct regmap *regmap; > > > > + /* Retrieve device name to pass it as driver specific data */ > > + chip_data = device_get_match_data(&spi->dev); > > + if (!chip_data) > > + chip_data = (const struct adxl345_chip_info *) spi_get_device_id(spi)->driver_data; > > Are you sure you need the cast? > > And why aren't you using spi_get_device_match_data()? Agree > > + > > /* Bail out if max_speed_hz exceeds 5 MHz */ > > if (spi->max_speed_hz > ADXL345_MAX_SPI_FREQ_HZ) > > return dev_err_probe(&spi->dev, -EINVAL, "SPI CLK, %d Hz exceeds 5 MHz\n", > > spi->max_speed_hz); > > > > regmap = devm_regmap_init_spi(spi, &adxl345_spi_regmap_config); > > - if (IS_ERR(regmap)) > > - return dev_err_probe(&spi->dev, PTR_ERR(regmap), "Error initializing regmap\n"); > > + if (IS_ERR(regmap)) { > > + dev_err_probe(&spi->dev, PTR_ERR(regmap), "Error initializing spi regmap: %ld\n", > > + PTR_ERR(regmap)); > > + return PTR_ERR(regmap); > > Why are you changing correct code into incorrect? I'll adjust that. It looks odd, I agree, but is this incorrect code? I found similar code in the neighbor adxl313 accel driver. I may change/update that then, too. Thank you for the hints. adxl313_i2c.c 72 if (IS_ERR(regmap)) { 73 dev_err(&client->dev, "Error initializing i2c regmap: %ld\n", 74 PTR_ERR(regmap)); 75 return PTR_ERR(regmap); 76 } > > + } > > > > - return adxl345_core_probe(&spi->dev, regmap); > > + return adxl345_core_probe(&spi->dev, regmap, chip_data, &adxl345_spi_setup); > > } > > > Best regards, > Krzysztof >
