Re: [PATCH v3] soc: qcom: rpmh-rsc: Enhance check for VRM in-flight request

[Maulik Shah (mkshah)]

Hi,

On 2/14/2024 11:38 AM, Bjorn Andersson wrote:

> >   /**
> > - * cmd_db_read_slave_id - Get the slave ID for a given resource address
> > + * cmd_db_match_resource_addr - Compare if both Resource addresses are same
>
> () after the function name, please.

Thanks for the review, Addressed in v4.

> > + *
> > + * @addr1: Resource address to compare
> > + * @addr2: Resource address to compare
> > + *
> > + * Return: true on matching addresses, false otherwise
>
> "Return: true if the two addresses refer to the same resource"

Addressed in v4.

> > + */
> > +bool cmd_db_match_resource_addr(u32 addr1, u32 addr2)
> > +{
> > +	/*
> > +	 * Each RPMh VRM accelerator resource has 3 or 4 contiguous 4-byte
> > +	 * aligned addresses associated with it. Ignore the offset to check
> > +	 * for VRM requests.
> > +	 */
> > +	if (SLAVE_ID(addr1) == CMD_DB_HW_VRM
> > +	    && VRM_ADDR(addr1) == VRM_ADDR(addr2))
>
> One line please, it's just 83 characters.

Addressed in v4.

> > +		return true;
> > +	else if (addr1 == addr2)
> > +		return true;
> > +	else
> > +		return false;
> > +}
> > +EXPORT_SYMBOL_GPL(cmd_db_match_resource_addr);
> > +
> > +/**
> > + * cmd_db_read_slave_id - Get the slave ID for a given resource name
> >    *
> > - * @id: Resource id to query the DB for version
> > + * @id: Resource id to query the DB for slave id
>
> Although trivial, it's unrelated to the newly introduced logic. Please
> submit a separate patch. Please also then add the () after the function
> name.
>
> Regards,
> Bjorn

Sure, i will send out separate patch for other comments to update.

Thanks,
Maulik