On Tue, Dec 03, 2013 at 08:57:28PM +0100, Stefan Agner wrote:
[...]
> Changes since v2:
> - Removed reg_ from reg_version
> - Moved walk through version dependent tables to find_regulator_info,
> removed the inline definition. This reduces .o size and encapsulates
> the logic of finding the right regulator into one function. It comes
> with a slight code duplication, the table search now appears twice.
Well, the table was searched twice in the original patch, too, so this
variant isn't any worse, really.
> ---
> arch/arm/boot/dts/tegra20-colibri-512.dtsi | 4 +-
> drivers/regulator/tps6586x-regulator.c | 91 +++++++++++++++++++++++-------
> 2 files changed, 74 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
[...]
> -static inline struct tps6586x_regulator *find_regulator_info(int id)
> +static struct tps6586x_regulator *find_regulator_info(int id, int version)
> {
> struct tps6586x_regulator *ri;
> + struct tps6586x_regulator *table = NULL;
> + int num;
> int i;
>
> + switch (version) {
> + case TPS658623:
> + table = tps658623_regulator;
> + num = ARRAY_SIZE(tps658623_regulator);
> + break;
> + case TPS658643:
> + table = tps658643_regulator;
> + num = ARRAY_SIZE(tps658643_regulator);
> + break;
> + }
> +
I think you still need to check for table to be valid here. Depending on
the version it might still be NULL.
> + /* Search version specific table first */
> + for (i = 0; i < num; i++) {
> + ri = &table[i];
> + if (ri->desc.id == id)
> + return ri;
> + }
So this would need to be wrapped in an "if (table) { }" block.
Attachment:
pgpGqF0PHdOxA.pgp
Description: PGP signature
