netdev - Re: [PATCH v2] ravb: add wake-on-lan support via magic packet

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMuHMdXmyoMeq8K0BpEzviX1BymR0WyOHeXmNnuR-BBs1asn_Q@mail.gmail.com>
Date:   Mon, 14 Aug 2017 12:24:06 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Niklas Söderlund <niklas.soderlund@...natech.se>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>
Subject: Re: [PATCH v2] ravb: add wake-on-lan support via magic packet

Hi Andrew, Niklas,

On Sun, Jul 30, 2017 at 9:51 PM, Niklas Söderlund
<niklas.soderlund@...natech.se> wrote:
> On 2017-07-30 19:07:38 +0200, Andrew Lunn wrote:
>> > @@ -2041,6 +2073,11 @@ static int ravb_probe(struct platform_device *pdev)
>> >
>> >     priv->chip_id = chip_id;
>> >
>> > +   /* Get clock, if not found that's OK but Wake-On-Lan is unavailable */
>> > +   priv->clk = devm_clk_get(&pdev->dev, NULL);
>> > +   if (IS_ERR(priv->clk))
>> > +           priv->clk = NULL;
>>
>> Can you get EPROBE_DEFER returned?
>
> I don't think so, but I'm not sure :-)
>
> The clock I'm trying to get is the module clock of the ravb itself, so
> if that clock is not available (and enabled) no register writes to the
> ravb would be possible in the first place, so i guess it's safe to
> assume -EPROBE_DEFER can not happen here?

In theory, the devm_clk_get() can fail.
In practice, it cannot, as the EtherAVB device is part of a clock domain,
through its "power-domains" property in DT.
Hence if the clock (which is the module clock, provided by the SoC's system
clock controller (CPG/MSSR)) is not yet available, the PM Domain subsystem
will prevent the device from being probed.

> I'm just trying to play it safe here since the clock is only needed to
> support WoL, I though it best to not change behavior here. Try to get
> the clock, if we can great we can do WoL if not then user-space will be
> prevented from enabling WoL and nothing in the current behavior changes.

And all this explicit clock handling is done only because
device_set_wakeup_enable() does not yet prevent the PM Domain code from
powering down the device during system suspend.  Once that's handled by genpd,
all clock magic can be removed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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