Re: [PATCH v2 09/11] remoteproc: pas: Extend parse_fw callback to parse resource table

[Stephan Gerhold <stephan.gerhold@linaro.org>]

On Wed, Aug 20, 2025 at 04:44:48PM +0530, Mukesh Ojha wrote:
> On Wed, Aug 20, 2025 at 10:36:10AM +0200, Stephan Gerhold wrote:
> > On Tue, Aug 19, 2025 at 10:24:44PM +0530, Mukesh Ojha wrote:
> [...]
> > > diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
> > > index ea7034c4b996..8456cca3f3e0 100644
> > > --- a/drivers/soc/qcom/mdt_loader.c
> > > +++ b/drivers/soc/qcom/mdt_loader.c
> > > @@ -22,7 +22,6 @@
> > >  #include <linux/slab.h>
> > >  #include <linux/soc/qcom/mdt_loader.h>
> > >  
> > > -#define MAX_RSCTABLE_SIZE	SZ_16K;
> > 
> > I'm confused why there is a semicolon here suddenly. Did you edit this
> > patch by hand?
> > 
> > Applying: remoteproc: pas: Extend parse_fw callback to parse resource table
> > Patch failed at 0009 remoteproc: pas: Extend parse_fw callback to parse resource table
> > error: patch failed: drivers/soc/qcom/mdt_loader.c:22
> > error: drivers/soc/qcom/mdt_loader.c: patch does not apply
> 
> Yes, I did this edit just before sending when checkpatch caught this.
> Will avoid this in future.
> 

Please don't edit patches manually, fix the commit instead. ;)

> > 
> > >  #define RSC_TABLE_HASH_BITS	     5  // 32 buckets
> > >  
> > >  DEFINE_HASHTABLE(qcom_pas_rsc_table_map, RSC_TABLE_HASH_BITS);
> > > diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
> > > index 62f239f64dfb..92ad862e733e 100644
> > > --- a/include/linux/soc/qcom/mdt_loader.h
> > > +++ b/include/linux/soc/qcom/mdt_loader.h
> > > @@ -8,6 +8,8 @@
> > >  #define QCOM_MDT_TYPE_HASH	(2 << 24)
> > >  #define QCOM_MDT_RELOCATABLE	BIT(27)
> > >  
> > > +#define MAX_RSCTABLE_SIZE	SZ_16K
> > > +
> > >  struct device;
> > >  struct firmware;
> > >  struct qcom_scm_pas_ctx;
> > 
> > You added this define yourself in PATCH 08/11, so just add it in the
> > right place directly. Make sure you scroll through your patch set before
> > sending to make sure all changes are in the right commit. :-)
> 
> I did this intentionally, because there is outside user of this macro 
> with this patch.
> 

I don't think putting it in the header without an immediate user is a
problem, as long as a user appears somewhere in the same series.

Right now this patch touches multiple subsystems at once (qcom soc and
remoteproc), which should be avoided when possible.

Thanks,
Stpehan