| Message ID | 1480410716-25993-1-git-send-email-jacopo@jmondi.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Jacopo, Thank you for the patch. On Tuesday 29 Nov 2016 10:11:56 Jacopo Mondi wrote: > Add pin configuration support for Gyro-ADC, named ADI on r8a7791 SoC. > > The Gyro-ADC supports three different configurations: > a single ADC (adi and adi_b groups), 2 ADCs selectable through a single > channel select signal (adi_chsel1 and adi_chsel1_b groups), I've only briefly looked up at the datasheet, but is that a supported mode ? It seems that mode 1 uses 4 channels and mode 2 and 3 use 8 channels. > up to 4 ADCs > through 2 channel select signals (adi_chsel2 and adi_chsel2_b groups) > and up to 8 ADCs through 3 channel select signals (adi_chsel3 and > adi_chsel3_b groups) > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > > Compiled only, not tested with an actual ADC. > > For reference only, these are the changes introduced by Geert's private > review of first sketch of this patch: > * separate ADI chsel in 3 overlapping groups: > the user can now select how many pins to assign to channel selection > function. > > --- > drivers/pinctrl/sh-pfc/pfc-r8a7791.c | 86 +++++++++++++++++++++++++++++++++ > 1 file changed, 86 insertions(+) > > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c > b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c index 7ca37c3..3898747 100644 > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c > @@ -1691,6 +1691,72 @@ static const struct sh_pfc_pin pinmux_pins[] = { > PINMUX_GPIO_GP_ALL(), > }; > > +/* - ADI --------------------------------------------------------------- */ > +static const unsigned int adi_pins[] = { > + /* ADIDATA, ADICS, CLK*/ Missing space before */. Same comment in other places below. > + RCAR_GP_PIN(6, 24), RCAR_GP_PIN(6, 25), RCAR_GP_PIN(6, 26), > +}; > +static const unsigned int adi_mux[] = { > + /* ADIDATA, ADICS, CLK*/ > + ADIDATA_MARK, ADICS_SAMP_MARK, ADICLK_MARK, > +}; > +static const unsigned int adi_chsel1_pins[] = { > + /* CHS 0 */ > + RCAR_GP_PIN(6, 27), > +}; > +static const unsigned int adi_chsel1_mux[] = { > + /* CHS 0 */ > + ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK, One pin, three mux ? > +}; > +static const unsigned int adi_chsel2_pins[] = { > + /* CHS 0, 1 */ > + RCAR_GP_PIN(6, 27), RCAR_GP_PIN(6, 28), > +}; > +static const unsigned int adi_chsel2_mux[] = { > + /* CHS 0, 1 */ > + ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK, Two pins, three mux ? > +}; > +static const unsigned int adi_chsel3_pins[] = { > + /* CHS 0, 1, 2 */ > + RCAR_GP_PIN(6, 27), RCAR_GP_PIN(6, 28), RCAR_GP_PIN(6, 29), > +}; > +static const unsigned int adi_chsel3_mux[] = { > + /* CHS 0, 1, 2 */ > + ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK, > +}; > +static const unsigned int adi_b_pins[] = { > + /* ADIDATA B, ADICS B, CLK B */ > + RCAR_GP_PIN(5, 25), RCAR_GP_PIN(5, 26), RCAR_GP_PIN(5, 27), > +}; > +static const unsigned int adi_b_mux[] = { > + /* ADIDATA B, ADICS B, CLK B */ > + ADIDATA_B_MARK, ADICS_SAMP_B_MARK, ADICLK_B_MARK, > +}; > +static const unsigned int adi_chsel1_b_pins[] = { > + /* CHS B 0 */ > + RCAR_GP_PIN(5, 28), > +}; > +static const unsigned int adi_chsel1_b_mux[] = { > + /* CHS B 0 */ > + ADICHS0_B_MARK, > +}; > +static const unsigned int adi_chsel2_b_pins[] = { > + /* CHS B 0, 1 */ > + RCAR_GP_PIN(5, 28), RCAR_GP_PIN(5, 29), > +}; > +static const unsigned int adi_chsel2_b_mux[] = { > + /* CHS B 0, 1 */ > + ADICHS0_B_MARK, ADICHS1_B_MARK, > +}; > +static const unsigned int adi_chsel3_b_pins[] = { > + /* CHS B 0, 1, 2 */ > + RCAR_GP_PIN(5, 28), RCAR_GP_PIN(5, 29), RCAR_GP_PIN(5, 30), > +}; > +static const unsigned int adi_chsel3_b_mux[] = { > + /* CHS B 0, 1, 2 */ > + ADICHS0_B_MARK, ADICHS1_B_MARK, ADICHS2_B_MARK, > +}; > + > /* - Audio Clock ------------------------------------------------------- */ > static const unsigned int audio_clk_a_pins[] = { > /* CLK */ > @@ -4343,6 +4409,14 @@ static const unsigned int vin2_clk_mux[] = { > }; > > static const struct sh_pfc_pin_group pinmux_groups[] = { > + SH_PFC_PIN_GROUP(adi), > + SH_PFC_PIN_GROUP(adi_chsel1), > + SH_PFC_PIN_GROUP(adi_chsel2), > + SH_PFC_PIN_GROUP(adi_chsel3), > + SH_PFC_PIN_GROUP(adi_b), > + SH_PFC_PIN_GROUP(adi_chsel1_b), > + SH_PFC_PIN_GROUP(adi_chsel2_b), > + SH_PFC_PIN_GROUP(adi_chsel3_b), > SH_PFC_PIN_GROUP(audio_clk_a), > SH_PFC_PIN_GROUP(audio_clk_b), > SH_PFC_PIN_GROUP(audio_clk_b_b), > @@ -4687,6 +4761,17 @@ static const struct sh_pfc_pin_group pinmux_groups[] > = { SH_PFC_PIN_GROUP(vin2_clk), > }; > > +static const char * const adi_groups[] = { > + "adi", > + "adi_chsel1", > + "adi_chsel2", > + "adi_chsel3", > + "adi_b", > + "adi_chsel1_b", > + "adi_chsel2_b", > + "adi_chsel3_b", > +}; > + > static const char * const audio_clk_groups[] = { > "audio_clk_a", > "audio_clk_b", > @@ -5192,6 +5277,7 @@ static const char * const vin2_groups[] = { > }; > > static const struct sh_pfc_function pinmux_functions[] = { > + SH_PFC_FUNCTION(adi), > SH_PFC_FUNCTION(audio_clk), > SH_PFC_FUNCTION(avb), > SH_PFC_FUNCTION(can0),
Hi Laurent, On Tue, Nov 29, 2016 at 8:30 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Tuesday 29 Nov 2016 10:11:56 Jacopo Mondi wrote: >> Add pin configuration support for Gyro-ADC, named ADI on r8a7791 SoC. >> >> The Gyro-ADC supports three different configurations: >> a single ADC (adi and adi_b groups), 2 ADCs selectable through a single >> channel select signal (adi_chsel1 and adi_chsel1_b groups), > > I've only briefly looked up at the datasheet, but is that a supported mode ? > It seems that mode 1 uses 4 channels and mode 2 and 3 use 8 channels. I think you can always connect less ADCs than there are channels. If you connect e.g. only one, you don't need any CHS signals. If you connect e.g. two, you can use only one CHS signal. AFAICS, you still have to use an external demux to create individual chip selects from the single CS signal and (up to 3) CHS signals. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Nov 29, 2016 at 10:39 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Tue, Nov 29, 2016 at 8:30 PM, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: >> On Tuesday 29 Nov 2016 10:11:56 Jacopo Mondi wrote: >>> Add pin configuration support for Gyro-ADC, named ADI on r8a7791 SoC. >>> >>> The Gyro-ADC supports three different configurations: >>> a single ADC (adi and adi_b groups), 2 ADCs selectable through a single >>> channel select signal (adi_chsel1 and adi_chsel1_b groups), >> >> I've only briefly looked up at the datasheet, but is that a supported mode ? >> It seems that mode 1 uses 4 channels and mode 2 and 3 use 8 channels. > > I think you can always connect less ADCs than there are channels. > If you connect e.g. only one, you don't need any CHS signals. > If you connect e.g. two, you can use only one CHS signal. > > AFAICS, you still have to use an external demux to create individual chip > selects from the single CS signal and (up to 3) CHS signals. Upon closer look, the demux is intended to be at the analog side, but that doesn't matter much. When using an MB88101A, the (4ch) demux is part of the ADC chip, which has 4 analog inputs and 2 channel select inputs. The other supported ADCs (ADCS7476/ADC121, AD7476, MAX1162) are really single-channel ADCs, and thus require an external demux. But nobody says you have to use/populate all channels. The Gyro-ADC will still scan all 4 or 8 channels, though. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Jacopo, On Tue, Nov 29, 2016 at 10:11 AM, Jacopo Mondi <jacopo@jmondi.org> wrote: > Add pin configuration support for Gyro-ADC, named ADI on r8a7791 SoC. > > The Gyro-ADC supports three different configurations: > a single ADC (adi and adi_b groups), 2 ADCs selectable through a single > channel select signal (adi_chsel1 and adi_chsel1_b groups), up to 4 ADCs > through 2 channel select signals (adi_chsel2 and adi_chsel2_b groups) > and up to 8 ADCs through 3 channel select signals (adi_chsel3 and > adi_chsel3_b groups) > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > +static const char * const adi_groups[] = { > + "adi", As the group names below have a suffix ("_chselX"), I think it would be good to have a suffix in this group name, too. Usually we have splits like "data" and "ctrl", but in this case, the group covers both data and control signals. "adi_common"? "adi_base"? "adi_spi" (but it's only SPI in mode 2/3)? I'm out of inspiration... > + "adi_chsel1", > + "adi_chsel2", > + "adi_chsel3", Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Laurent, Geert,
thanks for review
On 29/11/2016 22:39, Geert Uytterhoeven wrote:
> Hi Laurent,
>
> On Tue, Nov 29, 2016 at 8:30 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>> On Tuesday 29 Nov 2016 10:11:56 Jacopo Mondi wrote:
>>> Add pin configuration support for Gyro-ADC, named ADI on r8a7791 SoC.
>>>
>>> The Gyro-ADC supports three different configurations:
>>> a single ADC (adi and adi_b groups), 2 ADCs selectable through a single
>>> channel select signal (adi_chsel1 and adi_chsel1_b groups),
>>
>> I've only briefly looked up at the datasheet, but is that a supported mode ?
>> It seems that mode 1 uses 4 channels and mode 2 and 3 use 8 channels.
>
> I think you can always connect less ADCs than there are channels.
> If you connect e.g. only one, you don't need any CHS signals.
> If you connect e.g. two, you can use only one CHS signal.
>
> AFAICS, you still have to use an external demux to create individual chip
> selects from the single CS signal and (up to 3) CHS signals.
Actually Laurent's right and I have mis-interpreted the CHS signal purpose.
The CHS signal are not intended to select an external ADC where to
sample from among the several connected ones, but instead to select
which line to drive to a single ADC.
Quoting the processor manual, ADI supports three different ADC models
through 3 different "modes"
- mode1: MB88101A
- mode2: ADCS7476/ADC121 and AD7476
- mode3: MAX1162
and cycles through channels moving the CHS[0:1] lines in mode1, and
CHS[0:2] lines in mode2 and mode3.
Actually, being the MB88101A a 4-channel ADC, the CHS[0:1] lines could
be connected to the ADC itself and used to cycle between its 4 input pins.
In mode2 and mode3 an external DEMUX is probably required, as the
supported ADCs for these modes are single-channel ones.
For pin configuration purposes anyway, it's enough to know that we
cannot split CHS signals in three separate groups, but in 2 only (one
for mode1 and one for mode2 and 3).
Will send v2 shortly.
Thanks
j
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c index 7ca37c3..3898747 100644 --- a/drivers/pinctrl/sh-pfc/pfc-r8a7791.c +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7791.c @@ -1691,6 +1691,72 @@ static const struct sh_pfc_pin pinmux_pins[] = { PINMUX_GPIO_GP_ALL(), }; +/* - ADI -------------------------------------------------------------------- */ +static const unsigned int adi_pins[] = { + /* ADIDATA, ADICS, CLK*/ + RCAR_GP_PIN(6, 24), RCAR_GP_PIN(6, 25), RCAR_GP_PIN(6, 26), +}; +static const unsigned int adi_mux[] = { + /* ADIDATA, ADICS, CLK*/ + ADIDATA_MARK, ADICS_SAMP_MARK, ADICLK_MARK, +}; +static const unsigned int adi_chsel1_pins[] = { + /* CHS 0 */ + RCAR_GP_PIN(6, 27), +}; +static const unsigned int adi_chsel1_mux[] = { + /* CHS 0 */ + ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK, +}; +static const unsigned int adi_chsel2_pins[] = { + /* CHS 0, 1 */ + RCAR_GP_PIN(6, 27), RCAR_GP_PIN(6, 28), +}; +static const unsigned int adi_chsel2_mux[] = { + /* CHS 0, 1 */ + ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK, +}; +static const unsigned int adi_chsel3_pins[] = { + /* CHS 0, 1, 2 */ + RCAR_GP_PIN(6, 27), RCAR_GP_PIN(6, 28), RCAR_GP_PIN(6, 29), +}; +static const unsigned int adi_chsel3_mux[] = { + /* CHS 0, 1, 2 */ + ADICHS0_MARK, ADICHS1_MARK, ADICHS2_MARK, +}; +static const unsigned int adi_b_pins[] = { + /* ADIDATA B, ADICS B, CLK B */ + RCAR_GP_PIN(5, 25), RCAR_GP_PIN(5, 26), RCAR_GP_PIN(5, 27), +}; +static const unsigned int adi_b_mux[] = { + /* ADIDATA B, ADICS B, CLK B */ + ADIDATA_B_MARK, ADICS_SAMP_B_MARK, ADICLK_B_MARK, +}; +static const unsigned int adi_chsel1_b_pins[] = { + /* CHS B 0 */ + RCAR_GP_PIN(5, 28), +}; +static const unsigned int adi_chsel1_b_mux[] = { + /* CHS B 0 */ + ADICHS0_B_MARK, +}; +static const unsigned int adi_chsel2_b_pins[] = { + /* CHS B 0, 1 */ + RCAR_GP_PIN(5, 28), RCAR_GP_PIN(5, 29), +}; +static const unsigned int adi_chsel2_b_mux[] = { + /* CHS B 0, 1 */ + ADICHS0_B_MARK, ADICHS1_B_MARK, +}; +static const unsigned int adi_chsel3_b_pins[] = { + /* CHS B 0, 1, 2 */ + RCAR_GP_PIN(5, 28), RCAR_GP_PIN(5, 29), RCAR_GP_PIN(5, 30), +}; +static const unsigned int adi_chsel3_b_mux[] = { + /* CHS B 0, 1, 2 */ + ADICHS0_B_MARK, ADICHS1_B_MARK, ADICHS2_B_MARK, +}; + /* - Audio Clock ------------------------------------------------------------ */ static const unsigned int audio_clk_a_pins[] = { /* CLK */ @@ -4343,6 +4409,14 @@ static const unsigned int vin2_clk_mux[] = { }; static const struct sh_pfc_pin_group pinmux_groups[] = { + SH_PFC_PIN_GROUP(adi), + SH_PFC_PIN_GROUP(adi_chsel1), + SH_PFC_PIN_GROUP(adi_chsel2), + SH_PFC_PIN_GROUP(adi_chsel3), + SH_PFC_PIN_GROUP(adi_b), + SH_PFC_PIN_GROUP(adi_chsel1_b), + SH_PFC_PIN_GROUP(adi_chsel2_b), + SH_PFC_PIN_GROUP(adi_chsel3_b), SH_PFC_PIN_GROUP(audio_clk_a), SH_PFC_PIN_GROUP(audio_clk_b), SH_PFC_PIN_GROUP(audio_clk_b_b), @@ -4687,6 +4761,17 @@ static const struct sh_pfc_pin_group pinmux_groups[] = { SH_PFC_PIN_GROUP(vin2_clk), }; +static const char * const adi_groups[] = { + "adi", + "adi_chsel1", + "adi_chsel2", + "adi_chsel3", + "adi_b", + "adi_chsel1_b", + "adi_chsel2_b", + "adi_chsel3_b", +}; + static const char * const audio_clk_groups[] = { "audio_clk_a", "audio_clk_b", @@ -5192,6 +5277,7 @@ static const char * const vin2_groups[] = { }; static const struct sh_pfc_function pinmux_functions[] = { + SH_PFC_FUNCTION(adi), SH_PFC_FUNCTION(audio_clk), SH_PFC_FUNCTION(avb), SH_PFC_FUNCTION(can0),
Add pin configuration support for Gyro-ADC, named ADI on r8a7791 SoC. The Gyro-ADC supports three different configurations: a single ADC (adi and adi_b groups), 2 ADCs selectable through a single channel select signal (adi_chsel1 and adi_chsel1_b groups), up to 4 ADCs through 2 channel select signals (adi_chsel2 and adi_chsel2_b groups) and up to 8 ADCs through 3 channel select signals (adi_chsel3 and adi_chsel3_b groups) Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- Compiled only, not tested with an actual ADC. For reference only, these are the changes introduced by Geert's private review of first sketch of this patch: * separate ADI chsel in 3 overlapping groups: the user can now select how many pins to assign to channel selection function. --- drivers/pinctrl/sh-pfc/pfc-r8a7791.c | 86 ++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+)