All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: linux-sh@vger.kernel.org
Subject: Re: [PATCH 1/5] sh-pfc: r8a7778: add SRU/SSI pin support
Date: Thu, 22 Aug 2013 01:28:17 +0000	[thread overview]
Message-ID: <2734831.WKpsDbcXlG@avalon> (raw)
In-Reply-To: <8738qmjgav.wl%kuninori.morimoto.gx@renesas.com>

Hi Morimoto-san,

Thank you for the patch.

On Tuesday 06 August 2013 21:06:19 Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7778.c |  163 +++++++++++++++++++++++++++++++
>  1 file changed, 163 insertions(+)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7778.c
> b/drivers/pinctrl/sh-pfc/pfc-r8a7778.c index 428d2a6..a1bc846 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7778.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7778.c
> @@ -1304,6 +1304,22 @@ SH_PFC_MUX1(ether_link,		ETH_LINK);
>  SH_PFC_PINS(ether_magic,	RCAR_GP_PIN(4, 20));
>  SH_PFC_MUX1(ether_magic,	ETH_MAGIC);
> 
> +/* - AUDIO macro ------------------------------------------------------- */
> +#define AUDIO_PFC_PIN(name, pin)	SH_PFC_PINS(name, pin)
> +#define AUDIO_PFC_DAT(name, pin)	SH_PFC_MUX1(name, pin)
> +
> +/* - AUDIO clock --------------------------------------------------------*/
> +AUDIO_PFC_PIN(audio_clk_a,	RCAR_GP_PIN(2, 22));
> +AUDIO_PFC_DAT(audio_clk_a,	AUDIO_CLKA);
> +AUDIO_PFC_PIN(audio_clk_b,	RCAR_GP_PIN(2, 23));
> +AUDIO_PFC_DAT(audio_clk_b,	AUDIO_CLKB);
> +AUDIO_PFC_PIN(audio_clk_c,	RCAR_GP_PIN(2, 7));
> +AUDIO_PFC_DAT(audio_clk_c,	AUDIO_CLKC);
> +AUDIO_PFC_PIN(audio_clkout_a,	RCAR_GP_PIN(2, 16));
> +AUDIO_PFC_DAT(audio_clkout_a,	AUDIO_CLKOUT_A);
> +AUDIO_PFC_PIN(audio_clkout_b,	RCAR_GP_PIN(1, 16));
> +AUDIO_PFC_DAT(audio_clkout_b,	AUDIO_CLKOUT_B);
> +

Could you please move this block up to keep the blocks alphabetically sorted ?

