From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 83E6BC4332F for ; Mon, 19 Dec 2022 15:16:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232985AbiLSPQo (ORCPT ); Mon, 19 Dec 2022 10:16:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43310 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232849AbiLSPQQ (ORCPT ); Mon, 19 Dec 2022 10:16:16 -0500 Received: from mail.hugovil.com (mail.hugovil.com [162.243.120.170]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5EBBB38C; Mon, 19 Dec 2022 07:15:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=hugovil.com ; s=x; h=Subject:Content-Transfer-Encoding:Content-Type:Mime-Version: References:In-Reply-To:Message-Id:Cc:To:From:Date:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=w4sSFhh83uAji8+0eAowtyfKSzm6lsgYgeM/KPj3SJw=; b=SQh7IKfej+/jWDV02bPW0rvUTL +/T5pdlopHUCfUuGQMtAWwyYVIrwVk3abapQ+Nd2DmCUaf8MeKFSa7vikK01xS+0wIl+3zTzW/P3g fwEvcz+u1+avs9g5H9A37wuSZiM8BeRgcXmYcotSxGzT1bc5b0SscN7l0evXhV/GKdOk=; Received: from modemcable168.174-80-70.mc.videotron.ca ([70.80.174.168]:48604 helo=pettiford) by mail.hugovil.com with esmtpa (Exim 4.92) (envelope-from ) id 1p7HrT-0000v1-07; Mon, 19 Dec 2022 10:15:29 -0500 Date: Mon, 19 Dec 2022 10:15:26 -0500 From: Hugo Villeneuve To: Bruno Thomsen Cc: a.zummo@towertech.it, alexandre.belloni@bootlin.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, linux-rtc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Hugo Villeneuve , hugo@hugovil.com Message-Id: <20221219101526.ab27daa0971e827128d51a15@hugovil.com> In-Reply-To: References: <20221215150214.1109074-1-hugo@hugovil.com> <20221215150214.1109074-2-hugo@hugovil.com> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 70.80.174.168 X-SA-Exim-Mail-From: hugo@hugovil.com Subject: Re: [PATCH v3 01/14] rtc: pcf2127: add variant-specific configuration structure X-SA-Exim-Version: 4.2.1 (built Wed, 08 May 2019 21:11:16 +0000) X-SA-Exim-Scanned: Yes (on mail.hugovil.com) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 19 Dec 2022 10:05:53 +0100 Bruno Thomsen wrote: > Den tor. 15. dec. 2022 kl. 16.19 skrev Hugo Villeneuve : > > > > From: Hugo Villeneuve > > > > Create variant-specific configuration structures to simplify the > > implementation of new variants into this driver. It will also avoid > > to have too many tests for a specific variant, or a list of variants > > for new devices, inside the code itself. > > > > Add configuration options for the support of the NVMEM, bit CD0 in > > register WD_CTL as well as the maximum number of registers for each > > variant, instead of hardcoding the variant (PCF2127) inside the > > i2c_device_id and spi_device_id structures. > > > > Also specify a different maximum number of registers (max_register) > > for the PCF2129. > > > > Signed-off-by: Hugo Villeneuve > > --- > > drivers/rtc/rtc-pcf2127.c | 95 +++++++++++++++++++++++++++++++-------- > > 1 file changed, 76 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > > index 87f4fc9df68b..b9a5d47a439f 100644 > > --- a/drivers/rtc/rtc-pcf2127.c > > +++ b/drivers/rtc/rtc-pcf2127.c > > @@ -21,6 +21,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > > > @@ -101,10 +102,17 @@ > > PCF2127_BIT_CTRL2_WDTF | \ > > PCF2127_BIT_CTRL2_TSF2) > > > > +struct pcf21xx_config { > > + int max_register; > > + unsigned int has_nvmem:1; > > + unsigned int has_bit_wd_ctl_cd0:1; > > +}; > > + > > struct pcf2127 { > > struct rtc_device *rtc; > > struct watchdog_device wdd; > > struct regmap *regmap; > > + const struct pcf21xx_config *cfg; > > time64_t ts; > > bool ts_valid; > > bool irq_enabled; > > @@ -631,8 +639,27 @@ static const struct attribute_group pcf2127_attr_group = { > > .attrs = pcf2127_attrs, > > }; > > > > +enum pcf21xx_type { > > + PCF2127, > > + PCF2129, > > + PCF21XX_LAST_ID > > +}; > > + > > +static struct pcf21xx_config pcf21xx_cfg[] = { > > + [PCF2127] = { > > + .max_register = 0x1d, > > + .has_nvmem = 1, > > + .has_bit_wd_ctl_cd0 = 1, > > + }, > > + [PCF2129] = { > > + .max_register = 0x19, > > + .has_nvmem = 0, > > + .has_bit_wd_ctl_cd0 = 0, > > + }, > > +}; > > + > > static int pcf2127_probe(struct device *dev, struct regmap *regmap, > > - int alarm_irq, const char *name, bool is_pcf2127) > > + int alarm_irq, const char *name, const struct pcf21xx_config *config) > > { > > struct pcf2127 *pcf2127; > > int ret = 0; > > @@ -645,6 +672,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, > > return -ENOMEM; > > > > pcf2127->regmap = regmap; > > + pcf2127->cfg = config; > > > > dev_set_drvdata(dev, pcf2127); > > > > @@ -688,7 +716,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, > > set_bit(RTC_FEATURE_ALARM, pcf2127->rtc->features); > > } > > > > - if (is_pcf2127) { > > + if (pcf2127->cfg->has_nvmem) { > > struct nvmem_config nvmem_cfg = { > > .priv = pcf2127, > > .reg_read = pcf2127_nvmem_read, > > @@ -734,7 +762,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, > > PCF2127_BIT_WD_CTL_TF1 | > > PCF2127_BIT_WD_CTL_TF0, > > PCF2127_BIT_WD_CTL_CD1 | > > - (is_pcf2127 ? PCF2127_BIT_WD_CTL_CD0 : 0) | > > + (pcf2127->cfg->has_bit_wd_ctl_cd0 ? PCF2127_BIT_WD_CTL_CD0 : 0) | > > PCF2127_BIT_WD_CTL_TF1); > > if (ret) { > > dev_err(dev, "%s: watchdog config (wd_ctl) failed\n", __func__); > > @@ -799,9 +827,9 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap, > > > > #ifdef CONFIG_OF > > static const struct of_device_id pcf2127_of_match[] = { > > - { .compatible = "nxp,pcf2127" }, > > - { .compatible = "nxp,pcf2129" }, > > - { .compatible = "nxp,pca2129" }, > > + { .compatible = "nxp,pcf2127", .data = &pcf21xx_cfg[PCF2127] }, > > + { .compatible = "nxp,pcf2129", .data = &pcf21xx_cfg[PCF2129] }, > > + { .compatible = "nxp,pca2129", .data = &pcf21xx_cfg[PCF2129] }, > > {} > > }; > > MODULE_DEVICE_TABLE(of, pcf2127_of_match); > > @@ -886,26 +914,40 @@ static const struct regmap_bus pcf2127_i2c_regmap = { > > static struct i2c_driver pcf2127_i2c_driver; > > > > static const struct i2c_device_id pcf2127_i2c_id[] = { > > - { "pcf2127", 1 }, > > - { "pcf2129", 0 }, > > - { "pca2129", 0 }, > > + { "pcf2127", PCF2127 }, > > + { "pcf2129", PCF2129 }, > > + { "pca2129", PCF2129 }, > > { } > > }; > > MODULE_DEVICE_TABLE(i2c, pcf2127_i2c_id); > > > > static int pcf2127_i2c_probe(struct i2c_client *client) > > { > > - const struct i2c_device_id *id = i2c_match_id(pcf2127_i2c_id, client); > > struct regmap *regmap; > > - static const struct regmap_config config = { > > + static struct regmap_config config = { > > .reg_bits = 8, > > .val_bits = 8, > > - .max_register = 0x1d, > > }; > > + const struct pcf21xx_config *variant; > > Hi Hugo, > > Patch series does not apply on 6.1 tree as pcf2127_i2c_probe() call > signature does not match[1]. > > static int pcf2127_i2c_probe(struct i2c_client *client, > const struct i2c_device_id *id) > > > [1] https://elixir.bootlin.com/linux/v6.1/source/drivers/rtc/rtc-pcf2127.c#L888 > > /Bruno Hi Bruno, I based my driver on the git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git repo, as indicated in the MAINTAINERS file for the RTC subsystem (T: entry). I used the rtc-next branch on this repo. Can you tell me exactly which repo and branch I need to use to resubmit the driver? Thank you, Hugo. > > if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) > > return -ENODEV; > > > > + if (client->dev.of_node) { > > + variant = of_device_get_match_data(&client->dev); > > + if (!variant) > > + return -ENODEV; > > + } else { > > + enum pcf21xx_type type = > > + i2c_match_id(pcf2127_i2c_id, client)->driver_data; > > + > > + if (type >= PCF21XX_LAST_ID) > > + return -ENODEV; > > + variant = &pcf21xx_cfg[type]; > > + } > > + > > + config.max_register = variant->max_register, > > + > > regmap = devm_regmap_init(&client->dev, &pcf2127_i2c_regmap, > > &client->dev, &config); > > if (IS_ERR(regmap)) { > > @@ -915,7 +957,7 @@ static int pcf2127_i2c_probe(struct i2c_client *client) > > } > > > > return pcf2127_probe(&client->dev, regmap, client->irq, > > - pcf2127_i2c_driver.driver.name, id->driver_data); > > + pcf2127_i2c_driver.driver.name, variant); > > } > > > > static struct i2c_driver pcf2127_i2c_driver = { > > @@ -953,17 +995,32 @@ static void pcf2127_i2c_unregister_driver(void) > > #if IS_ENABLED(CONFIG_SPI_MASTER) > > > > static struct spi_driver pcf2127_spi_driver; > > +static const struct spi_device_id pcf2127_spi_id[]; > > > > static int pcf2127_spi_probe(struct spi_device *spi) > > { > > - static const struct regmap_config config = { > > + static struct regmap_config config = { > > .reg_bits = 8, > > .val_bits = 8, > > .read_flag_mask = 0xa0, > > .write_flag_mask = 0x20, > > - .max_register = 0x1d, > > }; > > struct regmap *regmap; > > + const struct pcf21xx_config *variant; > > + > > + if (spi->dev.of_node) { > > + variant = of_device_get_match_data(&spi->dev); > > + if (!variant) > > + return -ENODEV; > > + } else { > > + enum pcf21xx_type type = spi_get_device_id(spi)->driver_data; > > + > > + if (type >= PCF21XX_LAST_ID) > > + return -ENODEV; > > + variant = &pcf21xx_cfg[type]; > > + } > > + > > + config.max_register = variant->max_register, > > > > regmap = devm_regmap_init_spi(spi, &config); > > if (IS_ERR(regmap)) { > > @@ -974,13 +1031,13 @@ static int pcf2127_spi_probe(struct spi_device *spi) > > > > return pcf2127_probe(&spi->dev, regmap, spi->irq, > > pcf2127_spi_driver.driver.name, > > - spi_get_device_id(spi)->driver_data); > > + variant); > > } > > > > static const struct spi_device_id pcf2127_spi_id[] = { > > - { "pcf2127", 1 }, > > - { "pcf2129", 0 }, > > - { "pca2129", 0 }, > > + { "pcf2127", PCF2127 }, > > + { "pcf2129", PCF2129 }, > > + { "pca2129", PCF2129 }, > > { } > > }; > > MODULE_DEVICE_TABLE(spi, pcf2127_spi_id); > > -- > > 2.30.2 > > > -- Hugo Villeneuve