diff mbox

pinctrl: sh-pfc: pfc-r8a7790: Add full VIN1/2/3 support

Message ID 1382040403-19649-1-git-send-email-valentine.barshak@cogentembedded.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Valentine Barshak Oct. 17, 2013, 8:06 p.m. UTC
This adds missing VIN1/2/3 pin configuration to pfc-r8a7790.

Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
---
 drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 202 ++++++++++++++++++++++++++++++++++-
 1 file changed, 198 insertions(+), 4 deletions(-)

Comments

Simon Horman Oct. 29, 2013, 5:03 a.m. UTC | #1
On Fri, Oct 18, 2013 at 12:06:43AM +0400, Valentine Barshak wrote:
> This adds missing VIN1/2/3 pin configuration to pfc-r8a7790.
> 
> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>

Hi Laurent,

could you consider taking this change?

> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 202 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 198 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> index 64fcc006..9ab2f71 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -2996,22 +2996,168 @@ static const unsigned int vin0_clk_mux[] = {
>  	VI0_CLK_MARK,
>  };
>  /* - VIN1 ------------------------------------------------------------------- */
> -static const unsigned int vin1_data_pins[] = {
> +static const unsigned int vin1_data_g_pins[] = {
> +	RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 17),
> +	RCAR_GP_PIN(1, 20), RCAR_GP_PIN(1, 22), RCAR_GP_PIN(1, 12),
> +	RCAR_GP_PIN(1, 9), RCAR_GP_PIN(1, 7),
> +};
> +static const unsigned int vin1_data_g_mux[] = {
> +	VI1_G0_MARK, VI1_G1_MARK, VI1_G2_MARK,
> +	VI1_G3_MARK, VI1_G4_MARK, VI1_G5_MARK,
> +	VI1_G6_MARK, VI1_G7_MARK,
> +};
> +static const unsigned int vin1_data_r_pins[] = {
> +	RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28), RCAR_GP_PIN(0, 29),
> +	RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5), RCAR_GP_PIN(1, 6),
> +	RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 8),
> +};
> +static const unsigned int vin1_data_r_mux[] = {
> +	VI1_R0_MARK, VI1_R1_MARK, VI1_R2_MARK,
> +	VI1_R3_MARK, VI1_R4_MARK, VI1_R5_MARK,
> +	VI1_R6_MARK, VI1_R7_MARK,
> +};
> +static const unsigned int vin1_data_b_pins[] = {
>  	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11), RCAR_GP_PIN(2, 12),
>  	RCAR_GP_PIN(2, 13), RCAR_GP_PIN(2, 14), RCAR_GP_PIN(2, 15),
>  	RCAR_GP_PIN(2, 16), RCAR_GP_PIN(2, 17),
>  };
> -static const unsigned int vin1_data_mux[] = {
> +static const unsigned int vin1_data_b_mux[] = {
>  	VI1_DATA0_VI1_B0_MARK, VI1_DATA1_VI1_B1_MARK, VI1_DATA2_VI1_B2_MARK,
>  	VI1_DATA3_VI1_B3_MARK, VI1_DATA4_VI1_B4_MARK, VI1_DATA5_VI1_B5_MARK,
>  	VI1_DATA6_VI1_B6_MARK, VI1_DATA7_VI1_B7_MARK,
>  };
> +static const unsigned int vin1_hsync_signal_pins[] = {
> +	RCAR_GP_PIN(1, 24),
> +};
> +static const unsigned int vin1_hsync_signal_mux[] = {
> +	VI1_HSYNC_N_MARK,
> +};
> +static const unsigned int vin1_vsync_signal_pins[] = {
> +	RCAR_GP_PIN(1, 25),
> +};
> +static const unsigned int vin1_vsync_signal_mux[] = {
> +	VI1_VSYNC_N_MARK,
> +};
> +static const unsigned int vin1_field_signal_pins[] = {
> +	RCAR_GP_PIN(1, 13),
> +};
> +static const unsigned int vin1_field_signal_mux[] = {
> +	VI1_FIELD_MARK,
> +};
> +static const unsigned int vin1_data_enable_pins[] = {
> +	RCAR_GP_PIN(1, 26),
> +};
> +static const unsigned int vin1_data_enable_mux[] = {
> +	VI1_CLKENB_MARK,
> +};
>  static const unsigned int vin1_clk_pins[] = {
>  	RCAR_GP_PIN(2, 9),
>  };
>  static const unsigned int vin1_clk_mux[] = {
>  	VI1_CLK_MARK,
>  };
> +/* - VIN2 ----------------------------------------------------------------- */
> +static const unsigned int vin2_data_g_pins[] = {
> +	RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28), RCAR_GP_PIN(0, 29),
> +	RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5),
> +	RCAR_GP_PIN(1, 6), RCAR_GP_PIN(1, 7),
> +};
> +static const unsigned int vin2_data_g_mux[] = {
> +	VI2_G0_MARK, VI2_G1_MARK, VI2_G2_MARK,
> +	VI2_G3_MARK, VI2_G4_MARK, VI2_G5_MARK,
> +	VI2_G6_MARK, VI2_G7_MARK,
> +};
> +static const unsigned int vin2_data_r_pins[] = {
> +	RCAR_GP_PIN(1, 12), RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
> +	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 20),
> +	RCAR_GP_PIN(1, 22), RCAR_GP_PIN(1, 24),
> +};
> +static const unsigned int vin2_data_r_mux[] = {
> +	VI2_R0_MARK, VI2_R1_MARK, VI2_R2_MARK,
> +	VI2_R3_MARK, VI2_R4_MARK, VI2_R5_MARK,
> +	VI2_R6_MARK, VI2_R7_MARK,
> +};
> +static const unsigned int vin2_data_b_pins[] = {
> +	RCAR_GP_PIN(0, 8), RCAR_GP_PIN(0, 9), RCAR_GP_PIN(0, 10),
> +	RCAR_GP_PIN(0, 11), RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
> +	RCAR_GP_PIN(0, 14), RCAR_GP_PIN(0, 15),
> +};
> +static const unsigned int vin2_data_b_mux[] = {
> +	VI2_DATA0_VI2_B0_MARK, VI2_DATA1_VI2_B1_MARK, VI2_DATA2_VI2_B2_MARK,
> +	VI2_DATA3_VI2_B3_MARK, VI2_DATA4_VI2_B4_MARK, VI2_DATA5_VI2_B5_MARK,
> +	VI2_DATA6_VI2_B6_MARK, VI2_DATA7_VI2_B7_MARK,
> +};
> +static const unsigned int vin2_hsync_signal_pins[] = {
> +	RCAR_GP_PIN(1, 16),
> +};
> +static const unsigned int vin2_hsync_signal_mux[] = {
> +	VI2_HSYNC_N_MARK,
> +};
> +static const unsigned int vin2_vsync_signal_pins[] = {
> +	RCAR_GP_PIN(1, 21),
> +};
> +static const unsigned int vin2_vsync_signal_mux[] = {
> +	VI2_VSYNC_N_MARK,
> +};
> +static const unsigned int vin2_field_signal_pins[] = {
> +	RCAR_GP_PIN(1, 9),
> +};
> +static const unsigned int vin2_field_signal_mux[] = {
> +	VI2_FIELD_MARK,
> +};
> +static const unsigned int vin2_data_enable_pins[] = {
> +	RCAR_GP_PIN(1, 8),
> +};
> +static const unsigned int vin2_data_enable_mux[] = {
> +	VI2_CLKENB_MARK,
> +};
> +static const unsigned int vin2_clk_pins[] = {
> +	RCAR_GP_PIN(1, 11),
> +};
> +static const unsigned int vin2_clk_mux[] = {
> +	VI2_CLK_MARK,
> +};
> +/* - VIN3 ----------------------------------------------------------------- */
> +static const unsigned int vin3_data_pins[] = {
> +	RCAR_GP_PIN(0, 0), RCAR_GP_PIN(0, 1), RCAR_GP_PIN(0, 2),
> +	RCAR_GP_PIN(0, 3), RCAR_GP_PIN(0, 4), RCAR_GP_PIN(0, 5),
> +	RCAR_GP_PIN(0, 6), RCAR_GP_PIN(0, 7),
> +};
> +static const unsigned int vin3_data_mux[] = {
> +	VI3_DATA0_MARK, VI3_DATA1_MARK, VI3_DATA2_MARK,
> +	VI3_DATA3_MARK, VI3_DATA4_MARK, VI3_DATA5_MARK,
> +	VI3_DATA6_MARK, VI3_DATA7_MARK,
> +};
> +static const unsigned int vin3_hsync_signal_pins[] = {
> +	RCAR_GP_PIN(1, 16),
> +};
> +static const unsigned int vin3_hsync_signal_mux[] = {
> +	VI3_HSYNC_N_MARK,
> +};
> +static const unsigned int vin3_vsync_signal_pins[] = {
> +	RCAR_GP_PIN(1, 17),
> +};
> +static const unsigned int vin3_vsync_signal_mux[] = {
> +	VI2_VSYNC_N_MARK,
> +};
> +static const unsigned int vin3_field_signal_pins[] = {
> +	RCAR_GP_PIN(1, 15),
> +};
> +static const unsigned int vin3_field_signal_mux[] = {
> +	VI3_FIELD_MARK,
> +};
> +static const unsigned int vin3_data_enable_pins[] = {
> +	RCAR_GP_PIN(1, 14),
> +};
> +static const unsigned int vin3_data_enable_mux[] = {
> +	VI3_CLKENB_MARK,
> +};
> +static const unsigned int vin3_clk_pins[] = {
> +	RCAR_GP_PIN(1, 23),
> +};
> +static const unsigned int vin3_clk_mux[] = {
> +	VI3_CLK_MARK,
> +};
>  
>  static const struct sh_pfc_pin_group pinmux_groups[] = {
>  	SH_PFC_PIN_GROUP(du_rgb666),
> @@ -3185,8 +3331,28 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
>  	SH_PFC_PIN_GROUP(vin0_field_signal),
>  	SH_PFC_PIN_GROUP(vin0_data_enable),
>  	SH_PFC_PIN_GROUP(vin0_clk),
> -	SH_PFC_PIN_GROUP(vin1_data),
> +	SH_PFC_PIN_GROUP(vin1_data_g),
> +	SH_PFC_PIN_GROUP(vin1_data_r),
> +	SH_PFC_PIN_GROUP(vin1_data_b),
> +	SH_PFC_PIN_GROUP(vin1_hsync_signal),
> +	SH_PFC_PIN_GROUP(vin1_vsync_signal),
> +	SH_PFC_PIN_GROUP(vin1_field_signal),
> +	SH_PFC_PIN_GROUP(vin1_data_enable),
>  	SH_PFC_PIN_GROUP(vin1_clk),
> +	SH_PFC_PIN_GROUP(vin2_data_g),
> +	SH_PFC_PIN_GROUP(vin2_data_r),
> +	SH_PFC_PIN_GROUP(vin2_data_b),
> +	SH_PFC_PIN_GROUP(vin2_hsync_signal),
> +	SH_PFC_PIN_GROUP(vin2_vsync_signal),
> +	SH_PFC_PIN_GROUP(vin2_field_signal),
> +	SH_PFC_PIN_GROUP(vin2_data_enable),
> +	SH_PFC_PIN_GROUP(vin2_clk),
> +	SH_PFC_PIN_GROUP(vin3_data),
> +	SH_PFC_PIN_GROUP(vin3_hsync_signal),
> +	SH_PFC_PIN_GROUP(vin3_vsync_signal),
> +	SH_PFC_PIN_GROUP(vin3_field_signal),
> +	SH_PFC_PIN_GROUP(vin3_data_enable),
> +	SH_PFC_PIN_GROUP(vin3_clk),
>  };
>  
>  static const char * const du_groups[] = {
> @@ -3457,10 +3623,36 @@ static const char * const vin0_groups[] = {
>  };
>  
>  static const char * const vin1_groups[] = {
> -	"vin1_data",
> +	"vin1_data_g",
> +	"vin1_data_r",
> +	"vin1_data_b",
> +	"vin1_hsync_signal",
> +	"vin1_vsync_signal",
> +	"vin1_field_signal",
> +	"vin1_data_enable",
>  	"vin1_clk",
>  };
>  
> +static const char * const vin2_groups[] = {
> +	"vin2_data_g",
> +	"vin2_data_r",
> +	"vin2_data_b",
> +	"vin2_hsync_signal",
> +	"vin2_vsync_signal",
> +	"vin2_field_signal",
> +	"vin2_data_enable",
> +	"vin2_clk",
> +};
> +
> +static const char * const vin3_groups[] = {
> +	"vin3_data",
> +	"vin3_hsync_signal",
> +	"vin3_vsync_signal",
> +	"vin3_field_signal",
> +	"vin3_data_enable",
> +	"vin3_clk",
> +};
> +
>  static const struct sh_pfc_function pinmux_functions[] = {
>  	SH_PFC_FUNCTION(du),
>  	SH_PFC_FUNCTION(du0),
> @@ -3495,6 +3687,8 @@ static const struct sh_pfc_function pinmux_functions[] = {
>  	SH_PFC_FUNCTION(usb2),
>  	SH_PFC_FUNCTION(vin0),
>  	SH_PFC_FUNCTION(vin1),
> +	SH_PFC_FUNCTION(vin2),
> +	SH_PFC_FUNCTION(vin3),
>  };
>  
>  static struct pinmux_cfg_reg pinmux_config_regs[] = {
> -- 
> 1.8.3.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Pinchart Oct. 29, 2013, 6:10 p.m. UTC | #2
Hi Valentine,

Thanks you for the patch.

On Friday 18 October 2013 00:06:43 Valentine Barshak wrote:
> This adds missing VIN1/2/3 pin configuration to pfc-r8a7790.
> 
> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 202 +++++++++++++++++++++++++++++++-
>  1 file changed, 198 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c index 64fcc006..9ab2f71 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -2996,22 +2996,168 @@ static const unsigned int vin0_clk_mux[] = {
>  	VI0_CLK_MARK,
>  };
>  /* - VIN1 -------------------------------------------------------------- */
> -static const unsigned int vin1_data_pins[] = {
> +static const unsigned int vin1_data_g_pins[] = {
> +	RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 17),
> +	RCAR_GP_PIN(1, 20), RCAR_GP_PIN(1, 22), RCAR_GP_PIN(1, 12),
> +	RCAR_GP_PIN(1, 9), RCAR_GP_PIN(1, 7),
> +};
> +static const unsigned int vin1_data_g_mux[] = {
> +	VI1_G0_MARK, VI1_G1_MARK, VI1_G2_MARK,
> +	VI1_G3_MARK, VI1_G4_MARK, VI1_G5_MARK,
> +	VI1_G6_MARK, VI1_G7_MARK,
> +};
> +static const unsigned int vin1_data_r_pins[] = {
> +	RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28), RCAR_GP_PIN(0, 29),
> +	RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5), RCAR_GP_PIN(1, 6),
> +	RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 8),
> +};
> +static const unsigned int vin1_data_r_mux[] = {
> +	VI1_R0_MARK, VI1_R1_MARK, VI1_R2_MARK,
> +	VI1_R3_MARK, VI1_R4_MARK, VI1_R5_MARK,
> +	VI1_R6_MARK, VI1_R7_MARK,
> +};
> +static const unsigned int vin1_data_b_pins[] = {
>  	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11), RCAR_GP_PIN(2, 12),
>  	RCAR_GP_PIN(2, 13), RCAR_GP_PIN(2, 14), RCAR_GP_PIN(2, 15),
>  	RCAR_GP_PIN(2, 16), RCAR_GP_PIN(2, 17),
>  };
> -static const unsigned int vin1_data_mux[] = {
> +static const unsigned int vin1_data_b_mux[] = {
>  	VI1_DATA0_VI1_B0_MARK, VI1_DATA1_VI1_B1_MARK, VI1_DATA2_VI1_B2_MARK,
>  	VI1_DATA3_VI1_B3_MARK, VI1_DATA4_VI1_B4_MARK, VI1_DATA5_VI1_B5_MARK,
>  	VI1_DATA6_VI1_B6_MARK, VI1_DATA7_VI1_B7_MARK,
>  };

Given the wide variety of ways the VIN data pins can be used, I see two ways 
to group them:

- List all possible combinations of data pins with a single group per 
configuration. There are 11 combinations, but due to overlap in the pins used 
the number can be reduced to 8.

- List groups individually and combine them to create a valid configuration. 
We would have at least 8 groups in this case as well.

Given that the number of groups would be identical, my preference would go for 
the first solution. The RGB data pin group would contain all 24 RGB pins in 
that case, and additional overlapping groups would be defined for other input 
configurations. Would you be fine with that ? VIN1 would be handled 
identically, and VIN2 would need 5 groups only. VIN3 is already defined 
correctly, as it supports a single configuration only.

> +static const unsigned int vin1_hsync_signal_pins[] = {

I think you can drop _signal here and below.

> +	RCAR_GP_PIN(1, 24),
> +};
> +static const unsigned int vin1_hsync_signal_mux[] = {
> +	VI1_HSYNC_N_MARK,
> +};
> +static const unsigned int vin1_vsync_signal_pins[] = {
> +	RCAR_GP_PIN(1, 25),
> +};
> +static const unsigned int vin1_vsync_signal_mux[] = {
> +	VI1_VSYNC_N_MARK,
> +};

If I'm not mistaken the hsync and vsync signals can't be used independently, 
so they should be grouped together.

> +static const unsigned int vin1_field_signal_pins[] = {
> +	RCAR_GP_PIN(1, 13),
> +};
> +static const unsigned int vin1_field_signal_mux[] = {
> +	VI1_FIELD_MARK,
> +};
> +static const unsigned int vin1_data_enable_pins[] = {
> +	RCAR_GP_PIN(1, 26),
> +};
> +static const unsigned int vin1_data_enable_mux[] = {
> +	VI1_CLKENB_MARK,
> +};
>  static const unsigned int vin1_clk_pins[] = {
>  	RCAR_GP_PIN(2, 9),
>  };
>  static const unsigned int vin1_clk_mux[] = {
>  	VI1_CLK_MARK,
>  };
> +/* - VIN2 -----------------------------------------------------------------
> */ +static const unsigned int vin2_data_g_pins[] = {
> +	RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28), RCAR_GP_PIN(0, 29),
> +	RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5),
> +	RCAR_GP_PIN(1, 6), RCAR_GP_PIN(1, 7),
> +};
> +static const unsigned int vin2_data_g_mux[] = {
> +	VI2_G0_MARK, VI2_G1_MARK, VI2_G2_MARK,
> +	VI2_G3_MARK, VI2_G4_MARK, VI2_G5_MARK,
> +	VI2_G6_MARK, VI2_G7_MARK,
> +};
> +static const unsigned int vin2_data_r_pins[] = {
> +	RCAR_GP_PIN(1, 12), RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
> +	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 20),
> +	RCAR_GP_PIN(1, 22), RCAR_GP_PIN(1, 24),
> +};
> +static const unsigned int vin2_data_r_mux[] = {
> +	VI2_R0_MARK, VI2_R1_MARK, VI2_R2_MARK,
> +	VI2_R3_MARK, VI2_R4_MARK, VI2_R5_MARK,
> +	VI2_R6_MARK, VI2_R7_MARK,
> +};
> +static const unsigned int vin2_data_b_pins[] = {
> +	RCAR_GP_PIN(0, 8), RCAR_GP_PIN(0, 9), RCAR_GP_PIN(0, 10),
> +	RCAR_GP_PIN(0, 11), RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
> +	RCAR_GP_PIN(0, 14), RCAR_GP_PIN(0, 15),
> +};
> +static const unsigned int vin2_data_b_mux[] = {
> +	VI2_DATA0_VI2_B0_MARK, VI2_DATA1_VI2_B1_MARK, VI2_DATA2_VI2_B2_MARK,
> +	VI2_DATA3_VI2_B3_MARK, VI2_DATA4_VI2_B4_MARK, VI2_DATA5_VI2_B5_MARK,
> +	VI2_DATA6_VI2_B6_MARK, VI2_DATA7_VI2_B7_MARK,
> +};
> +static const unsigned int vin2_hsync_signal_pins[] = {
> +	RCAR_GP_PIN(1, 16),
> +};
> +static const unsigned int vin2_hsync_signal_mux[] = {
> +	VI2_HSYNC_N_MARK,
> +};
> +static const unsigned int vin2_vsync_signal_pins[] = {
> +	RCAR_GP_PIN(1, 21),
> +};
> +static const unsigned int vin2_vsync_signal_mux[] = {
> +	VI2_VSYNC_N_MARK,
> +};
> +static const unsigned int vin2_field_signal_pins[] = {
> +	RCAR_GP_PIN(1, 9),
> +};
> +static const unsigned int vin2_field_signal_mux[] = {
> +	VI2_FIELD_MARK,
> +};
> +static const unsigned int vin2_data_enable_pins[] = {
> +	RCAR_GP_PIN(1, 8),
> +};
> +static const unsigned int vin2_data_enable_mux[] = {
> +	VI2_CLKENB_MARK,
> +};
> +static const unsigned int vin2_clk_pins[] = {
> +	RCAR_GP_PIN(1, 11),
> +};
> +static const unsigned int vin2_clk_mux[] = {
> +	VI2_CLK_MARK,
> +};
> +/* - VIN3 -----------------------------------------------------------------
> */ +static const unsigned int vin3_data_pins[] = {
> +	RCAR_GP_PIN(0, 0), RCAR_GP_PIN(0, 1), RCAR_GP_PIN(0, 2),
> +	RCAR_GP_PIN(0, 3), RCAR_GP_PIN(0, 4), RCAR_GP_PIN(0, 5),
> +	RCAR_GP_PIN(0, 6), RCAR_GP_PIN(0, 7),
> +};
> +static const unsigned int vin3_data_mux[] = {
> +	VI3_DATA0_MARK, VI3_DATA1_MARK, VI3_DATA2_MARK,
> +	VI3_DATA3_MARK, VI3_DATA4_MARK, VI3_DATA5_MARK,
> +	VI3_DATA6_MARK, VI3_DATA7_MARK,
> +};
> +static const unsigned int vin3_hsync_signal_pins[] = {
> +	RCAR_GP_PIN(1, 16),
> +};
> +static const unsigned int vin3_hsync_signal_mux[] = {
> +	VI3_HSYNC_N_MARK,
> +};
> +static const unsigned int vin3_vsync_signal_pins[] = {
> +	RCAR_GP_PIN(1, 17),
> +};
> +static const unsigned int vin3_vsync_signal_mux[] = {
> +	VI2_VSYNC_N_MARK,
> +};
> +static const unsigned int vin3_field_signal_pins[] = {
> +	RCAR_GP_PIN(1, 15),
> +};
> +static const unsigned int vin3_field_signal_mux[] = {
> +	VI3_FIELD_MARK,
> +};
> +static const unsigned int vin3_data_enable_pins[] = {
> +	RCAR_GP_PIN(1, 14),
> +};
> +static const unsigned int vin3_data_enable_mux[] = {
> +	VI3_CLKENB_MARK,
> +};
> +static const unsigned int vin3_clk_pins[] = {
> +	RCAR_GP_PIN(1, 23),
> +};
> +static const unsigned int vin3_clk_mux[] = {
> +	VI3_CLK_MARK,
> +};
> 
>  static const struct sh_pfc_pin_group pinmux_groups[] = {
>  	SH_PFC_PIN_GROUP(du_rgb666),
> @@ -3185,8 +3331,28 @@ static const struct sh_pfc_pin_group pinmux_groups[]
> = { SH_PFC_PIN_GROUP(vin0_field_signal),
>  	SH_PFC_PIN_GROUP(vin0_data_enable),
>  	SH_PFC_PIN_GROUP(vin0_clk),
> -	SH_PFC_PIN_GROUP(vin1_data),
> +	SH_PFC_PIN_GROUP(vin1_data_g),
> +	SH_PFC_PIN_GROUP(vin1_data_r),
> +	SH_PFC_PIN_GROUP(vin1_data_b),
> +	SH_PFC_PIN_GROUP(vin1_hsync_signal),
> +	SH_PFC_PIN_GROUP(vin1_vsync_signal),
> +	SH_PFC_PIN_GROUP(vin1_field_signal),
> +	SH_PFC_PIN_GROUP(vin1_data_enable),
>  	SH_PFC_PIN_GROUP(vin1_clk),
> +	SH_PFC_PIN_GROUP(vin2_data_g),
> +	SH_PFC_PIN_GROUP(vin2_data_r),
> +	SH_PFC_PIN_GROUP(vin2_data_b),
> +	SH_PFC_PIN_GROUP(vin2_hsync_signal),
> +	SH_PFC_PIN_GROUP(vin2_vsync_signal),
> +	SH_PFC_PIN_GROUP(vin2_field_signal),
> +	SH_PFC_PIN_GROUP(vin2_data_enable),
> +	SH_PFC_PIN_GROUP(vin2_clk),
> +	SH_PFC_PIN_GROUP(vin3_data),
> +	SH_PFC_PIN_GROUP(vin3_hsync_signal),
> +	SH_PFC_PIN_GROUP(vin3_vsync_signal),
> +	SH_PFC_PIN_GROUP(vin3_field_signal),
> +	SH_PFC_PIN_GROUP(vin3_data_enable),
> +	SH_PFC_PIN_GROUP(vin3_clk),
>  };
> 
>  static const char * const du_groups[] = {
> @@ -3457,10 +3623,36 @@ static const char * const vin0_groups[] = {
>  };
> 
>  static const char * const vin1_groups[] = {
> -	"vin1_data",
> +	"vin1_data_g",
> +	"vin1_data_r",
> +	"vin1_data_b",
> +	"vin1_hsync_signal",
> +	"vin1_vsync_signal",
> +	"vin1_field_signal",
> +	"vin1_data_enable",
>  	"vin1_clk",
>  };
> 
> +static const char * const vin2_groups[] = {
> +	"vin2_data_g",
> +	"vin2_data_r",
> +	"vin2_data_b",
> +	"vin2_hsync_signal",
> +	"vin2_vsync_signal",
> +	"vin2_field_signal",
> +	"vin2_data_enable",
> +	"vin2_clk",
> +};
> +
> +static const char * const vin3_groups[] = {
> +	"vin3_data",
> +	"vin3_hsync_signal",
> +	"vin3_vsync_signal",
> +	"vin3_field_signal",
> +	"vin3_data_enable",
> +	"vin3_clk",
> +};
> +
>  static const struct sh_pfc_function pinmux_functions[] = {
>  	SH_PFC_FUNCTION(du),
>  	SH_PFC_FUNCTION(du0),
> @@ -3495,6 +3687,8 @@ static const struct sh_pfc_function pinmux_functions[]
> = { SH_PFC_FUNCTION(usb2),
>  	SH_PFC_FUNCTION(vin0),
>  	SH_PFC_FUNCTION(vin1),
> +	SH_PFC_FUNCTION(vin2),
> +	SH_PFC_FUNCTION(vin3),
>  };
> 
>  static struct pinmux_cfg_reg pinmux_config_regs[] = {
Valentine Barshak Dec. 9, 2013, 7:14 p.m. UTC | #3
On 10/29/2013 10:10 PM, Laurent Pinchart wrote:
> Hi Valentine,

Hi Laurent,
sorry for delay.

>
> Thanks you for the patch.
>
> On Friday 18 October 2013 00:06:43 Valentine Barshak wrote:
>> This adds missing VIN1/2/3 pin configuration to pfc-r8a7790.
>>
>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>> ---
>>   drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 202 +++++++++++++++++++++++++++++++-
>>   1 file changed, 198 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
>> b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c index 64fcc006..9ab2f71 100644
>> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
>> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
>> @@ -2996,22 +2996,168 @@ static const unsigned int vin0_clk_mux[] = {
>>   	VI0_CLK_MARK,
>>   };
>>   /* - VIN1 -------------------------------------------------------------- */
>> -static const unsigned int vin1_data_pins[] = {
>> +static const unsigned int vin1_data_g_pins[] = {
>> +	RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 17),
>> +	RCAR_GP_PIN(1, 20), RCAR_GP_PIN(1, 22), RCAR_GP_PIN(1, 12),
>> +	RCAR_GP_PIN(1, 9), RCAR_GP_PIN(1, 7),
>> +};
>> +static const unsigned int vin1_data_g_mux[] = {
>> +	VI1_G0_MARK, VI1_G1_MARK, VI1_G2_MARK,
>> +	VI1_G3_MARK, VI1_G4_MARK, VI1_G5_MARK,
>> +	VI1_G6_MARK, VI1_G7_MARK,
>> +};
>> +static const unsigned int vin1_data_r_pins[] = {
>> +	RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28), RCAR_GP_PIN(0, 29),
>> +	RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5), RCAR_GP_PIN(1, 6),
>> +	RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 8),
>> +};
>> +static const unsigned int vin1_data_r_mux[] = {
>> +	VI1_R0_MARK, VI1_R1_MARK, VI1_R2_MARK,
>> +	VI1_R3_MARK, VI1_R4_MARK, VI1_R5_MARK,
>> +	VI1_R6_MARK, VI1_R7_MARK,
>> +};
>> +static const unsigned int vin1_data_b_pins[] = {
>>   	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11), RCAR_GP_PIN(2, 12),
>>   	RCAR_GP_PIN(2, 13), RCAR_GP_PIN(2, 14), RCAR_GP_PIN(2, 15),
>>   	RCAR_GP_PIN(2, 16), RCAR_GP_PIN(2, 17),
>>   };
>> -static const unsigned int vin1_data_mux[] = {
>> +static const unsigned int vin1_data_b_mux[] = {
>>   	VI1_DATA0_VI1_B0_MARK, VI1_DATA1_VI1_B1_MARK, VI1_DATA2_VI1_B2_MARK,
>>   	VI1_DATA3_VI1_B3_MARK, VI1_DATA4_VI1_B4_MARK, VI1_DATA5_VI1_B5_MARK,
>>   	VI1_DATA6_VI1_B6_MARK, VI1_DATA7_VI1_B7_MARK,
>>   };
>
> Given the wide variety of ways the VIN data pins can be used, I see two ways
> to group them:
>
> - List all possible combinations of data pins with a single group per
> configuration. There are 11 combinations, but due to overlap in the pins used
> the number can be reduced to 8.
>
> - List groups individually and combine them to create a valid configuration.
> We would have at least 8 groups in this case as well.
>
> Given that the number of groups would be identical, my preference would go for
> the first solution. The RGB data pin group would contain all 24 RGB pins in
> that case, and additional overlapping groups would be defined for other input
> configurations. Would you be fine with that ? VIN1 would be handled
> identically, and VIN2 would need 5 groups only. VIN3 is already defined
> correctly, as it supports a single configuration only.

I'm only adding missing stuff here. VIN0/VIN1 are identical so I'm adding VIN1
the same way VIN0 is currently grouped.
I'm generally fine with re-grouping data pins, but I think it should be a separate change.
We'd also need to choose simple names for the groups which are easily readable and show
which pins are used there. Something like:
data_r07g07b07 (24 bits for RGB24)
data_r27g27b27 (pins 2-7 for ITU-R BT.601/BT.709 RGB-666)

>
>> +static const unsigned int vin1_hsync_signal_pins[] = {
>
> I think you can drop _signal here and below.

I didn't intend to edit the existing naming. All I wanted to do is add the definitions
for the missing pins here. I think name editing should be done by a separate patch if needed.
IMHO, otherwise it would be a mess.

>
>> +	RCAR_GP_PIN(1, 24),
>> +};
>> +static const unsigned int vin1_hsync_signal_mux[] = {
>> +	VI1_HSYNC_N_MARK,
>> +};
>> +static const unsigned int vin1_vsync_signal_pins[] = {
>> +	RCAR_GP_PIN(1, 25),
>> +};
>> +static const unsigned int vin1_vsync_signal_mux[] = {
>> +	VI1_VSYNC_N_MARK,
>> +};
>
> If I'm not mistaken the hsync and vsync signals can't be used independently,
> so they should be grouped together.

I'm just using the current VIN0 layout for VIN1 here.
I'm not intending to regroup anything here. The purpose of these
changes is to add missing pins. I believe regrouping should be done
by a separate patch if needed.

[snip]

Thanks,
Val.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Pinchart Dec. 10, 2013, 1:45 a.m. UTC | #4
Hi Valentine,

On Monday 09 December 2013 23:14:29 Valentine wrote:
> On 10/29/2013 10:10 PM, Laurent Pinchart wrote:
> > On Friday 18 October 2013 00:06:43 Valentine Barshak wrote:
> >> This adds missing VIN1/2/3 pin configuration to pfc-r8a7790.
> >> 
> >> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> >> ---
> >> 
> >>   drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 202 +++++++++++++++++++++++++++-
> >>   1 file changed, 198 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> >> b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c index 64fcc006..9ab2f71 100644
> >> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> >> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> >> @@ -2996,22 +2996,168 @@ static const unsigned int vin0_clk_mux[] = {
> >>   	VI0_CLK_MARK,
> >>   };
> >>   /* - VIN1 ---------------------------------------------------------- */
> >> -static const unsigned int vin1_data_pins[] = {
> >> +static const unsigned int vin1_data_g_pins[] = {
> >> +	RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 17),
> >> +	RCAR_GP_PIN(1, 20), RCAR_GP_PIN(1, 22), RCAR_GP_PIN(1, 12),
> >> +	RCAR_GP_PIN(1, 9), RCAR_GP_PIN(1, 7),
> >> +};
> >> +static const unsigned int vin1_data_g_mux[] = {
> >> +	VI1_G0_MARK, VI1_G1_MARK, VI1_G2_MARK,
> >> +	VI1_G3_MARK, VI1_G4_MARK, VI1_G5_MARK,
> >> +	VI1_G6_MARK, VI1_G7_MARK,
> >> +};
> >> +static const unsigned int vin1_data_r_pins[] = {
> >> +	RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28), RCAR_GP_PIN(0, 29),
> >> +	RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5), RCAR_GP_PIN(1, 6),
> >> +	RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 8),
> >> +};
> >> +static const unsigned int vin1_data_r_mux[] = {
> >> +	VI1_R0_MARK, VI1_R1_MARK, VI1_R2_MARK,
> >> +	VI1_R3_MARK, VI1_R4_MARK, VI1_R5_MARK,
> >> +	VI1_R6_MARK, VI1_R7_MARK,
> >> +};
> >> +static const unsigned int vin1_data_b_pins[] = {
> >>   	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11), RCAR_GP_PIN(2, 12),
> >>   	RCAR_GP_PIN(2, 13), RCAR_GP_PIN(2, 14), RCAR_GP_PIN(2, 15),
> >>   	RCAR_GP_PIN(2, 16), RCAR_GP_PIN(2, 17),
> >>   };
> >> 
> >> -static const unsigned int vin1_data_mux[] = {
> >> +static const unsigned int vin1_data_b_mux[] = {
> >> 
> >>   	VI1_DATA0_VI1_B0_MARK, VI1_DATA1_VI1_B1_MARK, VI1_DATA2_VI1_B2_MARK,
> >>   	VI1_DATA3_VI1_B3_MARK, VI1_DATA4_VI1_B4_MARK, VI1_DATA5_VI1_B5_MARK,
> >>   	VI1_DATA6_VI1_B6_MARK, VI1_DATA7_VI1_B7_MARK,
> >>   
> >>   };
> > 
> > Given the wide variety of ways the VIN data pins can be used, I see two
> > ways to group them:
> > 
> > - List all possible combinations of data pins with a single group per
> > configuration. There are 11 combinations, but due to overlap in the pins
> > used the number can be reduced to 8.
> > 
> > - List groups individually and combine them to create a valid
> > configuration. We would have at least 8 groups in this case as well.
> > 
> > Given that the number of groups would be identical, my preference would go
> > for the first solution. The RGB data pin group would contain all 24 RGB
> > pins in that case, and additional overlapping groups would be defined for
> > other input configurations. Would you be fine with that ? VIN1 would be
> > handled identically, and VIN2 would need 5 groups only. VIN3 is already
> > defined correctly, as it supports a single configuration only.
> 
> I'm only adding missing stuff here. VIN0/VIN1 are identical so I'm adding
> VIN1 the same way VIN0 is currently grouped.
> I'm generally fine with re-grouping data pins, but I think it should be a
> separate change. We'd also need to choose simple names for the groups which
> are easily readable and show which pins are used there. Something like:
> data_r07g07b07 (24 bits for RGB24)
> data_r27g27b27 (pins 2-7 for ITU-R BT.601/BT.709 RGB-666)

Renaming pin groups can be an issue if they are used by existing platforms, so 
I'd rather first think about the way pins should be grouped, fix the existing 
code if needed, and then adding the new groups. Would that be fine ?

> >> +static const unsigned int vin1_hsync_signal_pins[] = {
> > 
> > I think you can drop _signal here and below.
> 
> I didn't intend to edit the existing naming. All I wanted to do is add the
> definitions for the missing pins here. I think name editing should be done
> by a separate patch if needed. IMHO, otherwise it would be a mess.
> 
> >> +	RCAR_GP_PIN(1, 24),
> >> +};
> >> +static const unsigned int vin1_hsync_signal_mux[] = {
> >> +	VI1_HSYNC_N_MARK,
> >> +};
> >> +static const unsigned int vin1_vsync_signal_pins[] = {
> >> +	RCAR_GP_PIN(1, 25),
> >> +};
> >> +static const unsigned int vin1_vsync_signal_mux[] = {
> >> +	VI1_VSYNC_N_MARK,
> >> +};
> > 
> > If I'm not mistaken the hsync and vsync signals can't be used
> > independently, so they should be grouped together.
> 
> I'm just using the current VIN0 layout for VIN1 here.
> I'm not intending to regroup anything here. The purpose of these
> changes is to add missing pins. I believe regrouping should be done
> by a separate patch if needed.

Sure, but could you then please send patches that drop _signal and group 
hsync+vsync, and rebase this patch on top of that ?
Valentine Barshak Dec. 10, 2013, 11:45 a.m. UTC | #5
On 12/10/2013 05:45 AM, Laurent Pinchart wrote:
> Hi Valentine,
>
> On Monday 09 December 2013 23:14:29 Valentine wrote:
>> On 10/29/2013 10:10 PM, Laurent Pinchart wrote:
>>> On Friday 18 October 2013 00:06:43 Valentine Barshak wrote:
>>>> This adds missing VIN1/2/3 pin configuration to pfc-r8a7790.
>>>>
>>>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>>>> ---
>>>>
>>>>    drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 202 +++++++++++++++++++++++++++-
>>>>    1 file changed, 198 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
>>>> b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c index 64fcc006..9ab2f71 100644
>>>> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
>>>> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
>>>> @@ -2996,22 +2996,168 @@ static const unsigned int vin0_clk_mux[] = {
>>>>    	VI0_CLK_MARK,
>>>>    };
>>>>    /* - VIN1 ---------------------------------------------------------- */
>>>> -static const unsigned int vin1_data_pins[] = {
>>>> +static const unsigned int vin1_data_g_pins[] = {
>>>> +	RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 17),
>>>> +	RCAR_GP_PIN(1, 20), RCAR_GP_PIN(1, 22), RCAR_GP_PIN(1, 12),
>>>> +	RCAR_GP_PIN(1, 9), RCAR_GP_PIN(1, 7),
>>>> +};
>>>> +static const unsigned int vin1_data_g_mux[] = {
>>>> +	VI1_G0_MARK, VI1_G1_MARK, VI1_G2_MARK,
>>>> +	VI1_G3_MARK, VI1_G4_MARK, VI1_G5_MARK,
>>>> +	VI1_G6_MARK, VI1_G7_MARK,
>>>> +};
>>>> +static const unsigned int vin1_data_r_pins[] = {
>>>> +	RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28), RCAR_GP_PIN(0, 29),
>>>> +	RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5), RCAR_GP_PIN(1, 6),
>>>> +	RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 8),
>>>> +};
>>>> +static const unsigned int vin1_data_r_mux[] = {
>>>> +	VI1_R0_MARK, VI1_R1_MARK, VI1_R2_MARK,
>>>> +	VI1_R3_MARK, VI1_R4_MARK, VI1_R5_MARK,
>>>> +	VI1_R6_MARK, VI1_R7_MARK,
>>>> +};
>>>> +static const unsigned int vin1_data_b_pins[] = {
>>>>    	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11), RCAR_GP_PIN(2, 12),
>>>>    	RCAR_GP_PIN(2, 13), RCAR_GP_PIN(2, 14), RCAR_GP_PIN(2, 15),
>>>>    	RCAR_GP_PIN(2, 16), RCAR_GP_PIN(2, 17),
>>>>    };
>>>>
>>>> -static const unsigned int vin1_data_mux[] = {
>>>> +static const unsigned int vin1_data_b_mux[] = {
>>>>
>>>>    	VI1_DATA0_VI1_B0_MARK, VI1_DATA1_VI1_B1_MARK, VI1_DATA2_VI1_B2_MARK,
>>>>    	VI1_DATA3_VI1_B3_MARK, VI1_DATA4_VI1_B4_MARK, VI1_DATA5_VI1_B5_MARK,
>>>>    	VI1_DATA6_VI1_B6_MARK, VI1_DATA7_VI1_B7_MARK,
>>>>
>>>>    };
>>>
>>> Given the wide variety of ways the VIN data pins can be used, I see two
>>> ways to group them:
>>>
>>> - List all possible combinations of data pins with a single group per
>>> configuration. There are 11 combinations, but due to overlap in the pins
>>> used the number can be reduced to 8.
>>>
>>> - List groups individually and combine them to create a valid
>>> configuration. We would have at least 8 groups in this case as well.
>>>
>>> Given that the number of groups would be identical, my preference would go
>>> for the first solution. The RGB data pin group would contain all 24 RGB
>>> pins in that case, and additional overlapping groups would be defined for
>>> other input configurations. Would you be fine with that ? VIN1 would be
>>> handled identically, and VIN2 would need 5 groups only. VIN3 is already
>>> defined correctly, as it supports a single configuration only.
>>
>> I'm only adding missing stuff here. VIN0/VIN1 are identical so I'm adding
>> VIN1 the same way VIN0 is currently grouped.
>> I'm generally fine with re-grouping data pins, but I think it should be a
>> separate change. We'd also need to choose simple names for the groups which
>> are easily readable and show which pins are used there. Something like:
>> data_r07g07b07 (24 bits for RGB24)
>> data_r27g27b27 (pins 2-7 for ITU-R BT.601/BT.709 RGB-666)
>
> Renaming pin groups can be an issue if they are used by existing platforms, so
> I'd rather first think about the way pins should be grouped, fix the existing
> code if needed, and then adding the new groups. Would that be fine ?
>
>>>> +static const unsigned int vin1_hsync_signal_pins[] = {
>>>
>>> I think you can drop _signal here and below.
>>
>> I didn't intend to edit the existing naming. All I wanted to do is add the
>> definitions for the missing pins here. I think name editing should be done
>> by a separate patch if needed. IMHO, otherwise it would be a mess.
>>
>>>> +	RCAR_GP_PIN(1, 24),
>>>> +};
>>>> +static const unsigned int vin1_hsync_signal_mux[] = {
>>>> +	VI1_HSYNC_N_MARK,
>>>> +};
>>>> +static const unsigned int vin1_vsync_signal_pins[] = {
>>>> +	RCAR_GP_PIN(1, 25),
>>>> +};
>>>> +static const unsigned int vin1_vsync_signal_mux[] = {
>>>> +	VI1_VSYNC_N_MARK,
>>>> +};
>>>
>>> If I'm not mistaken the hsync and vsync signals can't be used
>>> independently, so they should be grouped together.
>>
>> I'm just using the current VIN0 layout for VIN1 here.
>> I'm not intending to regroup anything here. The purpose of these
>> changes is to add missing pins. I believe regrouping should be done
>> by a separate patch if needed.
>
> Sure, but could you then please send patches that drop _signal and group
> hsync+vsync, and rebase this patch on top of that ?
>

OK, I'll take a look at that, but I don't see much of a point in adjusting the names
and modifying/rebasing this patch on top of that.
I think that would just involve more work, while eventually having the same result.
I was planning to regroup the pins and rename the groups on top of this one.

Thanks,
Val.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
index 64fcc006..9ab2f71 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
@@ -2996,22 +2996,168 @@  static const unsigned int vin0_clk_mux[] = {
 	VI0_CLK_MARK,
 };
 /* - VIN1 ------------------------------------------------------------------- */
-static const unsigned int vin1_data_pins[] = {
+static const unsigned int vin1_data_g_pins[] = {
+	RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 17),
+	RCAR_GP_PIN(1, 20), RCAR_GP_PIN(1, 22), RCAR_GP_PIN(1, 12),
+	RCAR_GP_PIN(1, 9), RCAR_GP_PIN(1, 7),
+};
+static const unsigned int vin1_data_g_mux[] = {
+	VI1_G0_MARK, VI1_G1_MARK, VI1_G2_MARK,
+	VI1_G3_MARK, VI1_G4_MARK, VI1_G5_MARK,
+	VI1_G6_MARK, VI1_G7_MARK,
+};
+static const unsigned int vin1_data_r_pins[] = {
+	RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28), RCAR_GP_PIN(0, 29),
+	RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5), RCAR_GP_PIN(1, 6),
+	RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 8),
+};
+static const unsigned int vin1_data_r_mux[] = {
+	VI1_R0_MARK, VI1_R1_MARK, VI1_R2_MARK,
+	VI1_R3_MARK, VI1_R4_MARK, VI1_R5_MARK,
+	VI1_R6_MARK, VI1_R7_MARK,
+};
+static const unsigned int vin1_data_b_pins[] = {
 	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11), RCAR_GP_PIN(2, 12),
 	RCAR_GP_PIN(2, 13), RCAR_GP_PIN(2, 14), RCAR_GP_PIN(2, 15),
 	RCAR_GP_PIN(2, 16), RCAR_GP_PIN(2, 17),
 };
