Re: [PATCH 1/2] iio: adxl345: add spi-3wire

[Krzysztof Kozlowski <krzk@kernel.org>]

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@gmail.com>
> ---



>  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()?

> +
>  	/* 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?

> +	}
>  
> -	return adxl345_core_probe(&spi->dev, regmap);
> +	return adxl345_core_probe(&spi->dev, regmap, chip_data, &adxl345_spi_setup);
>  }


Best regards,
Krzysztof