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

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

[Cezary Rojewski]

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.

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 */
+#define SSPSP2		(0x44)	/* SSP Programmable Serial Protocol 2 */
 
 /* Common PXA2xx bits first */
 #define SSCR0_DSS	(0x0000000f)	/* Data Size Select (mask) */
-- 
2.17.1

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

[Andy Shevchenko]

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>

> 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.

> +#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

>  
>  /* Common PXA2xx bits first */
>  #define SSCR0_DSS	(0x0000000f)	/* Data Size Select (mask) */
> -- 
> 2.17.1
> 

-- 
With Best Regards,
Andy Shevchenko

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

[Cezary Rojewski]

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.