Hi, On Mon, Jul 9, 2018 at 4:43 AM, Amit Kucheria <amit.kucheria@xxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html