Messages in this thread | | | From | Amit Kucheria <> | Date | Thu, 13 Aug 2020 16:48:55 +0530 | Subject | Re: [RFC PATCH v5 1/7] drivers: thermal: tsens: Add VER_0 tsens version |
| |
On Tue, Aug 11, 2020 at 6:48 PM <ansuelsmth@gmail.com> wrote: > > > > > -----Messaggio originale----- > > Da: Amit Kucheria <amit.kucheria@linaro.org>
> > > > > if (IS_ERR(priv->rf[i])) > > > return PTR_ERR(priv->rf[i]); > > > } > > > @@ -775,12 +800,80 @@ int __init init_common(struct tsens_priv > > *priv) > > > goto err_put_device; > > > } > > > > > > - priv->rf[TSENS_EN] = devm_regmap_field_alloc(dev, priv->srot_map, > > > - priv->fields[TSENS_EN]); > > > - if (IS_ERR(priv->rf[TSENS_EN])) { > > > - ret = PTR_ERR(priv->rf[TSENS_EN]); > > > - goto err_put_device; > > > + if (tsens_version(priv) >= VER_0_1) { > > > + priv->rf[TSENS_EN] = devm_regmap_field_alloc( > > > + dev, priv->srot_map, priv->fields[TSENS_EN]); > > > + if (IS_ERR(priv->rf[TSENS_EN])) { > > > + ret = PTR_ERR(priv->rf[TSENS_EN]); > > > + goto err_put_device; > > > + } > > > + > > > + priv->rf[SENSOR_EN] = devm_regmap_field_alloc( > > > + dev, priv->srot_map, priv->fields[SENSOR_EN]); > > > + if (IS_ERR(priv->rf[SENSOR_EN])) { > > > + ret = PTR_ERR(priv->rf[SENSOR_EN]); > > > + goto err_put_device; > > > + } > > > + priv->rf[INT_EN] = devm_regmap_field_alloc( > > > + dev, priv->tm_map, priv->fields[INT_EN]); > > > + if (IS_ERR(priv->rf[INT_EN])) { > > > + ret = PTR_ERR(priv->rf[INT_EN]); > > > + goto err_put_device; > > > + } > > > + } else { > > > > Let's not create two big sections with if-else for 8960 and everything > > else. For example, what is wrong with using common code for TSENS_EN? > > > > If the concern is memory wasted trying to allocate fields not present > > on this older platform, perhaps consider adding a check in the loop to > > break early in case of 8960? > > > > About TSENS_EN the old platform doesn't have SROT so I need to use TM_MAP. > Should I set the srot map to match the tm map so we can use the common function? > Aside from this problem, I will try to remove the big if-else.
Ick. I guess srot_map and tm_map pointing to the same region is the lesser of two evils? It makes it so this will be constrained to a single place in init_common().
| |