Re: [PATCH] spi: pxa2xx: Add SSC2 and SSPSP2 SSP registers

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 2020-08-25 9:38 PM, Andy Shevchenko wrote:
> On Tue, Aug 25, 2020 at 07:17:30PM +0200, Cezary Rojewski wrote:
>> Update list of SSP registers with SSC2 and SSPSP2. These registers are
>> utilized by LPT/WPT AudioDSP architecture.
>>
>> While SSC2 shares the same offset (0x40) as SSACDD, description of this
>> register for SSP device present on mentioned AudioDSP is different so
>> define separate constant to avoid any ambiguity.
> 
> In general it's okay, couple of nit-picks, though. After addressing,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 

Thanks for quick review, Andy.

>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
>> ---
>>   include/linux/pxa2xx_ssp.h | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/pxa2xx_ssp.h b/include/linux/pxa2xx_ssp.h
>> index 6facf27865f9..935d7db5bc32 100644
>> --- a/include/linux/pxa2xx_ssp.h
>> +++ b/include/linux/pxa2xx_ssp.h
>> @@ -41,6 +41,8 @@
>>   #define SSTSS		(0x38)  /* SSP Timeslot Status */
>>   #define SSACD		(0x3C)  /* SSP Audio Clock Divider */
>>   #define SSACDD		(0x40)	/* SSP Audio Clock Dither Divider */
>> +#define SSC2		(0x40)	/* SSP Command / Status 2 */
> 
> Is it the same as been called SSCR2 in Tangier?  (Also called SSP Command
> Status register 2) I guess we may name it with R applied.
> 

It seems there is discrepancy in registers naming. LPT/ WPT specs have 
all 'R'(s) removed. Don't mind appending it - aligns with existing SSCR0 
& SSCR1.

Know nothing about Tangier, though : )

>> +#define SSPSP2		(0x44)	/* SSP Programmable Serial Protocol 2 */
> 
> Also, please move them below with a comment like LPSS SSP block:
> https://elixir.bootlin.com/linux/latest/source/include/linux/pxa2xx_ssp.h#L181
> 

Added in v2 as requested.