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

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxx>

> Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
> ---
>  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