Hi,
On Sat, Feb 03, 2018 at 11:49:37PM +0800, Icenowy Zheng wrote:
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> index 11b128f54ed2..fae732c8c548 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> @@ -110,7 +110,7 @@ struct sunxi_pinctrl_desc {
> int npins;
> unsigned pin_base;
> unsigned irq_banks;
> - unsigned irq_bank_base;
> + const unsigned int *irq_bank_map;
> bool irq_read_needs_mux;
> bool disable_strict_mode;
> };
> @@ -263,12 +263,21 @@ static inline u32 sunxi_pull_offset(u16 pin)
> return pin_num * PULL_PINS_BITS;
> }
>
> -static inline u32 sunxi_irq_cfg_reg(u16 irq, unsigned bank_base)
> +static inline u32 sunxi_irq_hw_bank_num(u8 bank, const unsigned int
> *bank_map)
> +{
> + if (!bank_map)
> + return bank;
> + else
> + return bank_map[bank];
> +}From a function API PoV, I guess it would make more sense and be cleaner to simply pass the sunxi_pinctrl_desc structure (as the first argument) and have the function there read that structure and act upon it. Giving it just a blank pointer, without any size indication, and either dereferencing it without any boundary check, or changing the behaviour based on whether it's null or not seems pretty fragile. I guess you could do it in two patches. One to change the function prototypes to call the sunxi_irq_cfg_reg function (and all its caller) to have the prototype (struct sunxi_pinctrl_desc *desc, u16 irq). And then in a later patch change the behaviour to introduce the map. Thanks! Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com
signature.asc
Description: PGP signature
