Re: [PATCH RESEND v6 3/5] spmi: pmic-arb: Make the APID init a version operation

[Abel Vesa <abel.vesa@linaro.org>]

On 24-03-28 09:51:49, Neil Armstrong wrote:
> On 26/03/2024 17:28, Abel Vesa wrote:
> > Rather than using conditionals in probe function, add the APID init
> > as a version specific operation. Due to v7, which supports multiple
> > buses, pass on the bus index to be used for sorting out the apid base
> > and count.
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> >   drivers/spmi/spmi-pmic-arb.c | 199 +++++++++++++++++++++++++++----------------
> >   1 file changed, 124 insertions(+), 75 deletions(-)
> > 
> > diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> > index 9ed1180fe31f..38fed8a585fe 100644
> > --- a/drivers/spmi/spmi-pmic-arb.c
> > +++ b/drivers/spmi/spmi-pmic-arb.c
> > @@ -183,6 +183,7 @@ struct spmi_pmic_arb {
> >    * struct pmic_arb_ver_ops - version dependent functionality.
> >    *
> >    * @ver_str:		version string.
> > + * @init_apid:		finds the apid base and count
> >    * @ppid_to_apid:	finds the apid for a given ppid.
> >    * @non_data_cmd:	on v1 issues an spmi non-data command.
> >    *			on v2 no HW support, returns -EOPNOTSUPP.
> > @@ -202,6 +203,7 @@ struct spmi_pmic_arb {
> >    */
> >   struct pmic_arb_ver_ops {
> >   	const char *ver_str;
> > +	int (*init_apid)(struct spmi_pmic_arb *pmic_arb, int index);
> >   	int (*ppid_to_apid)(struct spmi_pmic_arb *pmic_arb, u16 ppid);
> >   	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
> >   	int (*offset)(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
> > @@ -942,6 +944,38 @@ static int qpnpint_irq_domain_alloc(struct irq_domain *domain,
> >   	return 0;
> >   }
> > +static int pmic_arb_init_apid_min_max(struct spmi_pmic_arb *pmic_arb)
> > +{
> > +	/*
> > +	 * Initialize max_apid/min_apid to the opposite bounds, during
> > +	 * the irq domain translation, we are sure to update these
> > +	 */
> > +	pmic_arb->max_apid = 0;
> > +	pmic_arb->min_apid = pmic_arb->max_periphs - 1;
> > +
> > +	return 0;
> > +}
> > +
> > +static int pmic_arb_init_apid_v1(struct spmi_pmic_arb *pmic_arb, int index)
> > +{
> > +	u32 *mapping_table;
> > +
> > +	if (index) {
> > +		dev_err(&pmic_arb->spmic->dev, "Unsupported buses count %d detected\n",
> > +			index);
> > +		return -EINVAL;
> > +	}
> > +
> > +	mapping_table = devm_kcalloc(&pmic_arb->spmic->dev, pmic_arb->max_periphs,
> > +				     sizeof(*mapping_table), GFP_KERNEL);
> > +	if (!mapping_table)
> > +		return -ENOMEM;
> > +
> > +	pmic_arb->mapping_table = mapping_table;
> 
> Can you specify in the spmi_pmic_arb->mapping_table struct documentation the mapping_table
> is only used in v1 ? or even better rename it to mapping_table_v1
> 

Actually the mapping_table is used on version 1 through 3.