-static const unsigned int vin1_data_mux[] = {
+static const unsigned int vin1_data_b_mux[] = {
 	VI1_DATA0_VI1_B0_MARK, VI1_DATA1_VI1_B1_MARK, VI1_DATA2_VI1_B2_MARK,
 	VI1_DATA3_VI1_B3_MARK, VI1_DATA4_VI1_B4_MARK, VI1_DATA5_VI1_B5_MARK,
 	VI1_DATA6_VI1_B6_MARK, VI1_DATA7_VI1_B7_MARK,
 };
+static const unsigned int vin1_hsync_signal_pins[] = {
+	RCAR_GP_PIN(1, 24),
+};
+static const unsigned int vin1_hsync_signal_mux[] = {
+	VI1_HSYNC_N_MARK,
+};
+static const unsigned int vin1_vsync_signal_pins[] = {
+	RCAR_GP_PIN(1, 25),
+};
+static const unsigned int vin1_vsync_signal_mux[] = {
+	VI1_VSYNC_N_MARK,
+};
+static const unsigned int vin1_field_signal_pins[] = {
+	RCAR_GP_PIN(1, 13),
+};
+static const unsigned int vin1_field_signal_mux[] = {
+	VI1_FIELD_MARK,
+};
+static const unsigned int vin1_data_enable_pins[] = {
+	RCAR_GP_PIN(1, 26),
+};
+static const unsigned int vin1_data_enable_mux[] = {
+	VI1_CLKENB_MARK,
+};
 static const unsigned int vin1_clk_pins[] = {
 	RCAR_GP_PIN(2, 9),
 };
 static const unsigned int vin1_clk_mux[] = {
 	VI1_CLK_MARK,
 };
+/* - VIN2 ----------------------------------------------------------------- */
+static const unsigned int vin2_data_g_pins[] = {
+	RCAR_GP_PIN(0, 27), RCAR_GP_PIN(0, 28), RCAR_GP_PIN(0, 29),
+	RCAR_GP_PIN(1, 10), RCAR_GP_PIN(1, 4), RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6), RCAR_GP_PIN(1, 7),
+};
+static const unsigned int vin2_data_g_mux[] = {
+	VI2_G0_MARK, VI2_G1_MARK, VI2_G2_MARK,
+	VI2_G3_MARK, VI2_G4_MARK, VI2_G5_MARK,
+	VI2_G6_MARK, VI2_G7_MARK,
+};
+static const unsigned int vin2_data_r_pins[] = {
+	RCAR_GP_PIN(1, 12), RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 20),
+	RCAR_GP_PIN(1, 22), RCAR_GP_PIN(1, 24),
+};
+static const unsigned int vin2_data_r_mux[] = {
+	VI2_R0_MARK, VI2_R1_MARK, VI2_R2_MARK,
+	VI2_R3_MARK, VI2_R4_MARK, VI2_R5_MARK,
+	VI2_R6_MARK, VI2_R7_MARK,
+};
+static const unsigned int vin2_data_b_pins[] = {
+	RCAR_GP_PIN(0, 8), RCAR_GP_PIN(0, 9), RCAR_GP_PIN(0, 10),
+	RCAR_GP_PIN(0, 11), RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
+	RCAR_GP_PIN(0, 14), RCAR_GP_PIN(0, 15),
+};
+static const unsigned int vin2_data_b_mux[] = {
+	VI2_DATA0_VI2_B0_MARK, VI2_DATA1_VI2_B1_MARK, VI2_DATA2_VI2_B2_MARK,
+	VI2_DATA3_VI2_B3_MARK, VI2_DATA4_VI2_B4_MARK, VI2_DATA5_VI2_B5_MARK,
+	VI2_DATA6_VI2_B6_MARK, VI2_DATA7_VI2_B7_MARK,
+};
+static const unsigned int vin2_hsync_signal_pins[] = {
+	RCAR_GP_PIN(1, 16),
+};
+static const unsigned int vin2_hsync_signal_mux[] = {
+	VI2_HSYNC_N_MARK,
+};
+static const unsigned int vin2_vsync_signal_pins[] = {
+	RCAR_GP_PIN(1, 21),
+};
+static const unsigned int vin2_vsync_signal_mux[] = {
+	VI2_VSYNC_N_MARK,
+};
+static const unsigned int vin2_field_signal_pins[] = {
+	RCAR_GP_PIN(1, 9),
+};
+static const unsigned int vin2_field_signal_mux[] = {
+	VI2_FIELD_MARK,
+};
+static const unsigned int vin2_data_enable_pins[] = {
+	RCAR_GP_PIN(1, 8),
+};
+static const unsigned int vin2_data_enable_mux[] = {
+	VI2_CLKENB_MARK,
+};
+static const unsigned int vin2_clk_pins[] = {
+	RCAR_GP_PIN(1, 11),
+};
+static const unsigned int vin2_clk_mux[] = {
+	VI2_CLK_MARK,
+};
+/* - VIN3 ----------------------------------------------------------------- */
+static const unsigned int vin3_data_pins[] = {
+	RCAR_GP_PIN(0, 0), RCAR_GP_PIN(0, 1), RCAR_GP_PIN(0, 2),
+	RCAR_GP_PIN(0, 3), RCAR_GP_PIN(0, 4), RCAR_GP_PIN(0, 5),
+	RCAR_GP_PIN(0, 6), RCAR_GP_PIN(0, 7),
+};
+static const unsigned int vin3_data_mux[] = {
+	VI3_DATA0_MARK, VI3_DATA1_MARK, VI3_DATA2_MARK,
+	VI3_DATA3_MARK, VI3_DATA4_MARK, VI3_DATA5_MARK,
+	VI3_DATA6_MARK, VI3_DATA7_MARK,
+};
+static const unsigned int vin3_hsync_signal_pins[] = {
+	RCAR_GP_PIN(1, 16),
+};
+static const unsigned int vin3_hsync_signal_mux[] = {
+	VI3_HSYNC_N_MARK,
+};
+static const unsigned int vin3_vsync_signal_pins[] = {
+	RCAR_GP_PIN(1, 17),
+};
+static const unsigned int vin3_vsync_signal_mux[] = {
+	VI2_VSYNC_N_MARK,
+};
+static const unsigned int vin3_field_signal_pins[] = {
+	RCAR_GP_PIN(1, 15),
+};
+static const unsigned int vin3_field_signal_mux[] = {
+	VI3_FIELD_MARK,
+};
+static const unsigned int vin3_data_enable_pins[] = {
+	RCAR_GP_PIN(1, 14),
+};
+static const unsigned int vin3_data_enable_mux[] = {
+	VI3_CLKENB_MARK,
+};
+static const unsigned int vin3_clk_pins[] = {
+	RCAR_GP_PIN(1, 23),
+};
+static const unsigned int vin3_clk_mux[] = {
+	VI3_CLK_MARK,
+};
 
 static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(du_rgb666),
