Messages in this thread | | | From | Doug Anderson <> | Date | Wed, 11 Jul 2018 11:37:07 -0700 | Subject | Re: [PATCH v6 2/7] thermal: tsens: Add support to split up register address space into two |
| |
Hi,
On Mon, Jul 9, 2018 at 4:43 AM, Amit Kucheria <amit.kucheria@linaro.org> wrote: > There are two banks of registers for v2 TSENS IPs: SROT and TM. On older > SoCs these were contiguous, leading to DTs mapping them as one register > address space of size 0x2000. In newer SoCs, these two banks are not > contiguous anymore. > > Fixing old DTs to split the address space into allows us to have cleaner > common code e.g. get_temp() that is shared across new and old platforms.
This makes it sound like old DTs won't be supported anymore. ...but the code says otherwise. I'd just remove the above paragraph.
> @@ -126,11 +127,21 @@ static const struct regmap_config tsens_config = { > int __init init_common(struct tsens_device *tmdev) > { > void __iomem *base; > + struct platform_device *op = of_find_device_by_node(tmdev->dev->of_node); > > + if (!op) > + return -EINVAL; > base = of_iomap(tmdev->dev->of_node, 0); > if (!base) > return -EINVAL; > > + if (op->num_resources > 1) {
Maybe add a comment here that says that we don't actually map the SROT yet because you don't read anything from there? I kept getting confused about how this patch could possibly work with no code to map SROT...
> + tmdev->tm_offset = 0; > + } else { > + /* old DTs where SROT and TM were in a contiguous 2K block */ > + tmdev->tm_offset = 0x1000;
This patch without patch #4 will break compatibility. You should squash part of patch #4 into this one, specifically:
-#define STATUS_OFFSET 0x10a0 -#define LAST_TEMP_MASK 0xfff +#define STATUS_OFFSET 0xa0 +#define LAST_TEMP_MASK 0xfff
Without that you break bisect-ability and also confuse anyone trying to look at this patch.
-Doug
| |