On 12/10/2021 19:16, Sam Protsenko wrote: > Old Exynos SoCs have both Product ID and Revision ID in one single > register, while new SoCs tend to have two separate registers for those > IDs. Implement handling of both cases by passing Revision ID register > offsets in driver data. > > Signed-off-by: Sam Protsenko <semen.protsenko@xxxxxxxxxx> > --- > drivers/soc/samsung/exynos-chipid.c | 67 +++++++++++++++++++---- > include/linux/soc/samsung/exynos-chipid.h | 6 +- > 2 files changed, 58 insertions(+), 15 deletions(-) > > diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c > index 5c1d0f97f766..1264a18aef97 100644 > --- a/drivers/soc/samsung/exynos-chipid.c > +++ b/drivers/soc/samsung/exynos-chipid.c > @@ -16,6 +16,7 @@ > #include <linux/errno.h> > #include <linux/mfd/syscon.h> > #include <linux/of.h> > +#include <linux/of_device.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > #include <linux/slab.h> > @@ -24,6 +25,17 @@ > > #include "exynos-asv.h" > > +struct exynos_chipid_variant { > + unsigned int rev_reg; /* revision register offset */ > + unsigned int main_rev_bit; /* main revision offset */ I understand it is offset of a bit within register, so how about: unsigned int main_rev_shift; /* main revision offset within rev_reg unsigned int sub_rev_shift; /* sub revision offset within rev_reg */ > + unsigned int sub_rev_bit; /* sub revision offset */ > +}; > + > +struct exynos_chipid_info { > + u32 product_id; > + u32 revision; > +}; > + > static const struct exynos_soc_id { > const char *name; > unsigned int id; > @@ -49,31 +61,55 @@ static const char *product_id_to_soc_id(unsigned int product_id) > int i; > > for (i = 0; i < ARRAY_SIZE(soc_ids); i++) > - if ((product_id & EXYNOS_MASK) == soc_ids[i].id) > + if (product_id == soc_ids[i].id) > return soc_ids[i].name; > return NULL; > } > > +static int exynos_chipid_get_chipid_info(struct regmap *regmap, > + const struct exynos_chipid_variant *data, > + struct exynos_chipid_info *soc_info) > +{ > + int ret; > + unsigned int val, main_rev, sub_rev; > + > + ret = regmap_read(regmap, EXYNOS_CHIPID_REG_PRO_ID, &val); > + if (ret < 0) > + return ret; > + soc_info->product_id = val & EXYNOS_MASK; > + > + ret = regmap_read(regmap, data->rev_reg, &val); > + if (ret < 0) > + return ret; > + main_rev = (val >> data->main_rev_bit) & EXYNOS_REV_PART_LEN; > + sub_rev = (val >> data->sub_rev_bit) & EXYNOS_REV_PART_LEN; > + soc_info->revision = (main_rev << EXYNOS_REV_PART_OFF) | sub_rev; > + > + return 0; > +} > + > static int exynos_chipid_probe(struct platform_device *pdev) > { > + const struct exynos_chipid_variant *drv_data; > + struct exynos_chipid_info soc_info; > struct soc_device_attribute *soc_dev_attr; > struct soc_device *soc_dev; > struct device_node *root; > struct regmap *regmap; > - u32 product_id; > - u32 revision; > int ret; > > + drv_data = of_device_get_match_data(&pdev->dev); > + if (!drv_data) > + return -EINVAL; > + > regmap = device_node_to_regmap(pdev->dev.of_node); > if (IS_ERR(regmap)) > return PTR_ERR(regmap); > > - ret = regmap_read(regmap, EXYNOS_CHIPID_REG_PRO_ID, &product_id); > + ret = exynos_chipid_get_chipid_info(regmap, drv_data, &soc_info); > if (ret < 0) > return ret; > > - revision = product_id & EXYNOS_REV_MASK; > - > soc_dev_attr = devm_kzalloc(&pdev->dev, sizeof(*soc_dev_attr), > GFP_KERNEL); > if (!soc_dev_attr) > @@ -86,8 +122,8 @@ static int exynos_chipid_probe(struct platform_device *pdev) > of_node_put(root); > > soc_dev_attr->revision = devm_kasprintf(&pdev->dev, GFP_KERNEL, > - "%x", revision); > - soc_dev_attr->soc_id = product_id_to_soc_id(product_id); > + "%x", soc_info.revision); > + soc_dev_attr->soc_id = product_id_to_soc_id(soc_info.product_id); > if (!soc_dev_attr->soc_id) { > pr_err("Unknown SoC\n"); > return -ENODEV; > @@ -106,7 +142,7 @@ static int exynos_chipid_probe(struct platform_device *pdev) > > dev_info(soc_device_to_device(soc_dev), > "Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n", > - soc_dev_attr->soc_id, product_id, revision); > + soc_dev_attr->soc_id, soc_info.product_id, soc_info.revision); > > return 0; > > @@ -125,9 +161,18 @@ static int exynos_chipid_remove(struct platform_device *pdev) > return 0; > } > > +static const struct exynos_chipid_variant exynos4210_chipid_drv_data = { > + .rev_reg = 0x0, > + .main_rev_bit = 0, > + .sub_rev_bit = 4, > +}; > + > static const struct of_device_id exynos_chipid_of_device_ids[] = { > - { .compatible = "samsung,exynos4210-chipid" }, > - {} > + { > + .compatible = "samsung,exynos4210-chipid", > + .data = &exynos4210_chipid_drv_data, > + }, > + { } > }; > > static struct platform_driver exynos_chipid_driver = { > diff --git a/include/linux/soc/samsung/exynos-chipid.h b/include/linux/soc/samsung/exynos-chipid.h > index 8bca6763f99c..5270725ba408 100644 > --- a/include/linux/soc/samsung/exynos-chipid.h > +++ b/include/linux/soc/samsung/exynos-chipid.h > @@ -9,10 +9,8 @@ > #define __LINUX_SOC_EXYNOS_CHIPID_H > > #define EXYNOS_CHIPID_REG_PRO_ID 0x00 > -#define EXYNOS_SUBREV_MASK (0xf << 4) > -#define EXYNOS_MAINREV_MASK (0xf << 0) > -#define EXYNOS_REV_MASK (EXYNOS_SUBREV_MASK | \ > - EXYNOS_MAINREV_MASK) > +#define EXYNOS_REV_PART_LEN 0xf EXYNOS_REV_PART_MASK > +#define EXYNOS_REV_PART_OFF 4 define EXYNOS_REV_PART_SHIFT > #define EXYNOS_MASK 0xfffff000 > > #define EXYNOS_CHIPID_REG_PKG_ID 0x04 > Best regards, Krzysztof