| Message ID | 20200122080310.24653-1-faiz_abbas@ti.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add Support for MCAN in AM654x-idk | expand |
On 1/22/20 9:03 AM, Faiz Abbas wrote: > This series adds driver patches to support MCAN in TI's AM654x-idk. > > Faiz Abbas (3): > dt-bindings: net: can: m_can: Add Documentation for stb-gpios > can: m_can: m_can_platform: Add support for enabling transceiver > through the STB line > arm64: defconfig: Add Support for Bosch M_CAN controllers > > Documentation/devicetree/bindings/net/can/m_can.txt | 2 ++ > arch/arm64/configs/defconfig | 3 +++ > drivers/net/can/m_can/m_can_platform.c | 12 ++++++++++++ > 3 files changed, 17 insertions(+) What about adding support for xceiver-supply as done in several other drivers (ti_hecc.c, flexcan.c, mcp251x.c)? And using this for the stb line? Marc
Marc, On 23/01/20 4:47 pm, Marc Kleine-Budde wrote: > On 1/22/20 9:03 AM, Faiz Abbas wrote: >> This series adds driver patches to support MCAN in TI's AM654x-idk. >> >> Faiz Abbas (3): >> dt-bindings: net: can: m_can: Add Documentation for stb-gpios >> can: m_can: m_can_platform: Add support for enabling transceiver >> through the STB line >> arm64: defconfig: Add Support for Bosch M_CAN controllers >> >> Documentation/devicetree/bindings/net/can/m_can.txt | 2 ++ >> arch/arm64/configs/defconfig | 3 +++ >> drivers/net/can/m_can/m_can_platform.c | 12 ++++++++++++ >> 3 files changed, 17 insertions(+) > > What about adding support for xceiver-supply as done in several other > drivers (ti_hecc.c, flexcan.c, mcp251x.c)? And using this for the stb line? Looks like you had given this feedback a long time ago and I forgot about it. Sorry about that :-) https://lore.kernel.org/patchwork/patch/1006238/ But now that I think about it, its kinda weird that we are modelling part of the transceiver as a separate child node (Documentation/devicetree/bindings/net/can/can-transceiver.txt) and the other parts as a regulator. Anyone looking at the transceiver node would figure thats where the enable gpio/regulator node needs to go instead of the parent node. Shouldn't we have all transceiver properties under the same node? Thanks, Faiz
On 1/23/20 12:46 PM, Faiz Abbas wrote: > Marc, > > On 23/01/20 4:47 pm, Marc Kleine-Budde wrote: >> On 1/22/20 9:03 AM, Faiz Abbas wrote: >>> This series adds driver patches to support MCAN in TI's AM654x-idk. >>> >>> Faiz Abbas (3): >>> dt-bindings: net: can: m_can: Add Documentation for stb-gpios >>> can: m_can: m_can_platform: Add support for enabling transceiver >>> through the STB line >>> arm64: defconfig: Add Support for Bosch M_CAN controllers >>> >>> Documentation/devicetree/bindings/net/can/m_can.txt | 2 ++ >>> arch/arm64/configs/defconfig | 3 +++ >>> drivers/net/can/m_can/m_can_platform.c | 12 ++++++++++++ >>> 3 files changed, 17 insertions(+) >> >> What about adding support for xceiver-supply as done in several other >> drivers (ti_hecc.c, flexcan.c, mcp251x.c)? And using this for the stb line? > > Looks like you had given this feedback a long time ago and I forgot > about it. Sorry about that :-) > > https://lore.kernel.org/patchwork/patch/1006238/ > > But now that I think about it, its kinda weird that we are modelling > part of the transceiver as a separate child node > (Documentation/devicetree/bindings/net/can/can-transceiver.txt) and the > other parts as a regulator. We need a regulator, as there are dual phy chips with a single enable line. > Anyone looking at the transceiver node would figure thats where the > enable gpio/regulator node needs to go instead of the parent node. > Shouldn't we have all transceiver properties under the same node? Feel free to add support for the regulator to the transceiver node and convert the existing drivers to accept both bindings. Marc
This series adds driver patches to support MCAN in TI's AM654x-idk. Faiz Abbas (3): dt-bindings: net: can: m_can: Add Documentation for stb-gpios can: m_can: m_can_platform: Add support for enabling transceiver through the STB line arm64: defconfig: Add Support for Bosch M_CAN controllers Documentation/devicetree/bindings/net/can/m_can.txt | 2 ++ arch/arm64/configs/defconfig | 3 +++ drivers/net/can/m_can/m_can_platform.c | 12 ++++++++++++ 3 files changed, 17 insertions(+)