Re: [PATCH v2 1/2] pinctrl: sh-pfc: Add r8a77470 PFC support

[Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>]

On 04/28/2018 08:40 PM, Sergei Shtylyov wrote:

>>>> Subject: Re: [PATCH v2 1/2] pinctrl: sh-pfc: Add r8a77470 PFC support
>>>> On Wed, Apr 4, 2018 at 5:22 PM, Biju Das <biju.das@xxxxxxxxxxxxxx> wrote:
>>>>> Add PFC support for the R8A77470 SoC including pin groups for some
>>>>> on-chip devices such as SCIF, AVB and MMC.
>>>>>
>>>>> Signed-off-by: Biju Das <biju.das@xxxxxxxxxxxxxx>
>>>>> Reviewed-by: Fabrizio Castro <fabrizio.castro@xxxxxxxxxxxxxx>
>>
>>>>> --- /dev/null
>>>>> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77470.c
>>
>>>>> +/* - AVB
>>>>> +-------------------------------------------------------------------- */ static const
>>>> unsigned int avb_link_pins[] = {
>>>>> +       RCAR_GP_PIN(5, 14),
>>>>> +};
>>>>> +static const unsigned int avb_link_mux[] = {
>>>>> +       AVB_LINK_MARK,
>>>>> +};
>>>>> +static const unsigned int avb_magic_pins[] = {
>>>>> +       RCAR_GP_PIN(5, 15),
>>>>> +};
>>>>> +static const unsigned int avb_magic_mux[] = {
>>>>> +       AVB_MAGIC_MARK,
>>>>> +};
>>>>> +static const unsigned int avb_phy_int_pins[] = {
>>>>> +       RCAR_GP_PIN(5, 16),
>>>>> +};
>>>>> +static const unsigned int avb_phy_int_mux[] = {
>>>>> +       AVB_PHY_INT_MARK,
>>>>> +};
>>>>> +static const unsigned int avb_mdio_pins[] = {
>>>>> +       RCAR_GP_PIN(5, 12), RCAR_GP_PIN(5, 13), }; static const
>>>>> +unsigned int avb_mdio_mux[] = {
>>>>> +       AVB_MDC_MARK, AVB_MDIO_MARK,
>>>>> +};
>>>>> +static const unsigned int avb_mii_pins[] = {
>>>>> +       RCAR_GP_PIN(3, 14), RCAR_GP_PIN(3, 15), RCAR_GP_PIN(3, 16),
>>>>> +       RCAR_GP_PIN(3, 27),
>>>>> +
>>>>> +       RCAR_GP_PIN(3, 2), RCAR_GP_PIN(3, 3), RCAR_GP_PIN(3, 4),
>>>>> +       RCAR_GP_PIN(3, 5),
>>>>> +
>>>>> +       RCAR_GP_PIN(3, 10), RCAR_GP_PIN(3, 0), RCAR_GP_PIN(3, 1),
>>>>> +       RCAR_GP_PIN(5, 17), RCAR_GP_PIN(3, 13), RCAR_GP_PIN(5, 23),
>>>>> +       RCAR_GP_PIN(3, 12),
>>>>> +};
>>>>> +static const unsigned int avb_mii_mux[] = {
>>>>> +       AVB_TXD0_MARK, AVB_TXD1_MARK, AVB_TXD2_MARK,
>>>>> +       AVB_TXD3_MARK,
>>>>> +
>>>>> +       AVB_RXD0_MARK, AVB_RXD1_MARK, AVB_RXD2_MARK,
>>>>> +       AVB_RXD3_MARK,
>>>>> +
>>>>> +       AVB_RX_ER_MARK, AVB_RX_CLK_MARK, AVB_RX_DV_MARK,
>>>>> +       AVB_CRS_MARK, AVB_TX_EN_MARK, AVB_TX_ER_MARK,
>>>>> +       AVB_TX_CLK_MARK,
>>>>
>>>> You forgot AVB_COL, which is GP5_18?
>>>
>>> GP5_18 is shred between the signals AVB_COL and VI0_CLK. As per the RZ/G1C board schematic /hardware user guide
>>> Ethernet Phy collision pin(AVB_COL) pin  is not populated on this board by default. It is populated only for
>>> Video Input Channel0 pixel clock.
>>>
>>> Since it is initial submission, I thought of adding only board specific pins first and later
>>> add this collision pin alone as a separate pin entry  in the avb group. That way we can support existing board
>>> as well as any future RZ/G1C board which populates this pin. Are you ok for this approach?
> 
>> Oops. That means our grouping is suboptimal. Perhaps we should revisit
>> (for all SoCs, while keeping backwards compatibility)?
> 
> 
>> After reading some wikipedia, I came up with:
>>
>>     mii_tx
>>     mii_tx_er (optional)
>>     mii_rx
> 
>    I don't see why we should have the separate TX/RX subgroups just because they're
> described separately in the Wikipedia. Or did you have some special reason to do it? 
> 
>>     mii_col_crs (optional, half duplex)
>>
>> However, given your board uses AVB_CRS without AVB_COL, that would still
>> not be sufficient, so the last group should be split to cover your use case?
> 
>> BTW, how does it work with AVB_COL not wired to anything by default, and thus
>> floating? Do you enable pull-up/down for that pin in the PFC, or is the pin
>> just ignored in full-duplex mode?
> 
>    Well, I'm seeing the strong possibility the half-duplex mode just doesn't work as
> the collision indication isn't available.

   And indeed, the manuals tell me that RZ/G (and R-Car gen2) EtherAVB only supports
the full-duplex mode.

> [...]
> 
>> Gr{oetje,eeting}s,
>>
>>                         Geert

MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html