Re: [PATCH v5 06/17] soc: qcom: Add Qualcomm APSS minidump kernel driver

[Mukesh Ojha]

On 9/12/2023 12:39 AM, Jeff Johnson wrote:
> On 9/11/2023 4:07 AM, Krzysztof Kozlowski wrote:
> > On 09/09/2023 22:16, Mukesh Ojha wrote:
> > > +/**
> > > + * qcom_minidump_region_register() - Register region in APSS 
> > > Minidump table.
> > > + * @region: minidump region.
> > > + *
> > > + * Return: On success, it returns 0 and negative error value on 
> > > failure.
> > > + */
> > > +int qcom_minidump_region_register(const struct qcom_minidump_region 
> > > *region)
> > > +{
> > > +    struct minidump *md;
> > > +    int ret;
> > > +
> > > +    md = qcom_smem_minidump_ready();
> > > +    if (!md)
> > > +        return -EPROBE_DEFER;
> > > +
> > > +    if (!qcom_minidump_valid_region(region))
> > > +        return -EINVAL;
> > > +
> > > +    mutex_lock(&md->md_lock);
> > > +    ret = qcom_md_region_register(md, region);
> > > +    if (ret)
> > > +        goto unlock;
> > > +
> > > +    qcom_md_update_elfheader(md, region);
> > > +unlock:
> > > +    mutex_unlock(&md->md_lock);
> > > +    return ret;
> > > +}
> > > +EXPORT_SYMBOL_GPL(qcom_minidump_region_register);
> >
> > NAK, there is no user for this.
> >
> > Drop all exports from minidump drivers. Your upstream driver *must not*
> > expose stuff to your vendor drivers.
>
> Do we not expect that upstream drivers would want to register?

As of current version of patch, there is no user. Let's avoid till
any upstream QCOM driver uses it .


> Mind you, in the downstream code the following was a bad limitation:
> #define MAX_NUM_OF_SS           10

I don't think, there is any problem with above macro, instead there is
restriction on total number of APSS region can be registered.

#define MAX_NUM_ENTRIES	  201

-Mukesh
>