Re: [PATCH v2 1/6] gpio: pca953x: Convert PCA_TYPE from bits to number

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Wed, Oct 26, 2022 at 01:18:57PM +0200, Levente Révész wrote:
> There are 3 chip types already in the driver:
> 
>     * PCA953X_TYPE (standard register set, + pcal chips have extended set)

PCAL

>     * PCAL653X_TYPE (standard + extended register set, different spacing)
>     * PCA957X_TYPE (standard + other registers, different addresses)
> 
> A later patch in this series adds yet another type. This would take up all
> 4 bits allocated to chip type information in driver_data, leaving no space
> for new types in the future.
> 
> Convert PCA_TYPE defines from bits to numbers.
> 
> Replace if-else statements with cleaner switch statements. This makes it
> easier to add new types.

Suggested-by: ?

> Signed-off-by: Levente Révész <levente.revesz@xxxxxxxxxx>

...

> -#define PCA953X_TYPE		BIT(12)
> -#define PCA957X_TYPE		BIT(13)
> -#define PCAL653X_TYPE		BIT(14)
> +#define PCA953X_TYPE		(0x00 << 12)
> +#define PCAL653X_TYPE		(0x02 << 12)
> +#define PCA957X_TYPE		(0x03 << 12)
>  #define PCA_TYPE_MASK		GENMASK(15, 12)

Please, use decimal numbers and drop the shifts...

>  #define PCA_CHIP_TYPE(x)	((x) & PCA_TYPE_MASK)

...instead add a shift here.

...

I think you can consider adding default case to each of the new switch-case.

-- 
With Best Regards,
Andy Shevchenko