[RFC PATCH v1 4/4] NOT_FOR_MERGE: Add test code to emulate CPPC extension

[Sunil V L]

On Tue, Mar 21, 2023 at 10:28:09AM +0100, Andrew Jones wrote:
> On Tue, Mar 21, 2023 at 10:08:11AM +0100, Andrew Jones wrote:
> > Hi Sunil,
> > 
> > I know this isn't for merge, but I have comments which apply to
> > the previous patches here.
> > 
> > On Tue, Mar 21, 2023 at 09:50:54AM +0530, Sunil V L wrote:
> > > This is a test code to emulate the CPPC extension and it is
> > > not for merge.
> > > 
> > > Signed-off-by: Sunil V L <sunilvl at ventanamicro.com>
> > > ---
> > ...
> > > +static int sbi_cppc_test_probe(unsigned long reg)
> > > +{
> > > +	int ret = SBI_SUCCESS;
> > > +
> > > +	switch (reg) {
> > > +	case SBI_CPPC_DESIRED_PERF:
> > > +	case SBI_CPPC_PERF_LIMITED:
> > > +	case SBI_CPPC_HIGHEST_PERF:
> > > +	case SBI_CPPC_NOMINAL_PERF:
> > > +	case SBI_CPPC_LOW_NON_LINEAR_PERF:
> > > +	case SBI_CPPC_LOWEST_PERF:
> > > +	case SBI_CPPC_REFERENCE_PERF:
> > > +	case SBI_CPPC_LOWEST_FREQ:
> > > +	case SBI_CPPC_NOMINAL_FREQ:
> > > +	case SBI_CPPC_TRANSITION_LATENCY:
> > > +		ret = 32;
> > > +		break;
> > > +	case SBI_CPPC_REFERENCE_CTR:
> > > +	case SBI_CPPC_DELIVERED_CTR:
> > > +		ret = 64;
> > > +		break;
> > > +	case SBI_CPPC_GUARANTEED_PERF:
> > > +	case SBI_CPPC_MIN_PERF:
> > > +	case SBI_CPPC_MAX_PERF:
> > > +	case SBI_CPPC_PERF_REDUC_TOLERANCE:
> > > +	case SBI_CPPC_TIME_WINDOW:
> > > +	case SBI_CPPC_CTR_WRAP_TIME:
> > > +	case SBI_CPPC_ENABLE:
> > > +	case SBI_CPPC_AUTO_SEL_ENABLE:
> > > +	case SBI_CPPC_AUTO_ACT_WINDOW:
> > > +	case SBI_CPPC_ENERGY_PERF:
> > 
> > I think we should implement this switch in the main cppc code. Then,
> > here in the platform-specific code, all optional fields which are
> > not implemented will return zero, but everything else will use the
> > common implementation. I.e.
> > 
> >  int my_cppc_probe(unsigned long reg)
> >  {
> >     switch (ret) {
> >     case SBI_CPPC_GUARANTEED_PERF:
> >     case SBI_CPPC_MIN_PERF:
> >     case SBI_CPPC_MAX_PERF:
> >     case SBI_CPPC_PERF_REDUC_TOLERANCE:
> >     case SBI_CPPC_TIME_WINDOW:
> >     case SBI_CPPC_CTR_WRAP_TIME:
> >     case SBI_CPPC_ENABLE:
> >     case SBI_CPPC_AUTO_SEL_ENABLE:
> >     case SBI_CPPC_AUTO_ACT_WINDOW:
> >     case SBI_CPPC_ENERGY_PERF:
> >         return 0;
> >     }
> > 
> >     return cppc_probe(reg);
> 
> Thinking about this some more, it doesn't make sense to go
> common_probe -> specific_probe -> common_probe_helper. The common_probe
> should do what the helper is for when the specific_probe doesn't do
> anything. I.e.
> 
>  int specific_probe(unsigned long reg)
>  {
>     switch (reg) {
>     /* specific impl */
>     }
> 
>     /* Not handled here, handle in common code */
>     return -1;
>  }
> 
>  int common_probe(unsigned long reg)
>  {
>      int ret = specific_probe(reg);
> 
>      if (ret >= 0)
>         return ret;
> 
>      switch (reg) {
>      /* common impl */
>      }
>  }
> 
Thank you very much!, Drew. I have sent RFC v2 as per your suggestions.
Please review.

Thanks,
Sunil