| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMZ6RqJ-3fyLFMjuyq4euNB-1sz0sBU5YswMeRduXO4TJ1QmLw@mail.gmail.com>
Date: Thu, 9 Mar 2023 00:53:11 +0900
From: Vincent Mailhol <vincent.mailhol@...il.com>
To: Geert Uytterhoeven <geert+renesas@...der.be>
Cc: Wolfgang Grandegger <wg@...ndegger.com>,
Marc Kleine-Budde <mkl@...gutronix.de>,
linux-can@...r.kernel.org, netdev@...r.kernel.org,
linux-renesas-soc@...r.kernel.org,
Simon Horman <simon.horman@...igine.com>
Subject: Re: [PATCH v2] can: rcar_canfd: Add transceiver support
On Wed. 8 Mar. 2023 at 22:20, Geert Uytterhoeven
<geert+renesas@...der.be> wrote:
> Add support for CAN transceivers described as PHYs.
>
> While simple CAN transceivers can do without, this is needed for CAN
> transceivers like NXP TJR1443 that need a configuration step (like
> pulling standby or enable lines), and/or impose a bitrate limit.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
> Reviewed-by: Simon Horman <simon.horman@...igine.com>
I have one nitpick (see below). Aside from that:
Reviewed-by: Vincent Mailhol <mailhol.vincent@...adoo.fr>
> ---
> v2:
> - Add Reviewed-by.
> ---
> drivers/net/can/rcar/rcar_canfd.c | 30 +++++++++++++++++++++++++-----
> 1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index ef4e1b9a9e1ee280..6df9a259e5e4f92c 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -35,6 +35,7 @@
> #include <linux/netdevice.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> #include <linux/reset.h>
> #include <linux/types.h>
> @@ -530,6 +531,7 @@ struct rcar_canfd_channel {
> struct net_device *ndev;
> struct rcar_canfd_global *gpriv; /* Controller reference */
> void __iomem *base; /* Register base address */
> + struct phy *transceiver; /* Optional transceiver */
> struct napi_struct napi;
> u32 tx_head; /* Incremented on xmit */
> u32 tx_tail; /* Incremented on xmit done */
> @@ -1413,11 +1415,17 @@ static int rcar_canfd_open(struct net_device *ndev)
> struct rcar_canfd_global *gpriv = priv->gpriv;
> int err;
>
> + err = phy_power_on(priv->transceiver);
> + if (err) {
> + netdev_err(ndev, "failed to power on PHY, error %d\n", err);
> + return err;
> + }
> +
> /* Peripheral clock is already enabled in probe */
> err = clk_prepare_enable(gpriv->can_clk);
> if (err) {
> netdev_err(ndev, "failed to enable CAN clock, error %d\n", err);
^^
Nitpick: can you print the mnemotechnic instead of the error value?
netdev_err(ndev, "failed to enable CAN clock, error
%pe\n", ERR_PTR(err));
> - goto out_clock;
> + goto out_phy;
> }
>
> err = open_candev(ndev);
> @@ -1437,7 +1445,8 @@ static int rcar_canfd_open(struct net_device *ndev)
> close_candev(ndev);
> out_can_clock:
> clk_disable_unprepare(gpriv->can_clk);
> -out_clock:
> +out_phy:
> + phy_power_off(priv->transceiver);
> return err;
> }
>
> @@ -1480,6 +1489,7 @@ static int rcar_canfd_close(struct net_device *ndev)
> napi_disable(&priv->napi);
> clk_disable_unprepare(gpriv->can_clk);
> close_candev(ndev);
> + phy_power_off(priv->transceiver);
> return 0;
> }
>
> @@ -1711,7 +1721,7 @@ static const struct ethtool_ops rcar_canfd_ethtool_ops = {
> };
>
> static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
> - u32 fcan_freq)
> + u32 fcan_freq, struct phy *transceiver)
> {
> const struct rcar_canfd_hw_info *info = gpriv->info;
> struct platform_device *pdev = gpriv->pdev;
> @@ -1732,8 +1742,11 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
> ndev->flags |= IFF_ECHO;
> priv->ndev = ndev;
> priv->base = gpriv->base;
> + priv->transceiver = transceiver;
> priv->channel = ch;
> priv->gpriv = gpriv;
> + if (transceiver)
> + priv->can.bitrate_max = transceiver->attrs.max_link_rate;
> priv->can.clock.freq = fcan_freq;
> dev_info(dev, "can_clk rate is %u\n", priv->can.clock.freq);
>
> @@ -1836,6 +1849,7 @@ static void rcar_canfd_channel_remove(struct rcar_canfd_global *gpriv, u32 ch)
>
> static int rcar_canfd_probe(struct platform_device *pdev)
> {
> + struct phy *transceivers[RCANFD_NUM_CHANNELS] = { 0, };
> const struct rcar_canfd_hw_info *info;
> struct device *dev = &pdev->dev;
> void __iomem *addr;
> @@ -1857,9 +1871,14 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> for (i = 0; i < info->max_channels; ++i) {
> name[7] = '0' + i;
> of_child = of_get_child_by_name(dev->of_node, name);
> - if (of_child && of_device_is_available(of_child))
> + if (of_child && of_device_is_available(of_child)) {
> channels_mask |= BIT(i);
> + transceivers[i] = devm_of_phy_optional_get(dev,
> + of_child, NULL);
> + }
> of_node_put(of_child);
> + if (IS_ERR(transceivers[i]))
> + return PTR_ERR(transceivers[i]);
> }
>
> if (info->shared_global_irqs) {
> @@ -2035,7 +2054,8 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> }
>
> for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels) {
> - err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq);
> + err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq,
> + transceivers[ch]);
> if (err)
> goto fail_channel;
> }
> --
> 2.34.1
>
Powered by blists - more mailing lists