Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

[Linus Walleij]

On Fri, May 24, 2013 at 12:40 AM, Sebastian Hesselbarth
<sebastian.hesselbarth@gmail.com> wrote:
> On 05/23/2013 08:40 PM, Jason Cooper wrote:

>> I think marvell,psc1_reset =<>; gives us the most flexibility in
>> accurately describing the hardware.
>
>
> IMHO using that is just another workaround for a broken driver. We
> could hack the whole register setup in DT as it would still accurately
> describe HW. Don't get me wrong, but I don't like it.
>
> Haven't checked how happy Linus Walleij is about pinctrl drivers with
> reg values hacked in lately.

One of the things I've been ranting about lately is that Linux
subsystem maintainers have become de-facto device tree
standard commite chairs. :-(

So to the actual question:

In general I think we need to draw a line and define what we
mean with "describing the hardware" in a device tree.

We have some consensus:
- <reg> properties to describe regsiter BASE offset in physical
  memory and size.
- Resources like IRQ, DMA channel, regulator, GPIO pin control
  handles, are passed using <&ampersand> notation.

And so it goes on.

When it comes to defining different registers and their individual
bits and the meaning of these and/or default values, I personally
think that is making things harder for developers rather than
simplifying things. I know that pinctrl-single is anyway doing this
and I was talked into accepting it under circumstances where
developers are being passed opaque machine-generated
data that would otherwise be translated into unreadable header
files littering the kernel.

For a coder it is definately better if the *driver* know these
details, but whether that is possible seems to depend on things
like hardware development process.

IMO: if you want to go down that road, what you really want is not
ever more expressible device trees, but real open firmware,
or ACPI or UEFI that can interpret and run bytecode as some
"bios" for you. With DT coming from OF maybe this is a natural
progression of things, but one has to realize when we reach the
point where what we really want is a bios. Then your time is
likely better spent with Tianocore or something than with the
kernel.

Yours,
Linus Walleij

Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

[Jason Cooper]

On Fri, May 24, 2013 at 01:03:25PM +0200, Linus Walleij wrote:
> On Fri, May 24, 2013 at 12:40 AM, Sebastian Hesselbarth
> <sebastian.hesselbarth@gmail.com> wrote:
> > On 05/23/2013 08:40 PM, Jason Cooper wrote:
> 
> >> I think marvell,psc1_reset =<>; gives us the most flexibility in
> >> accurately describing the hardware.
> >
> >
> > IMHO using that is just another workaround for a broken driver. We
> > could hack the whole register setup in DT as it would still accurately
> > describe HW. Don't get me wrong, but I don't like it.
> >
> > Haven't checked how happy Linus Walleij is about pinctrl drivers with
> > reg values hacked in lately.
> 
> One of the things I've been ranting about lately is that Linux
> subsystem maintainers have become de-facto device tree
> standard commite chairs. :-(

This is the first I've heard this rant.  :(

To that end, I agree with you.  My frustration boiled down to trying to
predict the future, which is futile. :-P

For our scenario, once we can confirm our least popular kirkwood
variant, the 6282, behaves the same as we've seen so far, then
"marvell,kirkwood-eth" is fine by me.

> So to the actual question:
> 
> In general I think we need to draw a line and define what we
> mean with "describing the hardware" in a device tree.
> 
> We have some consensus:
> - <reg> properties to describe regsiter BASE offset in physical
>   memory and size.
> - Resources like IRQ, DMA channel, regulator, GPIO pin control
>   handles, are passed using <&ampersand> notation.
> 
> And so it goes on.
> 
> When it comes to defining different registers and their individual
> bits and the meaning of these and/or default values, I personally
> think that is making things harder for developers rather than
> simplifying things. I know that pinctrl-single is anyway doing this
> and I was talked into accepting it under circumstances where
> developers are being passed opaque machine-generated
> data that would otherwise be translated into unreadable header
> files littering the kernel.
> 
> For a coder it is definately better if the *driver* know these
> details, but whether that is possible seems to depend on things
> like hardware development process.

Agree.

> IMO: if you want to go down that road, what you really want is not
> ever more expressible device trees, but real open firmware,
> or ACPI or UEFI that can interpret and run bytecode as some
> "bios" for you. With DT coming from OF maybe this is a natural
> progression of things, but one has to realize when we reach the
> point where what we really want is a bios. Then your time is
> likely better spent with Tianocore or something than with the
> kernel.

shudder.  I like embedded systems because the *don't* have a bios.

thx,

Jason.

Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

[Russell King - ARM Linux]

On Fri, May 24, 2013 at 01:01:25PM -0400, Jason Cooper wrote:
> On Fri, May 24, 2013 at 01:03:25PM +0200, Linus Walleij wrote:
> > IMO: if you want to go down that road, what you really want is not
> > ever more expressible device trees, but real open firmware,
> > or ACPI or UEFI that can interpret and run bytecode as some
> > "bios" for you. With DT coming from OF maybe this is a natural
> > progression of things, but one has to realize when we reach the
> > point where what we really want is a bios. Then your time is
> > likely better spent with Tianocore or something than with the
> > kernel.
> 
> shudder.  I like embedded systems because the *don't* have a bios.

Then you're into scenarios like I have with my laptop, where - those
of you who check the nightly build results will have noticed - one
of my serial ports doesn't always exist.  That's because the ACPI data
in the BIOS is *wrong*.  It reports that it has been enabled when it
hasn't, and the disassembled byte code is at fault here.

The fix?  God knows.  As far as I'm concerned as a user, or even as an
OS developer, it's unfixable without getting the ACPI data structures
changed, and that's not something I can do.

Do you really want that on ARM?  Given the fiasco with the location of
the registers, are you sure you want to place more trust in that
direction?  Does it give you a warm fuzzy feeling to know that you
might have to work out some way to patch vendor supplied bytecode?

Re: [PATCH 2/2] net: mv643xx_eth: proper initialization for Kirkwood SoCs

[Sebastian Hesselbarth]

On 05/24/2013 07:13 PM, Russell King - ARM Linux wrote:
> Do you really want that on ARM?  Given the fiasco with the location of
> the registers, are you sure you want to place more trust in that
> direction?  Does it give you a warm fuzzy feeling to know that you
> might have to work out some way to patch vendor supplied bytecode?

Don't get me wrong. I want mv643xx_eth DT or even platform_data to
evolve to a fully self configured driver not depending on proper u-boot
setup at all.

But I don't want to go that road now and I wonder if it might be safer
for us (and PPC guys) if we start mv643xx_eth over from scratch one day.

For this patch set, I want a basic DT binding that works. Patching the
driver to play with kirkwood loosing the MAC and other important
registers is not my main concern here. If clearing that one bit doesn't
help for all kirkwood boards, I'd rather leave the non-gating
workaround.

mv643xx_eth not knowing DT for ARM is stalling last important bits for
Orion SoCs. I want this to go in first as with David another maintainer
is involved.

Sebastian