Re: [PATCH 1/2] rtc: rx6110: add i2c support

[Alexandre Belloni]

On 04/11/2020 11:26:28+0100, Claudius Heine wrote:
> +static int rx6110_i2c_probe(struct i2c_client *client,
> +			    const struct i2c_device_id *id)
> +{
> +	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> +	struct rx6110_data *rx6110;
> +	int err;
> +
> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA
> +				| I2C_FUNC_SMBUS_I2C_BLOCK)) {
> +		dev_err(&adapter->dev,
> +			"doesn't support required functionality\n");
> +		return -EIO;
> +	}
> +
> +	rx6110 = devm_kzalloc(&client->dev, sizeof(*rx6110), GFP_KERNEL);
> +	if (!rx6110)
> +		return -ENOMEM;
> +
> +	rx6110->regmap = devm_regmap_init_i2c(client, &regmap_i2c_config);
> +	if (IS_ERR(rx6110->regmap)) {
> +		dev_err(&client->dev, "regmap init failed for rtc rx6110\n");
> +		return PTR_ERR(rx6110->regmap);
> +	}
> +
> +	i2c_set_clientdata(client, rx6110);
> +
> +	rx6110->rtc = devm_rtc_device_register(&client->dev,
> +					       client->name,
> +					       &rx6110_rtc_ops, THIS_MODULE);
> +
> +	if (IS_ERR(rx6110->rtc))
> +		return PTR_ERR(rx6110->rtc);
> +
> +	err = rx6110_init(rx6110);
> +	if (err)
> +		return err;
> +
> +	rx6110->rtc->max_user_freq = 1;
> +

I would prefer if you could have a common function doing the RTC
registration and init for both i2c and spi. It wouldn't do much yet but
ideally, it would set the RTC range too and it would be better to not
have to duplicate that.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com