Messages in this thread | | | Date | Fri, 22 Feb 2013 15:52:36 +0000 | From | Ian Lartey <> | Subject | Re: [PATCH 2/2] mfd: palmas add variant and OTP detection |
| |
On 22/02/13 10:30, Laxman Dewangan wrote: > On Friday 22 February 2013 06:22 AM, Ian Lartey wrote: >> From: Graeme Gregory <gg@slimlogic.co.uk> >> >> Read the chip varient and the OTP information from the chip and display >> this on probe to aid in debugging of issues. >> > >> + /* Read varient info from the device */ >> + slave = PALMAS_BASE_TO_SLAVE(PALMAS_ID_BASE); >> + addr = PALMAS_BASE_TO_REG(PALMAS_ID_BASE, PALMAS_PRODUCT_ID_LSB); >> + ret = regmap_read(palmas->regmap[slave], addr, ®); >> + if (ret < 0) { >> + dev_err(palmas->dev, "Unable to read ID err: %d\n", ret); >> + goto err; >> + } >> + > > Can you please use the api palmas_* for reading register which I added > recently. This will reduce the calc of slave and addr and done in single > call? > Same of following code also.
Will do. > > > Also can we move all these new code to one function where we read id and > diaplay. To keep probe() smaller.
Yes, I'll do that too. > > >> + palmas->id = reg; >> + >> + slave = PALMAS_BASE_TO_SLAVE(PALMAS_ID_BASE); >> + addr = PALMAS_BASE_TO_REG(PALMAS_ID_BASE, PALMAS_PRODUCT_ID_MSB); >> + ret = regmap_read(palmas->regmap[slave], addr, ®); >> + if (ret < 0) { >> + dev_err(palmas->dev, "Unable to read ID err: %d\n", ret); >> + goto err; >> + } >> + >> + palmas->id |= reg << 8; > > Is it possible to change variable name id to product_id and then update > to have more meaningful name? > We can add for vendor_id also if require.
The rename of id to product_id is fine, if this what you mean by "update to have a more meaningful name" then I agree. Though if you wish for a additional change, I am not sure what the additional change is.
> >> + >> + dev_info(palmas->dev, "Product ID %x\n", palmas->id); >> + >> + slave = PALMAS_BASE_TO_SLAVE(PALMAS_DESIGNREV_BASE); >> + addr = PALMAS_BASE_TO_REG(PALMAS_DESIGNREV_BASE, PALMAS_DESIGNREV); >> + ret = regmap_read(palmas->regmap[slave], addr, ®); >> + if (ret < 0) { >> + dev_err(palmas->dev, "Unable to read DESIGNREV err: %d\n", ret); >> + goto err; >> + } >> + >> + palmas->designrev = reg & PALMAS_DESIGNREV_DESIGNREV_MASK; >> + >> + dev_info(palmas->dev, "Product Design Rev %x\n", palmas->designrev); >> + >> + slave = PALMAS_BASE_TO_SLAVE(PALMAS_PMU_CONTROL_BASE); >> + addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, >> PALMAS_SW_REVISION); >> + ret = regmap_read(palmas->regmap[slave], addr, ®); >> + if (ret < 0) { >> + dev_err(palmas->dev, "Unable to read SW_REVISION err: %d\n", >> + ret); >> + goto err; >> + } >> + >> + palmas->sw_revision = reg; >> + >> + dev_info(palmas->dev, "Product SW Rev %x\n", palmas->sw_revision); >> + > > > Here designrev also says the ES version and sw revision says the OTP SW > revision. > TI has make the design revision as ES1.0, ES2.0, ES2.1, ES2.2 like this > and other technical names are 0xA0, 0xB0, 0xB1, 0xB2 etc. > > Probably I will add this in follow on patch becasue we have some errata > which we need to implement based on the version number.
OK, I'll not make this change in favour of your later patch. > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
| |