From: Tobias Waldekranz <tobias@waldekranz•com>
To: Daniel Golle <daniel@makrotopia•org>
Cc: "Russell King (Oracle)" <linux@armlinux•org.uk>,
"Marek Behún" <kabel@kernel•org>,
davem@davemloft•net, kuba@kernel•org, andrew@lunn•ch,
hkallweit1@gmail•com, robh+dt@kernel•org,
krzysztof.kozlowski+dt@linaro•org, conor+dt@kernel•org,
netdev@vger•kernel.org, devicetree@vger•kernel.org
Subject: Re: [PATCH net-next 1/4] net: phy: marvell10g: Support firmware loading on 88X3310
Date: Sun, 06 Oct 2024 23:32:48 +0200 [thread overview]
Message-ID: <87h69oc3y7.fsf@waldekranz.com> (raw)
In-Reply-To: <ZwK3t_uEErLXlaQy@makrotopia.org>
On sön, okt 06, 2024 at 17:15, Daniel Golle <daniel@makrotopia•org> wrote:
> Hi Tobias,
Hi Daniel,
> On Tue, Jan 02, 2024 at 02:09:24PM +0100, Tobias Waldekranz wrote:
>> On tis, jan 02, 2024 at 10:12, "Russell King (Oracle)" <linux@armlinux•org.uk> wrote:
>> > On Tue, Dec 19, 2023 at 11:15:41AM +0100, Tobias Waldekranz wrote:
>> >> On tis, dec 19, 2023 at 10:22, Marek Behún <kabel@kernel•org> wrote:
>> >> > On Thu, 14 Dec 2023 21:14:39 +0100
>> >> > Tobias Waldekranz <tobias@waldekranz•com> wrote:
>> >> >
>> >> >> +MODULE_FIRMWARE("mrvl/x3310fw.hdr");
>> >> >
>> >> > And do you have permission to publish this firmware into linux-firmware?
>> >>
>> >> No, I do not.
>> >>
>> >> > Because when we tried this with Marvell, their lawyer guy said we can't
>> >> > do that...
>> >>
>> >> I don't even have good enough access to ask the question, much less get
>> >> rejected by Marvell :) I just used that path so that it would line up
>> >> with linux-firmware if Marvell was to publish it in the future.
>> >>
>> >> Should MODULE_FIRMWARE be avoided for things that are not in
>> >> linux-firmware?
>> >
>> > Without the firmware being published, what use is having this code in
>> > mainline kernels?
>>
>> Personally, I primarily want this merged so that future contributions to
>> the driver are easier to develop, since I'll be able test them on top of
>> a clean net-next base.
>
> I've been pointed to your series by Krzysztof Kozlowski who had reviewed
> the DT part of it. Are you still working on that or going to eventually
> re-submit it?
I'm not actively working on it, no, but I still want to get it
merged. I, perhaps wrongly, interpreted Russell's lack of reply to my
motivation for accepting the firmware loading without having the binary
in linux-firmware, as a NAK, and moved on to other things.
> I understand that the suggested LED support pre-dates commit
>
> 7ae215ee7bb8 net: phy: add support for PHY LEDs polarity modes
>
> which would allow using generic properties 'active-low' and
> 'inactive-high-impedance'. I assume that would be applicable to the LED
> patch which was part of this series as well?
>
> In that case, we would no longer need a vendor-specific property for that
> purpose. If the LEDs are active-low by default (or early boot firmware
> setting) and you would need a property for setting them to 'active-high'
> instead, I just suggested that in
>
> https://patchwork.kernel.org/project/netdevbpf/patch/e91ca84ac836fc40c94c52733f8fc607bcbed64c.1728145095.git.daniel@makrotopia.org/
>
> which is why I'm now contacting you, as I was a bit confused by Krzysztof's
> suggestion to take a look at marvell,marvell10g.yaml which would have been
> introduced by your series.
>
> Imho it would be better to use the (now existing) generic properties than
> resorting to a vendor-specific one.
>
> In every case, if you have a minute to look at commit 7ae215ee7bb8 and let
> us know whether that structure, with or without my suggested addition,
> would be suitable for your case as well, that would be nice.
To me, it seems cleaner to have a single attribute that defines the
behavior you want on the pin (as a string enum). That way you can also
explicitly declare that the kernel shouldn't mess with the settings
(e.g., `polarity = "keep";`, like you can do with the initial
brightness).
If you go that way, I would prefer if the "old" attributes where
deprecated and only evaluated in case the new attribute is not
available.
As for how generic it should be: To me it doesn't seem like there's
anything PHY-specific about it. I suppose where it might be confusing
would be when you have a gpio-led, when that is already taken care of at
the GPIO layer.
next prev parent reply other threads:[~2024-10-06 21:32 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-14 20:14 [PATCH net-next 0/4] net: phy: marvell10g: Firmware loading and LED support for 88X3310 Tobias Waldekranz
2023-12-14 20:14 ` [PATCH net-next 1/4] net: phy: marvell10g: Support firmware loading on 88X3310 Tobias Waldekranz
2023-12-15 14:30 ` Andrew Lunn
2023-12-15 14:34 ` Russell King (Oracle)
2023-12-18 17:11 ` Tobias Waldekranz
2023-12-15 14:33 ` Andrew Lunn
2023-12-15 15:52 ` Russell King (Oracle)
2023-12-16 14:35 ` kernel test robot
2023-12-19 9:22 ` Marek Behún
2023-12-19 10:15 ` Tobias Waldekranz
2023-12-19 10:49 ` Marek Behún
2023-12-19 13:15 ` Tobias Waldekranz
2024-01-02 10:12 ` Russell King (Oracle)
2024-01-02 13:09 ` Tobias Waldekranz
2024-10-06 16:15 ` Daniel Golle
2024-10-06 21:32 ` Tobias Waldekranz [this message]
2023-12-14 20:14 ` [PATCH net-next 2/4] net: phy: marvell10g: Fix power-up when strapped to start powered down Tobias Waldekranz
2023-12-15 15:44 ` Russell King (Oracle)
2023-12-18 18:02 ` Tobias Waldekranz
2023-12-14 20:14 ` [PATCH net-next 3/4] net: phy: marvell10g: Add LED support for 88X3310 Tobias Waldekranz
2023-12-15 14:44 ` Andrew Lunn
2023-12-15 15:12 ` Russell King (Oracle)
2023-12-18 15:55 ` Simon Horman
2023-12-14 20:14 ` [PATCH net-next 4/4] dt-bindings: net: marvell10g: Document LED polarity Tobias Waldekranz
2023-12-15 8:47 ` Krzysztof Kozlowski
2023-12-15 11:19 ` Andrew Lunn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87h69oc3y7.fsf@waldekranz.com \
--to=tobias@waldekranz$(echo .)com \
--cc=andrew@lunn$(echo .)ch \
--cc=conor+dt@kernel$(echo .)org \
--cc=daniel@makrotopia$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=devicetree@vger$(echo .)kernel.org \
--cc=hkallweit1@gmail$(echo .)com \
--cc=kabel@kernel$(echo .)org \
--cc=krzysztof.kozlowski+dt@linaro$(echo .)org \
--cc=kuba@kernel$(echo .)org \
--cc=linux@armlinux$(echo .)org.uk \
--cc=netdev@vger$(echo .)kernel.org \
--cc=robh+dt@kernel$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox