On 13/10/2021 22:21, 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..7837331fb753 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_shift; /* main revision offset in rev_reg */
> + unsigned int sub_rev_shift; /* sub revision offset in rev_reg */
> +};
> +
> +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);
Isn't this the same register as EXYNOS_CHIPID_REG_PRO_ID?
> + if (ret < 0)
> + return ret;
> + main_rev = (val >> data->main_rev_shift) & EXYNOS_REV_PART_MASK;
> + sub_rev = (val >> data->sub_rev_shift) & EXYNOS_REV_PART_MASK;
> + soc_info->revision = (main_rev << EXYNOS_REV_PART_SHIFT) | 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_shift = 0,
> + .sub_rev_shift = 4,
The code does not look correct here. Subrev is at 0:3 bits, mainrev is
at 4:7.
Did you test it that it produces same result? Looks not - I gave it a
try and got wrong revision.
Best regards,
Krzysztof