>  /* - SCIF macro -------------------------------------------------------- */
>  #define SCIF_PFC_PIN(name, args...)	SH_PFC_PINS(name, args)
>  #define SCIF_PFC_DAT(name, tx, rx)	SH_PFC_MUX2(name, tx, rx)
> @@ -1577,6 +1593,105 @@ SDHI_PFC_WPPN(sdhi2_wp_a,	SD2_WP_A);
>  SDHI_PFC_PINS(sdhi2_wp_b,	RCAR_GP_PIN(3, 28));
>  SDHI_PFC_WPPN(sdhi2_wp_b,	SD2_WP_B);
> 
> +/* - SSI macro --------------------------------------------------------- */
> +#define SSI_PFC_PINS(name, args...)		SH_PFC_PINS(name, args)
> +#define SSI_PFC_SETS(name, sck, ws, d)		SH_PFC_MUX3(name, sck, ws, d)
> +#define SSI_PFC_MULT(name, sck, ws, d1, d2)	SH_PFC_MUX4(name, sck, ws,\
> +							    d1, d2)
> +/* - SSI0
> --------------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi0,	RCAR_GP_PIN(3, 6),	RCAR_GP_PIN(3, 7),
> +			RCAR_GP_PIN(3, 10));
> +SSI_PFC_SETS(ssi0,	SSI_SCK012,		SSI_WS012,
> +			SSI_SDATA0);
> +
> +/* - SSI01 --------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi01,	RCAR_GP_PIN(3, 6),	RCAR_GP_PIN(3, 7),
> +			RCAR_GP_PIN(3, 10),	RCAR_GP_PIN(3, 9));
> +SSI_PFC_MULT(ssi01,	SSI_SCK012,		SSI_WS012,
> +			SSI_SDATA0,		SSI_SDATA1);
> +
> +/* - SSI012 -------------------------------------------------------------*/
> +static const unsigned ssi012_pins[] = {
> +	RCAR_GP_PIN(3, 6),	RCAR_GP_PIN(3, 7),
> +	RCAR_GP_PIN(3, 10),	RCAR_GP_PIN(3, 9),
> +	RCAR_GP_PIN(3, 8),
> +};
> +
> +static const unsigned int ssi012_mux[] = {
> +	SSI_SCK012_MARK,	SSI_WS012_MARK,
> +	SSI_SDATA0_MARK,	SSI_SDATA1_MARK,
> +	SSI_SDATA2_MARK,
> +};
> +
> +/* - SSI02
> -------------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi02,	RCAR_GP_PIN(3, 6),	RCAR_GP_PIN(3, 7),
> +			RCAR_GP_PIN(3, 10),	RCAR_GP_PIN(3, 8));
> +SSI_PFC_MULT(ssi02,	SSI_SCK012,		SSI_WS012,
> +			SSI_SDATA0,		SSI_SDATA2);

This looks a bit complex to me. Could you please explain how the various pins 
are used ? I see that the SSI_DATA1 and SSI_DATA2 signals are used here and 
below. What are the possible combinations ? For instance, if I select ssi02, 
will SDATA0 and SDATA2 be driven by the SSI0 instance, or by the SSI0 and SSI2 
instances respectively ?

> +
> +/* - SSI1
> --------------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi1_a,	RCAR_GP_PIN(2, 20),	RCAR_GP_PIN(2, 21),
> +			RCAR_GP_PIN(3, 9));
> +SSI_PFC_SETS(ssi1_a,	SSI_SCK1_A,		SSI_WS1_A,
> +			SSI_SDATA1);
> +SSI_PFC_PINS(ssi1_b,	PIN_NUMBER(3, 20),	RCAR_GP_PIN(1, 3),
> +			RCAR_GP_PIN(3, 9));
> +SSI_PFC_SETS(ssi1_b,	SSI_SCK1_B,		SSI_WS1_B,
> +			SSI_SDATA1);
> +
> +/* - SSI2
> --------------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi2_a,	RCAR_GP_PIN(2, 26),	RCAR_GP_PIN(3, 4),
> +			RCAR_GP_PIN(3, 8));
> +SSI_PFC_SETS(ssi2_a,	SSI_SCK2_A,		SSI_WS2_A,
> +			SSI_SDATA2);
> +
> +SSI_PFC_PINS(ssi2_b,	RCAR_GP_PIN(2, 6),	RCAR_GP_PIN(2, 17),
> +			RCAR_GP_PIN(3, 8));
> +SSI_PFC_SETS(ssi2_b,	SSI_SCK2_B,		SSI_WS2_B,
> +			SSI_SDATA2);
> +
> +/* - SSI3
> --------------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi3,	RCAR_GP_PIN(3, 2),	RCAR_GP_PIN(3, 3),
> +			RCAR_GP_PIN(3, 5));
> +SSI_PFC_SETS(ssi3,	SSI_SCK34,		SSI_WS34,
> +			SSI_SDATA3);
> +
> +/* - SSI34
> -------------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi34,	RCAR_GP_PIN(3, 2),	RCAR_GP_PIN(3, 3),
> +			RCAR_GP_PIN(3, 5),	RCAR_GP_PIN(3, 4));
> +SSI_PFC_MULT(ssi34,	SSI_SCK34,		SSI_WS34,
> +			SSI_SDATA3,		SSI_SDATA4);

Can SSI_SDATA4 be used without SSI_SDATA3 ? Same question for SSI_DATA7 and 
SSI_DATA8 below.

> +/* - SSI4
> --------------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi4,	RCAR_GP_PIN(1, 22),	RCAR_GP_PIN(1, 23),
> +			RCAR_GP_PIN(3, 4));
> +SSI_PFC_SETS(ssi4,	SSI_SCK4,		SSI_WS4,
> +			SSI_SDATA4);
> +
> +/* - SSI5
> --------------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi5,	RCAR_GP_PIN(2, 31),	RCAR_GP_PIN(3, 0),
> +			RCAR_GP_PIN(3, 1));
> +SSI_PFC_SETS(ssi5,	SSI_SCK5,		SSI_WS5,
> +			SSI_SDATA5);
> +
> +/* - SSI6
> --------------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi6,	RCAR_GP_PIN(2, 28),	RCAR_GP_PIN(2, 29),
> +			RCAR_GP_PIN(2, 30));
> +SSI_PFC_SETS(ssi6,	SSI_SCK6,		SSI_WS6,
> +			SSI_SDATA6);
> +
> +/* - SSI7
> --------------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi7,	RCAR_GP_PIN(2, 24),	RCAR_GP_PIN(2, 25),
> +			RCAR_GP_PIN(2, 27));
> +SSI_PFC_SETS(ssi7,	SSI_SCK78,		SSI_WS78,
> +			SSI_SDATA7);
> +
> +/* - SSI78
> --------------------------------------------------------------------*/
> +SSI_PFC_PINS(ssi78,	RCAR_GP_PIN(2, 24),	RCAR_GP_PIN(2, 25),
> +			RCAR_GP_PIN(2, 27),	RCAR_GP_PIN(2, 26));
> +SSI_PFC_MULT(ssi78,	SSI_SCK78,		SSI_WS78,
> +			SSI_SDATA7,		SSI_SDATA8);
> +
>  /* - USB0
> ------------------------------------------------------------------- */
> SH_PFC_PINS(usb0,		RCAR_GP_PIN(0, 1));
>  SH_PFC_MUX1(usb0,		PENC0);
> @@ -1624,6 +1739,11 @@ VIN_PFC_PINS(vin1_sync,		RCAR_GP_PIN(3,
> 21),	RCAR_GP_PIN(3, 22)); VIN_PFC_SYNC(vin1_sync,		VI1_HSYNC,		
VI1_VSYNC);
> 
>  static const struct sh_pfc_pin_group pinmux_groups[] = {
> +	SH_PFC_PIN_GROUP(audio_clk_a),
> +	SH_PFC_PIN_GROUP(audio_clk_b),
> +	SH_PFC_PIN_GROUP(audio_clk_c),
> +	SH_PFC_PIN_GROUP(audio_clkout_a),
> +	SH_PFC_PIN_GROUP(audio_clkout_b),
>  	SH_PFC_PIN_GROUP(ether_rmii),
>  	SH_PFC_PIN_GROUP(ether_link),
>  	SH_PFC_PIN_GROUP(ether_magic),
> @@ -1713,6 +1833,21 @@ static const struct sh_pfc_pin_group pinmux_groups[]
> = { SH_PFC_PIN_GROUP(sdhi2_data4_b),
>  	SH_PFC_PIN_GROUP(sdhi2_wp_a),
>  	SH_PFC_PIN_GROUP(sdhi2_wp_b),
> +	SH_PFC_PIN_GROUP(ssi0),
> +	SH_PFC_PIN_GROUP(ssi01),
> +	SH_PFC_PIN_GROUP(ssi012),
> +	SH_PFC_PIN_GROUP(ssi02),
> +	SH_PFC_PIN_GROUP(ssi1_a),
> +	SH_PFC_PIN_GROUP(ssi1_b),
> +	SH_PFC_PIN_GROUP(ssi2_a),
> +	SH_PFC_PIN_GROUP(ssi2_b),
> +	SH_PFC_PIN_GROUP(ssi3),
> +	SH_PFC_PIN_GROUP(ssi34),
> +	SH_PFC_PIN_GROUP(ssi4),
> +	SH_PFC_PIN_GROUP(ssi5),
> +	SH_PFC_PIN_GROUP(ssi6),
> +	SH_PFC_PIN_GROUP(ssi7),
> +	SH_PFC_PIN_GROUP(ssi78),
>  	SH_PFC_PIN_GROUP(usb0),
>  	SH_PFC_PIN_GROUP(usb0_ovc),
>  	SH_PFC_PIN_GROUP(usb1),
> @@ -1725,6 +1860,14 @@ static const struct sh_pfc_pin_group pinmux_groups[]
> = { SH_PFC_PIN_GROUP(vin1_sync),
>  };
> 
> +static const char * const audio_clk_groups[] = {
> +	"audio_clk_a",
> +	"audio_clk_b",
> +	"audio_clk_c",
> +	"audio_clkout_a",
> +	"audio_clkout_b",
> +};
> +
>  static const char * const ether_groups[] = {
>  	"ether_rmii",
>  	"ether_link",
> @@ -1875,6 +2018,24 @@ static const char * const sdhi2_groups[] = {
>  	"sdhi2_wp_b",
>  };
> 
> +static const char * const ssi_groups[] = {
> +	"ssi0",
> +	"ssi01",
> +	"ssi012",
> +	"ssi02",
> +	"ssi1_a",
> +	"ssi1_b",
> +	"ssi2_a",
> +	"ssi2_b",
> +	"ssi3",
> +	"ssi34",
> +	"ssi4",
> +	"ssi5",
> +	"ssi6",
> +	"ssi7",
> +	"ssi78",
> +};

I'm not familiar with SSI so I could be wrong, but shouldn't we have one group 
per SSI instance ?

> +
>  static const char * const usb0_groups[] = {
>  	"usb0",
>  	"usb0_ovc",
> @@ -1898,6 +2059,7 @@ static const char * const vin1_groups[] = {
>  };
> 
>  static const struct sh_pfc_function pinmux_functions[] = {
> +	SH_PFC_FUNCTION(audio_clk),
>  	SH_PFC_FUNCTION(ether),
>  	SH_PFC_FUNCTION(hscif0),
>  	SH_PFC_FUNCTION(hscif1),
> @@ -1918,6 +2080,7 @@ static const struct sh_pfc_function pinmux_functions[]
> = { SH_PFC_FUNCTION(sdhi0),
>  	SH_PFC_FUNCTION(sdhi1),
>  	SH_PFC_FUNCTION(sdhi2),
> +	SH_PFC_FUNCTION(ssi),
>  	SH_PFC_FUNCTION(usb0),
>  	SH_PFC_FUNCTION(usb1),
>  	SH_PFC_FUNCTION(vin0),
-- 
Regards,

Laurent Pinchart


  parent reply	other threads:[~2013-08-22  1:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-07  4:06 [PATCH 1/5] sh-pfc: r8a7778: add SRU/SSI pin support Kuninori Morimoto
2013-08-21  8:47 ` Simon Horman
2013-08-21 20:50 ` Linus Walleij
2013-08-22  0:19 ` Simon Horman
2013-08-22  1:28 ` Laurent Pinchart [this message]
2013-08-22  4:46 ` Kuninori Morimoto
2013-08-22 11:02 ` Laurent Pinchart
2013-08-23  0:51 ` Kuninori Morimoto
2013-08-23 11:16 ` Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2734831.WKpsDbcXlG@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=linux-sh@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.