Re: [PATCH] pinctrl: sh-pfc: r8a7795: Add DRIF support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ramesh,

On Wed, Jun 22, 2016 at 3:51 PM, Ramesh Shanmugasundaram
<ramesh.shanmugasundaram@xxxxxxxxxxxxxx> wrote:
>> CC linux-gpio, LinusW, Laurent
>>
>> On Fri, Jun 17, 2016 at 4:16 PM, Ramesh Shanmugasundaram
>> <ramesh.shanmugasundaram@xxxxxxxxxxxxxx> wrote:
>> >> >  drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 121
>> >> > +++++++++++++++++++++++++++++++++++
>> >> >  1 file changed, 121 insertions(+)
>> >> >
>> >> > diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
>> >> > b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
>> >> > index 33be5d56..6f246ec 100644
>> >> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
>> >> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
>> >> > @@ -1658,6 +1658,91 @@ static const unsigned int canfd1_data_mux[] =
>> {
>> >> >         CANFD1_TX_MARK,         CANFD1_RX_MARK,
>> >> >  };
>> >> >
>> >> > +/* - DRIF
>> >> > +--------------------------------------------------------------- */
>> >> static const unsigned int drif0_data_a_pins[] = {
>> >> > +       /* CLK, SYNC, D0, D1 */
>> >> > +       RCAR_GP_PIN(6, 8), RCAR_GP_PIN(6, 9), RCAR_GP_PIN(6, 10),
>> >> > +       RCAR_GP_PIN(6, 7),
>> >> > +};
>> >> > +static const unsigned int drif0_data_a_mux[] = {
>> >> > +       RIF0_CLK_A_MARK, RIF0_SYNC_A_MARK, RIF0_D0_A_MARK,
>> >> > +RIF0_D1_A_MARK, };
>> >>
>> >> According to my information, each DRIF module consists of two
>> >> sub-modules (that's why there are 8 module clocks for 4 DRIF
>> >> modules), each handling a data pin, but sharing the CLK and SYNC
>> signals.
>> >> Hence it's possible to receive using only one data pin. Is that
>> correct?
>> >
>> > Yes, that is correct.
>> >
>> >> Shouldn't the pinctrl data reflect that? The second unused data pin
>> >> could be used for something else.
>> >
>> > Is that possible? For e.g. when MOD_SEL0(bit8 -> sel_drif2) is set to 0,
>> all the 4 pins are owned by DRIF IP(RIF2_XXX_A set) even though one of the
>> RIF2_D0_A or RIF2_D1_A may be unused depending on the master it interfaces
>> with. Am I missing something?
>>
>> You're right, the whole block is switched.
>>
>> The unused data pin can still be configured as a GPIO, as the GPSR
>> register (to select between GPIO and IP function) takes precedence.
>
> Is this precedence mentioned in h/w manual or is it based on current code & execution order? I assume it's the later?

Please see Figure 6.2 ("Procedure for Changing Pin Function from Peripheral
function to GPIO."), which says clearing a bit in GPSR is sufficient to
configure the corresponding pin as a GPIO.

>> Linus/Laurent: I assume the GPIO also takes precedence in the Linux
>> pinctrl subsystem, or would it return an error, as the pin is claimed by
>> the IP group?
>
> Looking at the code, pinctrl core does not allow multiple pin requests/claims when "strict" pmxops flag is set. However, Renesas pinctrl driver does not set this flag. So the request can proceed further in our case.
>
> On my board, after IP claiming all the 4 pins I could still export one of the pins as GPIO (pinctrl subsystem is OK with it). From this point IP did not receive any data on this pin as expected. The other way around is around (GPIO->IP) is not possible because of this check in sh_pfc_func_set_mux.
>
> 359                 if (cfg->type != PINMUX_TYPE_NONE) {

Thanks for verifying!

> I am happy with splitting the drif pins into three groups based on the above observation
>
>   - "drif0_ctrl_a_pins" for CLK and SYNC,
>   - "drif0_data0_a_pins for D0,
>   - "drif0_data1_a_pins for D1.
>
> If you are OK, I will send the reworked patch.

Thanks, that sounds great!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux