[PATCH 2/3] soc: qcom: rpmh and cmd-db drivers

[Caleb Connolly]

Hi Sumit,

Sorry for the late response.

In general, I'd rather keep this port closer to the Linux original, so 
that future cherry-picking of new features and fixes is easier. This is 
quite standard practice in U-Boot (and why the compatibility macro's are 
included in the first place).

Otherwise, we end up with our own quite different implementation, with 
the higher maintenance burden that comes with it.

This port doesn't do a perfect job at keeping close to upstream, but 
still I don't think we should stray further without a good justification.

Kind regards,

>> +EXPORT_SYMBOL(cmd_db_read_addr);
> 
> Export symbols aren't required in U-Boot here and other instances in this patch.

...

>> +       /*
>> +        * Wait until we read back the same value.  Use a counter rather than
>> +        * ktime for timeout since this may be called after timekeeping stops.
>> +        */
>> +       for (i = 0; i < USEC_PER_SEC; i++) {
>> +               if (readl(addr) == data)
>> +                       return;
>> +               udelay(1);
>> +       }
> 
> Can we rather use readx_poll_sleep_timeout() here instead?

...
>> +       for (i = 0; i < 5 * USEC_PER_SEC; i++) {
>> +               if (readl(tcs_reg_addr(drv, reg, tcs_id)) & BIT(tcs_id))
>> +                       break;
>> +               udelay(1);
>> +       }
> 
> Can we rather use readx_poll_sleep_timeout() here instead?

...

>> +
>> +       spin_lock_irqsave(&drv->lock, flags);
> 
> Locks aren't needed in U-Boot, can be dropped here and other places.
> 

...

>> +
>> +       spin_lock_init(&drv->lock);
>> +       init_waitqueue_head(&drv->tcs_wait);
> 
> Similarly waitqueue should be dropped too.
> 
> -Sumit

-- 
// Caleb (they/them)