@@ -3185,8 +3331,28 @@  static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(vin0_field_signal),
 	SH_PFC_PIN_GROUP(vin0_data_enable),
 	SH_PFC_PIN_GROUP(vin0_clk),
-	SH_PFC_PIN_GROUP(vin1_data),
+	SH_PFC_PIN_GROUP(vin1_data_g),
+	SH_PFC_PIN_GROUP(vin1_data_r),
+	SH_PFC_PIN_GROUP(vin1_data_b),
+	SH_PFC_PIN_GROUP(vin1_hsync_signal),
+	SH_PFC_PIN_GROUP(vin1_vsync_signal),
+	SH_PFC_PIN_GROUP(vin1_field_signal),
+	SH_PFC_PIN_GROUP(vin1_data_enable),
 	SH_PFC_PIN_GROUP(vin1_clk),
+	SH_PFC_PIN_GROUP(vin2_data_g),
+	SH_PFC_PIN_GROUP(vin2_data_r),
+	SH_PFC_PIN_GROUP(vin2_data_b),
+	SH_PFC_PIN_GROUP(vin2_hsync_signal),
+	SH_PFC_PIN_GROUP(vin2_vsync_signal),
+	SH_PFC_PIN_GROUP(vin2_field_signal),
+	SH_PFC_PIN_GROUP(vin2_data_enable),
+	SH_PFC_PIN_GROUP(vin2_clk),
+	SH_PFC_PIN_GROUP(vin3_data),
+	SH_PFC_PIN_GROUP(vin3_hsync_signal),
+	SH_PFC_PIN_GROUP(vin3_vsync_signal),
+	SH_PFC_PIN_GROUP(vin3_field_signal),
+	SH_PFC_PIN_GROUP(vin3_data_enable),
+	SH_PFC_PIN_GROUP(vin3_clk),
 };
 
 static const char * const du_groups[] = {
@@ -3457,10 +3623,36 @@  static const char * const vin0_groups[] = {
 };
 
 static const char * const vin1_groups[] = {
-	"vin1_data",
+	"vin1_data_g",
+	"vin1_data_r",
+	"vin1_data_b",
+	"vin1_hsync_signal",
+	"vin1_vsync_signal",
+	"vin1_field_signal",
+	"vin1_data_enable",
 	"vin1_clk",
 };
 
+static const char * const vin2_groups[] = {
+	"vin2_data_g",
+	"vin2_data_r",
+	"vin2_data_b",
+	"vin2_hsync_signal",
+	"vin2_vsync_signal",
+	"vin2_field_signal",
+	"vin2_data_enable",
+	"vin2_clk",
+};
+
+static const char * const vin3_groups[] = {
+	"vin3_data",
+	"vin3_hsync_signal",
+	"vin3_vsync_signal",
+	"vin3_field_signal",
+	"vin3_data_enable",
+	"vin3_clk",
+};
+
 static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(du),
 	SH_PFC_FUNCTION(du0),
@@ -3495,6 +3687,8 @@  static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(usb2),
 	SH_PFC_FUNCTION(vin0),
 	SH_PFC_FUNCTION(vin1),
+	SH_PFC_FUNCTION(vin2),
+	SH_PFC_FUNCTION(vin3),
 };
 
 static struct pinmux_cfg_reg pinmux_config_regs[] = {