Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-08-23 Thread Alexander Amelkin
20.07.2018 03:07, Andrew Jeffery wrote:
>> Andrew, can you start with a list that shows what you expect us to need
>> on our systems ?
>>
> Okay, our Witherspoon and Romulus platforms containing the ASPEED AST2500 
> currently need the following tuneables exposed:
>
> From the SCU:
> - Debug UART enable
> - VGA DAC mux
> - VGA scratch registers 0-7
> - LPC SuperIO decode enable
> - VGA MMIO decode enable
>
> From the LPC controller:
> - iLPC2AHB enable
> - SuperIO scratch registers 0x20-0x2f
>
> (The LPC controller is just as much of a collection of random bits as the SCU)
>
> Lastly, our Palmetto platform uses an AST2400 which has fewer features 
> compared to the AST2500. Its tuneable list is the same as the above with the 
> exception of "Debug UART enable".
>
> Tuneables that we may need to expose in the future include:
>
> From the SCU:
> - PCI VID/DID for the BMC PCIe device
> - VGA device enable (may need to be disabled if the platform contains a 
> discrete graphics processor)
>
> From the LPC controller:
> - UART mux
>
> Alexander, Eugene, can you chime in with your platforms' needs?
In addition to what you've mentioned, we also need (and I believe you need it 
too) the SCU3C register (reset control/status) to determine the BMC reset 
reason and only perform certain actions on cold boots (see 
https://github.com/openbmc/openbmc/issues/3031). Although, this could probably 
be handled by a separate driver, don't know which though.

Alexander.



signature.asc
Description: OpenPGP digital signature


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-08-23 Thread Alexander Amelkin
20.07.2018 03:07, Andrew Jeffery wrote:
>> Andrew, can you start with a list that shows what you expect us to need
>> on our systems ?
>>
> Okay, our Witherspoon and Romulus platforms containing the ASPEED AST2500 
> currently need the following tuneables exposed:
>
> From the SCU:
> - Debug UART enable
> - VGA DAC mux
> - VGA scratch registers 0-7
> - LPC SuperIO decode enable
> - VGA MMIO decode enable
>
> From the LPC controller:
> - iLPC2AHB enable
> - SuperIO scratch registers 0x20-0x2f
>
> (The LPC controller is just as much of a collection of random bits as the SCU)
>
> Lastly, our Palmetto platform uses an AST2400 which has fewer features 
> compared to the AST2500. Its tuneable list is the same as the above with the 
> exception of "Debug UART enable".
>
> Tuneables that we may need to expose in the future include:
>
> From the SCU:
> - PCI VID/DID for the BMC PCIe device
> - VGA device enable (may need to be disabled if the platform contains a 
> discrete graphics processor)
>
> From the LPC controller:
> - UART mux
>
> Alexander, Eugene, can you chime in with your platforms' needs?
In addition to what you've mentioned, we also need (and I believe you need it 
too) the SCU3C register (reset control/status) to determine the BMC reset 
reason and only perform certain actions on cold boots (see 
https://github.com/openbmc/openbmc/issues/3031). Although, this could probably 
be handled by a separate driver, don't know which though.

Alexander.



signature.asc
Description: OpenPGP digital signature


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-08-09 Thread Kun Yi
Andrew, Benjamin, Rob,

Thanks for bringing up the set of patches and a great discussion.
After going through the thread I figured that I'd like to share a few
things we needed to hack when programming several BMC boards:

- Debug UART enable/mux
- Disable GPIO D/E passthrough (I think this is supported by the
current pinctrl driver)
- RMII/RGMII strapping
- iLPC2AHB control
- SPI master mux select
- Various SuperIO configurations

As for the discussion whether these belong to a platform driver or
device tree nodes, I think in an ideal world all these configurations
could be nicely grouped and abstracted in a platform kernel driver (or
drivers). However in reality think this as an "M * N" problem: there
are M variants of BMCs and N different platforms built with these
BMCs. Each platform-BMC combination is going to have its own quirks
and slightly different requirements in BMC "tunables". Were there a
kernel driver for the M BMC variants, it would inevitably have a lot
of churn due to the different needs of the platforms.

What I like about the device tree approach is the expressiveness of
the format and the ability to specify non-conflicting initial values
easily. Sometimes we need initial values for these parameters set
before running userspace, and setting such values in device tree is
easier than using #defines or kernel parameters.




On Thu, Jul 19, 2018 at 9:57 PM Benjamin Herrenschmidt
 wrote:
>
> On Fri, 2018-07-20 at 09:37 +0930, Andrew Jeffery wrote:
> > >
> > > Andrew, can you start with a list that shows what you expect us to need
> > > on our systems ?
> > >
> >
> > Okay, our Witherspoon and Romulus platforms containing the ASPEED AST2500 
> > currently need the following tuneables exposed:
> >
> > > From the SCU:
> >
> > - Debug UART enable
> > - VGA DAC mux
> > - VGA scratch registers 0-7
> > - LPC SuperIO decode enable
> > - VGA MMIO decode enable
> >
> > > From the LPC controller:
> >
> > - iLPC2AHB enable
> > - SuperIO scratch registers 0x20-0x2f
> >
> > (The LPC controller is just as much of a collection of random bits as the 
> > SCU)
> >
> > Lastly, our Palmetto platform uses an AST2400 which has fewer features 
> > compared to the AST2500. Its tuneable list is the same as the above with 
> > the exception of "Debug UART enable".
> >
> > Tuneables that we may need to expose in the future include:
> >
> > > From the SCU:
> >
> > - PCI VID/DID for the BMC PCIe device
> > - VGA device enable (may need to be disabled if the platform contains a 
> > discrete graphics processor)
>
> Additionally there's a bunch of resigters controlling the mapping of
> various MMIO regions of the BMC PCIe device to portions of the BMC
> address space. I'm not sure what's the best way to handle that.
>
> This specific set might require a dedicated device as a subnode of
> the SCU in the DT that contains all the mappings as properties...
>
> That or we consider them static enough and just whack it in u-boot.
>
> > > From the LPC controller:
> >
> > - UART mux
> >
> > Alexander, Eugene, can you chime in with your platforms' needs?
> >
> > Cheers,
> >
> > Andrew



--
Regards,
Kun


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-08-09 Thread Kun Yi
Andrew, Benjamin, Rob,

Thanks for bringing up the set of patches and a great discussion.
After going through the thread I figured that I'd like to share a few
things we needed to hack when programming several BMC boards:

- Debug UART enable/mux
- Disable GPIO D/E passthrough (I think this is supported by the
current pinctrl driver)
- RMII/RGMII strapping
- iLPC2AHB control
- SPI master mux select
- Various SuperIO configurations

As for the discussion whether these belong to a platform driver or
device tree nodes, I think in an ideal world all these configurations
could be nicely grouped and abstracted in a platform kernel driver (or
drivers). However in reality think this as an "M * N" problem: there
are M variants of BMCs and N different platforms built with these
BMCs. Each platform-BMC combination is going to have its own quirks
and slightly different requirements in BMC "tunables". Were there a
kernel driver for the M BMC variants, it would inevitably have a lot
of churn due to the different needs of the platforms.

What I like about the device tree approach is the expressiveness of
the format and the ability to specify non-conflicting initial values
easily. Sometimes we need initial values for these parameters set
before running userspace, and setting such values in device tree is
easier than using #defines or kernel parameters.




On Thu, Jul 19, 2018 at 9:57 PM Benjamin Herrenschmidt
 wrote:
>
> On Fri, 2018-07-20 at 09:37 +0930, Andrew Jeffery wrote:
> > >
> > > Andrew, can you start with a list that shows what you expect us to need
> > > on our systems ?
> > >
> >
> > Okay, our Witherspoon and Romulus platforms containing the ASPEED AST2500 
> > currently need the following tuneables exposed:
> >
> > > From the SCU:
> >
> > - Debug UART enable
> > - VGA DAC mux
> > - VGA scratch registers 0-7
> > - LPC SuperIO decode enable
> > - VGA MMIO decode enable
> >
> > > From the LPC controller:
> >
> > - iLPC2AHB enable
> > - SuperIO scratch registers 0x20-0x2f
> >
> > (The LPC controller is just as much of a collection of random bits as the 
> > SCU)
> >
> > Lastly, our Palmetto platform uses an AST2400 which has fewer features 
> > compared to the AST2500. Its tuneable list is the same as the above with 
> > the exception of "Debug UART enable".
> >
> > Tuneables that we may need to expose in the future include:
> >
> > > From the SCU:
> >
> > - PCI VID/DID for the BMC PCIe device
> > - VGA device enable (may need to be disabled if the platform contains a 
> > discrete graphics processor)
>
> Additionally there's a bunch of resigters controlling the mapping of
> various MMIO regions of the BMC PCIe device to portions of the BMC
> address space. I'm not sure what's the best way to handle that.
>
> This specific set might require a dedicated device as a subnode of
> the SCU in the DT that contains all the mappings as properties...
>
> That or we consider them static enough and just whack it in u-boot.
>
> > > From the LPC controller:
> >
> > - UART mux
> >
> > Alexander, Eugene, can you chime in with your platforms' needs?
> >
> > Cheers,
> >
> > Andrew



--
Regards,
Kun


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-19 Thread Benjamin Herrenschmidt
On Fri, 2018-07-20 at 09:37 +0930, Andrew Jeffery wrote:
> > 
> > Andrew, can you start with a list that shows what you expect us to need
> > on our systems ?
> > 
> 
> Okay, our Witherspoon and Romulus platforms containing the ASPEED AST2500 
> currently need the following tuneables exposed:
> 
> > From the SCU:
> 
> - Debug UART enable
> - VGA DAC mux
> - VGA scratch registers 0-7
> - LPC SuperIO decode enable
> - VGA MMIO decode enable
> 
> > From the LPC controller:
> 
> - iLPC2AHB enable
> - SuperIO scratch registers 0x20-0x2f
> 
> (The LPC controller is just as much of a collection of random bits as the SCU)
> 
> Lastly, our Palmetto platform uses an AST2400 which has fewer features 
> compared to the AST2500. Its tuneable list is the same as the above with the 
> exception of "Debug UART enable".
> 
> Tuneables that we may need to expose in the future include:
> 
> > From the SCU:
> 
> - PCI VID/DID for the BMC PCIe device
> - VGA device enable (may need to be disabled if the platform contains a 
> discrete graphics processor)

Additionally there's a bunch of resigters controlling the mapping of
various MMIO regions of the BMC PCIe device to portions of the BMC
address space. I'm not sure what's the best way to handle that.

This specific set might require a dedicated device as a subnode of
the SCU in the DT that contains all the mappings as properties... 

That or we consider them static enough and just whack it in u-boot.

> > From the LPC controller:
> 
> - UART mux
> 
> Alexander, Eugene, can you chime in with your platforms' needs?
> 
> Cheers,
> 
> Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-19 Thread Benjamin Herrenschmidt
On Fri, 2018-07-20 at 09:37 +0930, Andrew Jeffery wrote:
> > 
> > Andrew, can you start with a list that shows what you expect us to need
> > on our systems ?
> > 
> 
> Okay, our Witherspoon and Romulus platforms containing the ASPEED AST2500 
> currently need the following tuneables exposed:
> 
> > From the SCU:
> 
> - Debug UART enable
> - VGA DAC mux
> - VGA scratch registers 0-7
> - LPC SuperIO decode enable
> - VGA MMIO decode enable
> 
> > From the LPC controller:
> 
> - iLPC2AHB enable
> - SuperIO scratch registers 0x20-0x2f
> 
> (The LPC controller is just as much of a collection of random bits as the SCU)
> 
> Lastly, our Palmetto platform uses an AST2400 which has fewer features 
> compared to the AST2500. Its tuneable list is the same as the above with the 
> exception of "Debug UART enable".
> 
> Tuneables that we may need to expose in the future include:
> 
> > From the SCU:
> 
> - PCI VID/DID for the BMC PCIe device
> - VGA device enable (may need to be disabled if the platform contains a 
> discrete graphics processor)

Additionally there's a bunch of resigters controlling the mapping of
various MMIO regions of the BMC PCIe device to portions of the BMC
address space. I'm not sure what's the best way to handle that.

This specific set might require a dedicated device as a subnode of
the SCU in the DT that contains all the mappings as properties... 

That or we consider them static enough and just whack it in u-boot.

> > From the LPC controller:
> 
> - UART mux
> 
> Alexander, Eugene, can you chime in with your platforms' needs?
> 
> Cheers,
> 
> Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-19 Thread Andrew Jeffery
> 
> Andrew, can you start with a list that shows what you expect us to need
> on our systems ?
> 

Okay, our Witherspoon and Romulus platforms containing the ASPEED AST2500 
currently need the following tuneables exposed:

>From the SCU:
- Debug UART enable
- VGA DAC mux
- VGA scratch registers 0-7
- LPC SuperIO decode enable
- VGA MMIO decode enable

>From the LPC controller:
- iLPC2AHB enable
- SuperIO scratch registers 0x20-0x2f

(The LPC controller is just as much of a collection of random bits as the SCU)

Lastly, our Palmetto platform uses an AST2400 which has fewer features compared 
to the AST2500. Its tuneable list is the same as the above with the exception 
of "Debug UART enable".

Tuneables that we may need to expose in the future include:

>From the SCU:
- PCI VID/DID for the BMC PCIe device
- VGA device enable (may need to be disabled if the platform contains a 
discrete graphics processor)

>From the LPC controller:
- UART mux

Alexander, Eugene, can you chime in with your platforms' needs?

Cheers,

Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-19 Thread Andrew Jeffery
> 
> Andrew, can you start with a list that shows what you expect us to need
> on our systems ?
> 

Okay, our Witherspoon and Romulus platforms containing the ASPEED AST2500 
currently need the following tuneables exposed:

>From the SCU:
- Debug UART enable
- VGA DAC mux
- VGA scratch registers 0-7
- LPC SuperIO decode enable
- VGA MMIO decode enable

>From the LPC controller:
- iLPC2AHB enable
- SuperIO scratch registers 0x20-0x2f

(The LPC controller is just as much of a collection of random bits as the SCU)

Lastly, our Palmetto platform uses an AST2400 which has fewer features compared 
to the AST2500. Its tuneable list is the same as the above with the exception 
of "Debug UART enable".

Tuneables that we may need to expose in the future include:

>From the SCU:
- PCI VID/DID for the BMC PCIe device
- VGA device enable (may need to be disabled if the platform contains a 
discrete graphics processor)

>From the LPC controller:
- UART mux

Alexander, Eugene, can you chime in with your platforms' needs?

Cheers,

Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Benjamin Herrenschmidt
On Thu, 2018-07-19 at 11:58 +0930, Andrew Jeffery wrote:
> > > I agree
> > > that not using /dev/mem is a good thing, but there are several ways
> > > you could do that independent of any DT binding.
> > 
> > Such as ? The only other option is to have one or more kernel drivers
> > that have built-in the precise and complete list of those tunables that
> > need to be exposed.
> > 
> > It's not impossible, but I worry that it's not going to scale terribly
> > well, and that vendors will constantly "fork" that driver to add
> > different things to that list.
> 
> Picking on that last point, "different things" doesn't necessarily
> mean "different fields in the SoC" (though it may). We could just
> need to use different initial values for the fields already
> described. 

I don't worry about initial values too much. Those could be specified
by userspace. It would be trivial to have something akin to
/etc/sysctl.conf that contains the initial values and a script blasting
them early. In fact I prefer this being in userspace to be frank. It
could be in the initramfs if we want it done early enough, maybe with a
usermode helper.

Unless we have some demonstrable reasons why some of these must be set
before the various kernel drivers initialize.

> So taking that into account, the field descriptions could vary wildly
> from platform to platform - where "platform" here is the combination
> of the BMC, its host system, and the host system's required
> configuration - not just referring to the BMC in isolation.  That in
> turn may cause a fork of the driver (changes that are incompatible
> with other platforms), or not scale terribly well as Ben suggests.

I really only worry about the actual set of registers/fields. I think
folding in initial values goes a bit too far.

> The initial value concept can help reduce the impact on userspace as
> userpace may no-longer need to care about it, so I think it's a
> desirable property.

I don't worry too much about userspace containing system specific bits
and pieces such as a config file with the appropriate initial values
for the platform. Userspace will have to contain platform specific
stuff regardless (if anything, stuff like thermal control is
intrinsicly different from one platform to the next).

>  With respect to devicetree, the field definitions will stay in the
> SoC dtsi, but the initial values would be described on a per-platform 
> basis in the dts.

If the fields are in the SoC dtsi, then Rob has a reasonable argument
that the list of fields is rather fixed for a given SoC and thus could
be hard wired in the driver 

I think at this stage, we need to create more clarity. In order to do
that, I think we need to come up with a list, as exhaustive as we can,
of what are the fields/register we need for our platforms, and find any
reason why userspace wouldn't be a good enough location to stick the
initial values.

Then we need Dell and Yadro (and maybe others) to help with their
variant of that list for the same SoCs to begin with.

With that, we'll get an idea of whether we think we can get a
reasonable stable long term list that's appropriate for most vendors.

If that's the case, then my objections to have it in the kernel instead
of the DT fade away a bit.

If one or two of those fields absolutely must have their defaults
early, then we can consider a specific set of one or two properties in
the SoC node for that SoC family that provides those specific defaults.

But unless we have that list, I think we'll be throwing too many
hypotheticals around to convince Rob and others.

Andrew, can you start with a list that shows what you expect us to need
on our systems ?

Cheers,
Ben



Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Benjamin Herrenschmidt
On Thu, 2018-07-19 at 11:58 +0930, Andrew Jeffery wrote:
> > > I agree
> > > that not using /dev/mem is a good thing, but there are several ways
> > > you could do that independent of any DT binding.
> > 
> > Such as ? The only other option is to have one or more kernel drivers
> > that have built-in the precise and complete list of those tunables that
> > need to be exposed.
> > 
> > It's not impossible, but I worry that it's not going to scale terribly
> > well, and that vendors will constantly "fork" that driver to add
> > different things to that list.
> 
> Picking on that last point, "different things" doesn't necessarily
> mean "different fields in the SoC" (though it may). We could just
> need to use different initial values for the fields already
> described. 

I don't worry about initial values too much. Those could be specified
by userspace. It would be trivial to have something akin to
/etc/sysctl.conf that contains the initial values and a script blasting
them early. In fact I prefer this being in userspace to be frank. It
could be in the initramfs if we want it done early enough, maybe with a
usermode helper.

Unless we have some demonstrable reasons why some of these must be set
before the various kernel drivers initialize.

> So taking that into account, the field descriptions could vary wildly
> from platform to platform - where "platform" here is the combination
> of the BMC, its host system, and the host system's required
> configuration - not just referring to the BMC in isolation.  That in
> turn may cause a fork of the driver (changes that are incompatible
> with other platforms), or not scale terribly well as Ben suggests.

I really only worry about the actual set of registers/fields. I think
folding in initial values goes a bit too far.

> The initial value concept can help reduce the impact on userspace as
> userpace may no-longer need to care about it, so I think it's a
> desirable property.

I don't worry too much about userspace containing system specific bits
and pieces such as a config file with the appropriate initial values
for the platform. Userspace will have to contain platform specific
stuff regardless (if anything, stuff like thermal control is
intrinsicly different from one platform to the next).

>  With respect to devicetree, the field definitions will stay in the
> SoC dtsi, but the initial values would be described on a per-platform 
> basis in the dts.

If the fields are in the SoC dtsi, then Rob has a reasonable argument
that the list of fields is rather fixed for a given SoC and thus could
be hard wired in the driver 

I think at this stage, we need to create more clarity. In order to do
that, I think we need to come up with a list, as exhaustive as we can,
of what are the fields/register we need for our platforms, and find any
reason why userspace wouldn't be a good enough location to stick the
initial values.

Then we need Dell and Yadro (and maybe others) to help with their
variant of that list for the same SoCs to begin with.

With that, we'll get an idea of whether we think we can get a
reasonable stable long term list that's appropriate for most vendors.

If that's the case, then my objections to have it in the kernel instead
of the DT fade away a bit.

If one or two of those fields absolutely must have their defaults
early, then we can consider a specific set of one or two properties in
the SoC node for that SoC family that provides those specific defaults.

But unless we have that list, I think we'll be throwing too many
hypotheticals around to convince Rob and others.

Andrew, can you start with a list that shows what you expect us to need
on our systems ?

Cheers,
Ben



Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Andrew Jeffery
> > I agree
> > that not using /dev/mem is a good thing, but there are several ways
> > you could do that independent of any DT binding.
> 
> Such as ? The only other option is to have one or more kernel drivers
> that have built-in the precise and complete list of those tunables that
> need to be exposed.
> 
> It's not impossible, but I worry that it's not going to scale terribly
> well, and that vendors will constantly "fork" that driver to add
> different things to that list.

Picking on that last point, "different things" doesn't necessarily mean 
"different fields in the SoC" (though it may). We could just need to use 
different initial values for the fields already described. So taking that into 
account, the field descriptions could vary wildly from platform to platform - 
where "platform" here is the combination of the BMC, its host system, and the 
host system's required configuration - not just referring to the BMC in 
isolation.  That in turn may cause a fork of the driver (changes that are 
incompatible with other platforms), or not scale terribly well as Ben suggests.

The initial value concept can help reduce the impact on userspace as userpace 
may no-longer need to care about it, so I think it's a desirable property. With 
respect to devicetree, the field definitions will stay in the SoC dtsi, but the 
initial values would be described on a per-platform basis in the dts.

> 
> I might be wrong here, so I'd like for Eugene (Dell) and Alexander
> (Yadro) to chime in, but experience with BMCs has shown that we
> regularily , as we add a feature or rewrite something, need to find
> another new magic SoC tunable the HW manufacturer hid somewhere that
> will make our stuff work.
> 
> > > Now, updating the device-tree in the board flash with whatever vendor
> > > specific information is needed is a LOT easier than getting the kernel
> > > driver constantly updated. The device-tree after all is there to
> > > reflect among other things system specific ways in which the SoC is
> > > wired and configured. This is rather close...
> > 
> > Sadly, updating my kernel is easier than updating my PC firmware
> > (though packaged firmware on my current laptop changes that). I can
> > assure you that ARM boards are generally much worse in that regard.
> > BTW, you may want to pay attention to EBBR[1][2]. Not sure how much
> > you care for BMCs, but there may be some interest.
> 
> You are conflating your host kernel and your BMC here. The BMC kernel
> is part of the "firmware", as is its DTS and the BMC userspace.
> 
> (Again this isn't the host DTS, this is the BMC DTS).
> 
> They get all updated together. My point isn't about the ease or
> difficulty for a *user* to udpate their BMC, in that case the solutions
> above are equivalent.
> 
> The point is from a system vendor perspective. A system vendor using
> OpenBMC will *customize* their BMC build in various ways. Typically
> they *will* have a custom DT since this is what represents their
> specific system and they will have some specific userspace bits.
> 
> However, we are trying very hard for them *not* to fork the kernel, and
> if possible move OpenBMC towards a fully upstream kernel (still working
> on getting all the SoC drivers cleaned up and pushed up but it's moving
> in the right direction).
> 
> So from a vendor perspective, such as us IBM, we *alread* have custom
> DTs and customized userspace, that's part of the normal flow of
> deploying a BMC.
> 
> However, we are trying *NOT* to have a custom kernel (and we don't, at
> the moment there is an OpenBMC kernel accross vendors, though it's not
> *yet* fully upstream).
> 
> So if the solution proposed is prone to requiring frequent changes to a
> kernel driver, that solution makes the above a lot more difficult and
> will encourage vendors to keep forking kernels.
> 
> This is what we are trying to avoid.

Well described. Thanks Ben.

Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Andrew Jeffery
> > I agree
> > that not using /dev/mem is a good thing, but there are several ways
> > you could do that independent of any DT binding.
> 
> Such as ? The only other option is to have one or more kernel drivers
> that have built-in the precise and complete list of those tunables that
> need to be exposed.
> 
> It's not impossible, but I worry that it's not going to scale terribly
> well, and that vendors will constantly "fork" that driver to add
> different things to that list.

Picking on that last point, "different things" doesn't necessarily mean 
"different fields in the SoC" (though it may). We could just need to use 
different initial values for the fields already described. So taking that into 
account, the field descriptions could vary wildly from platform to platform - 
where "platform" here is the combination of the BMC, its host system, and the 
host system's required configuration - not just referring to the BMC in 
isolation.  That in turn may cause a fork of the driver (changes that are 
incompatible with other platforms), or not scale terribly well as Ben suggests.

The initial value concept can help reduce the impact on userspace as userpace 
may no-longer need to care about it, so I think it's a desirable property. With 
respect to devicetree, the field definitions will stay in the SoC dtsi, but the 
initial values would be described on a per-platform basis in the dts.

> 
> I might be wrong here, so I'd like for Eugene (Dell) and Alexander
> (Yadro) to chime in, but experience with BMCs has shown that we
> regularily , as we add a feature or rewrite something, need to find
> another new magic SoC tunable the HW manufacturer hid somewhere that
> will make our stuff work.
> 
> > > Now, updating the device-tree in the board flash with whatever vendor
> > > specific information is needed is a LOT easier than getting the kernel
> > > driver constantly updated. The device-tree after all is there to
> > > reflect among other things system specific ways in which the SoC is
> > > wired and configured. This is rather close...
> > 
> > Sadly, updating my kernel is easier than updating my PC firmware
> > (though packaged firmware on my current laptop changes that). I can
> > assure you that ARM boards are generally much worse in that regard.
> > BTW, you may want to pay attention to EBBR[1][2]. Not sure how much
> > you care for BMCs, but there may be some interest.
> 
> You are conflating your host kernel and your BMC here. The BMC kernel
> is part of the "firmware", as is its DTS and the BMC userspace.
> 
> (Again this isn't the host DTS, this is the BMC DTS).
> 
> They get all updated together. My point isn't about the ease or
> difficulty for a *user* to udpate their BMC, in that case the solutions
> above are equivalent.
> 
> The point is from a system vendor perspective. A system vendor using
> OpenBMC will *customize* their BMC build in various ways. Typically
> they *will* have a custom DT since this is what represents their
> specific system and they will have some specific userspace bits.
> 
> However, we are trying very hard for them *not* to fork the kernel, and
> if possible move OpenBMC towards a fully upstream kernel (still working
> on getting all the SoC drivers cleaned up and pushed up but it's moving
> in the right direction).
> 
> So from a vendor perspective, such as us IBM, we *alread* have custom
> DTs and customized userspace, that's part of the normal flow of
> deploying a BMC.
> 
> However, we are trying *NOT* to have a custom kernel (and we don't, at
> the moment there is an OpenBMC kernel accross vendors, though it's not
> *yet* fully upstream).
> 
> So if the solution proposed is prone to requiring frequent changes to a
> kernel driver, that solution makes the above a lot more difficult and
> will encourage vendors to keep forking kernels.
> 
> This is what we are trying to avoid.

Well described. Thanks Ben.

Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Andrew Jeffery
On Thu, 19 Jul 2018, at 04:37, Rob Herring wrote:
> On Tue, Jul 17, 2018 at 5:28 PM Andrew Jeffery  wrote:
> >
> > On Tue, 17 Jul 2018, at 14:26, Benjamin Herrenschmidt wrote:
> > > On Mon, 2018-07-16 at 07:55 -0600, Rob Herring wrote:
> > > > If that data is one set per SoC, then i'm not that concerned having
> > > > platform-specific data in the driver. That doesn't mean the driver is
> > > > not "generic". It's still not clear to me in this thread, how much of
> > > > this is board specific, but given that you've placed all the data in
> > > > an SoC dtsi file it seems to be all per SoC.
> > >
> > > So Rob, I think that's precisely where the disconnect is.
> > >
> > > I think we all (well hopefully) agree that those few tunables don't fit
> > > in any existing subystem and aren't likely to ever do (famous last
> > > words...).
> > >
> > > Where we disagree is we want to make this parametrized via the DT, and
> > > you want us to hard wire the list in some kind of SoC driver for a
> > > given SoC family/version.
> > >
> > > The reason I think hard wiring the list in the driver is not a great
> > > solution is that that list in itself is prone to variations, possibly
> > > fairly often, between boards, vendors, versions of boards, etc...
> > >
> > > We can't know for sure every SoC tunable (out of the gazillions in
> > > those chips) are going to be needed for a given system. We know which
> > > ones we do use for ours, and that's a couple of handfuls, but it could
> > > be that Dell need a slightly different set, and so might Yadro, or so
> > > might our next board revision for that matter.
> > >
> > > Now, updating the device-tree in the board flash with whatever vendor
> > > specific information is needed is a LOT easier than getting the kernel
> > > driver constantly updated. The device-tree after all is there to
> > > reflect among other things system specific ways in which the SoC is
> > > wired and configured. This is rather close...
> >
> > Not sure this helps, but I feel that the proposal pretty closely matches 
> > what's described in Documentation/devicetree/bindings/mfd/mfd.txt. It's 
> > intended that nodes using the bindings I'm proposing are children of a 
> > 'compatible = "syscon", "simple-mfd"' node (this is the case with the 
> > features we're hoping to describe for our SoC). I should explicitly call 
> > that out.
> 
> IMO, any binding that has only those compatibles is not correct and a
> specific compatible is needed. We should be able identify a specific
> h/w block.

I didn't intend for that to get interpreted quite as literally, so apologies 
for that. We do have h/w-block-specific compatibles in there too. The point was 
to demonstrate that we're dealing (at this point, only) with mfds/syscons.

> 
> > But to go on, "simple-mfd" is effectively an alias of "simple-bus", which 
> > means its intended to match child node compatibles to drivers provided by 
> > the kernel. If we shouldn't be describing minor features of a SoC in the 
> > devicetree, doesn't this invalidate the case for simple-mfd? What is the 
> > *correct* use of simple-mfd? When is it not used to expose minor features 
> > in set of "miscellaneous system registers"? Why doesn't this proposed case 
> > fit?
> 
> I'm no fan of simple-mfd either. I think it is abused and often a sign
> of bad binding design.

Ah, yes, this is a familiar feeling when reflecting on things I've done in the 
past. Hence trying to understand how to use it right.

> The general problem with MFD bindings is people
> define child nodes based on what drivers they happen to have for some
> OS. DT is not the only way to instantiate drivers. Child nodes are
> really only needed when you have resources per child that need to be
> defined. For example, if the MFD has an interrupt controller and
> interrupts to sub-blocks or sub-blocks have their own clocks.
> "simple-mfd" was for when the parent node has no driver or the child
> nodes have no dependency on the parent.

Thanks for the clarification, I'll keep that in mind going forward.

Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Andrew Jeffery
On Thu, 19 Jul 2018, at 04:37, Rob Herring wrote:
> On Tue, Jul 17, 2018 at 5:28 PM Andrew Jeffery  wrote:
> >
> > On Tue, 17 Jul 2018, at 14:26, Benjamin Herrenschmidt wrote:
> > > On Mon, 2018-07-16 at 07:55 -0600, Rob Herring wrote:
> > > > If that data is one set per SoC, then i'm not that concerned having
> > > > platform-specific data in the driver. That doesn't mean the driver is
> > > > not "generic". It's still not clear to me in this thread, how much of
> > > > this is board specific, but given that you've placed all the data in
> > > > an SoC dtsi file it seems to be all per SoC.
> > >
> > > So Rob, I think that's precisely where the disconnect is.
> > >
> > > I think we all (well hopefully) agree that those few tunables don't fit
> > > in any existing subystem and aren't likely to ever do (famous last
> > > words...).
> > >
> > > Where we disagree is we want to make this parametrized via the DT, and
> > > you want us to hard wire the list in some kind of SoC driver for a
> > > given SoC family/version.
> > >
> > > The reason I think hard wiring the list in the driver is not a great
> > > solution is that that list in itself is prone to variations, possibly
> > > fairly often, between boards, vendors, versions of boards, etc...
> > >
> > > We can't know for sure every SoC tunable (out of the gazillions in
> > > those chips) are going to be needed for a given system. We know which
> > > ones we do use for ours, and that's a couple of handfuls, but it could
> > > be that Dell need a slightly different set, and so might Yadro, or so
> > > might our next board revision for that matter.
> > >
> > > Now, updating the device-tree in the board flash with whatever vendor
> > > specific information is needed is a LOT easier than getting the kernel
> > > driver constantly updated. The device-tree after all is there to
> > > reflect among other things system specific ways in which the SoC is
> > > wired and configured. This is rather close...
> >
> > Not sure this helps, but I feel that the proposal pretty closely matches 
> > what's described in Documentation/devicetree/bindings/mfd/mfd.txt. It's 
> > intended that nodes using the bindings I'm proposing are children of a 
> > 'compatible = "syscon", "simple-mfd"' node (this is the case with the 
> > features we're hoping to describe for our SoC). I should explicitly call 
> > that out.
> 
> IMO, any binding that has only those compatibles is not correct and a
> specific compatible is needed. We should be able identify a specific
> h/w block.

I didn't intend for that to get interpreted quite as literally, so apologies 
for that. We do have h/w-block-specific compatibles in there too. The point was 
to demonstrate that we're dealing (at this point, only) with mfds/syscons.

> 
> > But to go on, "simple-mfd" is effectively an alias of "simple-bus", which 
> > means its intended to match child node compatibles to drivers provided by 
> > the kernel. If we shouldn't be describing minor features of a SoC in the 
> > devicetree, doesn't this invalidate the case for simple-mfd? What is the 
> > *correct* use of simple-mfd? When is it not used to expose minor features 
> > in set of "miscellaneous system registers"? Why doesn't this proposed case 
> > fit?
> 
> I'm no fan of simple-mfd either. I think it is abused and often a sign
> of bad binding design.

Ah, yes, this is a familiar feeling when reflecting on things I've done in the 
past. Hence trying to understand how to use it right.

> The general problem with MFD bindings is people
> define child nodes based on what drivers they happen to have for some
> OS. DT is not the only way to instantiate drivers. Child nodes are
> really only needed when you have resources per child that need to be
> defined. For example, if the MFD has an interrupt controller and
> interrupts to sub-blocks or sub-blocks have their own clocks.
> "simple-mfd" was for when the parent node has no driver or the child
> nodes have no dependency on the parent.

Thanks for the clarification, I'll keep that in mind going forward.

Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Benjamin Herrenschmidt
On Wed, 2018-07-18 at 13:50 -0600, Rob Herring wrote:
> 
> > So Rob, I think that's precisely where the disconnect is.
> > 
> > I think we all (well hopefully) agree that those few tunables don't fit
> > in any existing subystem and aren't likely to ever do (famous last
> > words...).
> > 
> > Where we disagree is we want to make this parametrized via the DT, and
> > you want us to hard wire the list in some kind of SoC driver for a
> > given SoC family/version.
> > 
> > The reason I think hard wiring the list in the driver is not a great
> > solution is that that list in itself is prone to variations, possibly
> > fairly often, between boards, vendors, versions of boards, etc...
> 
> Can we separate the list of what's enabled from the details of the
> registers?  Even if we put all this into DT, we may still want to have
> some separation for dts file structure. Some portion of this has to be
> SoC specific because you are simply exposing SoC registers.

Not sure I understand what you mean by "what's enabled".

The goal here is to expose register "fields" (nor raw values, some
registers need locking for RMW etc... the kernel would handle that via
syscon locking I expect), so basically named "tunables" to userspace.

Userspace is the one that knows the values for a given system.

> > We can't know for sure every SoC tunable (out of the gazillions in
> > those chips) are going to be needed for a given system. We know which
> > ones we do use for ours, and that's a couple of handfuls, but it could
> > be that Dell need a slightly different set, and so might Yadro, or so
> > might our next board revision for that matter.
> 
> That's very hand wavy. Do you have some concrete examples (i.e. dts
> files) showing the differences.

We could list what we have on the pile for some of our IBM systems
today, but we would need Dell and Yadro to chime in with what they
need.

I still, from experience with that stuff and gut feeling, am pretty
sure it's going to be a moving list, and updating the kernel driver
constantly isn't going to fly.

> One problem I'm having with this is you are still going to need per
> board specifics in userspace.

Yes. Userspace is ultimately the one that knows what needs to be done
on a given machine.

>  You may be moving some of details out,
> but moving to DT is not going to completely eliminate that. 

We aren't trying to either. We are trying to make sure we don't need to
change the *kernel* all the time, in part bcs we are pushing hard for
OpenBMC vendors to use upstream with, if possible, no vendor changes.

So userspace has to know the board specific tunables anyways. Today in
many systems, it does that by whacking /dev/mem. I guess you can
understand why that is bad :-)

> I agree
> that not using /dev/mem is a good thing, but there are several ways
> you could do that independent of any DT binding.

Such as ? The only other option is to have one or more kernel drivers
that have built-in the precise and complete list of those tunables that
need to be exposed.

It's not impossible, but I worry that it's not going to scale terribly
well, and that vendors will constantly "fork" that driver to add
different things to that list.

I might be wrong here, so I'd like for Eugene (Dell) and Alexander
(Yadro) to chime in, but experience with BMCs has shown that we
regularily , as we add a feature or rewrite something, need to find
another new magic SoC tunable the HW manufacturer hid somewhere that
will make our stuff work.

> > Now, updating the device-tree in the board flash with whatever vendor
> > specific information is needed is a LOT easier than getting the kernel
> > driver constantly updated. The device-tree after all is there to
> > reflect among other things system specific ways in which the SoC is
> > wired and configured. This is rather close...
> 
> Sadly, updating my kernel is easier than updating my PC firmware
> (though packaged firmware on my current laptop changes that). I can
> assure you that ARM boards are generally much worse in that regard.
> BTW, you may want to pay attention to EBBR[1][2]. Not sure how much
> you care for BMCs, but there may be some interest.

You are conflating your host kernel and your BMC here. The BMC kernel
is part of the "firmware", as is its DTS and the BMC userspace.

(Again this isn't the host DTS, this is the BMC DTS).

They get all updated together. My point isn't about the ease or
difficulty for a *user* to udpate their BMC, in that case the solutions
above are equivalent.

The point is from a system vendor perspective. A system vendor using
OpenBMC will *customize* their BMC build in various ways. Typically
they *will* have a custom DT since this is what represents their
specific system and they will have some specific userspace bits.

However, we are trying very hard for them *not* to fork the kernel, and
if possible move OpenBMC towards a fully upstream kernel (still working
on getting all the SoC drivers cleaned up and pushed up but 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Benjamin Herrenschmidt
On Wed, 2018-07-18 at 13:50 -0600, Rob Herring wrote:
> 
> > So Rob, I think that's precisely where the disconnect is.
> > 
> > I think we all (well hopefully) agree that those few tunables don't fit
> > in any existing subystem and aren't likely to ever do (famous last
> > words...).
> > 
> > Where we disagree is we want to make this parametrized via the DT, and
> > you want us to hard wire the list in some kind of SoC driver for a
> > given SoC family/version.
> > 
> > The reason I think hard wiring the list in the driver is not a great
> > solution is that that list in itself is prone to variations, possibly
> > fairly often, between boards, vendors, versions of boards, etc...
> 
> Can we separate the list of what's enabled from the details of the
> registers?  Even if we put all this into DT, we may still want to have
> some separation for dts file structure. Some portion of this has to be
> SoC specific because you are simply exposing SoC registers.

Not sure I understand what you mean by "what's enabled".

The goal here is to expose register "fields" (nor raw values, some
registers need locking for RMW etc... the kernel would handle that via
syscon locking I expect), so basically named "tunables" to userspace.

Userspace is the one that knows the values for a given system.

> > We can't know for sure every SoC tunable (out of the gazillions in
> > those chips) are going to be needed for a given system. We know which
> > ones we do use for ours, and that's a couple of handfuls, but it could
> > be that Dell need a slightly different set, and so might Yadro, or so
> > might our next board revision for that matter.
> 
> That's very hand wavy. Do you have some concrete examples (i.e. dts
> files) showing the differences.

We could list what we have on the pile for some of our IBM systems
today, but we would need Dell and Yadro to chime in with what they
need.

I still, from experience with that stuff and gut feeling, am pretty
sure it's going to be a moving list, and updating the kernel driver
constantly isn't going to fly.

> One problem I'm having with this is you are still going to need per
> board specifics in userspace.

Yes. Userspace is ultimately the one that knows what needs to be done
on a given machine.

>  You may be moving some of details out,
> but moving to DT is not going to completely eliminate that. 

We aren't trying to either. We are trying to make sure we don't need to
change the *kernel* all the time, in part bcs we are pushing hard for
OpenBMC vendors to use upstream with, if possible, no vendor changes.

So userspace has to know the board specific tunables anyways. Today in
many systems, it does that by whacking /dev/mem. I guess you can
understand why that is bad :-)

> I agree
> that not using /dev/mem is a good thing, but there are several ways
> you could do that independent of any DT binding.

Such as ? The only other option is to have one or more kernel drivers
that have built-in the precise and complete list of those tunables that
need to be exposed.

It's not impossible, but I worry that it's not going to scale terribly
well, and that vendors will constantly "fork" that driver to add
different things to that list.

I might be wrong here, so I'd like for Eugene (Dell) and Alexander
(Yadro) to chime in, but experience with BMCs has shown that we
regularily , as we add a feature or rewrite something, need to find
another new magic SoC tunable the HW manufacturer hid somewhere that
will make our stuff work.

> > Now, updating the device-tree in the board flash with whatever vendor
> > specific information is needed is a LOT easier than getting the kernel
> > driver constantly updated. The device-tree after all is there to
> > reflect among other things system specific ways in which the SoC is
> > wired and configured. This is rather close...
> 
> Sadly, updating my kernel is easier than updating my PC firmware
> (though packaged firmware on my current laptop changes that). I can
> assure you that ARM boards are generally much worse in that regard.
> BTW, you may want to pay attention to EBBR[1][2]. Not sure how much
> you care for BMCs, but there may be some interest.

You are conflating your host kernel and your BMC here. The BMC kernel
is part of the "firmware", as is its DTS and the BMC userspace.

(Again this isn't the host DTS, this is the BMC DTS).

They get all updated together. My point isn't about the ease or
difficulty for a *user* to udpate their BMC, in that case the solutions
above are equivalent.

The point is from a system vendor perspective. A system vendor using
OpenBMC will *customize* their BMC build in various ways. Typically
they *will* have a custom DT since this is what represents their
specific system and they will have some specific userspace bits.

However, we are trying very hard for them *not* to fork the kernel, and
if possible move OpenBMC towards a fully upstream kernel (still working
on getting all the SoC drivers cleaned up and pushed up but 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Rob Herring
On Mon, Jul 16, 2018 at 10:56 PM Benjamin Herrenschmidt
 wrote:
>
> On Mon, 2018-07-16 at 07:55 -0600, Rob Herring wrote:
> > If that data is one set per SoC, then i'm not that concerned having
> > platform-specific data in the driver. That doesn't mean the driver is
> > not "generic". It's still not clear to me in this thread, how much of
> > this is board specific, but given that you've placed all the data in
> > an SoC dtsi file it seems to be all per SoC.
>
> So Rob, I think that's precisely where the disconnect is.
>
> I think we all (well hopefully) agree that those few tunables don't fit
> in any existing subystem and aren't likely to ever do (famous last
> words...).
>
> Where we disagree is we want to make this parametrized via the DT, and
> you want us to hard wire the list in some kind of SoC driver for a
> given SoC family/version.
>
> The reason I think hard wiring the list in the driver is not a great
> solution is that that list in itself is prone to variations, possibly
> fairly often, between boards, vendors, versions of boards, etc...

Can we separate the list of what's enabled from the details of the
registers?  Even if we put all this into DT, we may still want to have
some separation for dts file structure. Some portion of this has to be
SoC specific because you are simply exposing SoC registers.

> We can't know for sure every SoC tunable (out of the gazillions in
> those chips) are going to be needed for a given system. We know which
> ones we do use for ours, and that's a couple of handfuls, but it could
> be that Dell need a slightly different set, and so might Yadro, or so
> might our next board revision for that matter.

That's very hand wavy. Do you have some concrete examples (i.e. dts
files) showing the differences.

One problem I'm having with this is you are still going to need per
board specifics in userspace. You may be moving some of details out,
but moving to DT is not going to completely eliminate that. I agree
that not using /dev/mem is a good thing, but there are several ways
you could do that independent of any DT binding.

> Now, updating the device-tree in the board flash with whatever vendor
> specific information is needed is a LOT easier than getting the kernel
> driver constantly updated. The device-tree after all is there to
> reflect among other things system specific ways in which the SoC is
> wired and configured. This is rather close...

Sadly, updating my kernel is easier than updating my PC firmware
(though packaged firmware on my current laptop changes that). I can
assure you that ARM boards are generally much worse in that regard.
BTW, you may want to pay attention to EBBR[1][2]. Not sure how much
you care for BMCs, but there may be some interest.

Rob

[1] https://github.com/ARM-software/ebbr
[2] https://lists.linaro.org/pipermail/boot-architecture/


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Rob Herring
On Mon, Jul 16, 2018 at 10:56 PM Benjamin Herrenschmidt
 wrote:
>
> On Mon, 2018-07-16 at 07:55 -0600, Rob Herring wrote:
> > If that data is one set per SoC, then i'm not that concerned having
> > platform-specific data in the driver. That doesn't mean the driver is
> > not "generic". It's still not clear to me in this thread, how much of
> > this is board specific, but given that you've placed all the data in
> > an SoC dtsi file it seems to be all per SoC.
>
> So Rob, I think that's precisely where the disconnect is.
>
> I think we all (well hopefully) agree that those few tunables don't fit
> in any existing subystem and aren't likely to ever do (famous last
> words...).
>
> Where we disagree is we want to make this parametrized via the DT, and
> you want us to hard wire the list in some kind of SoC driver for a
> given SoC family/version.
>
> The reason I think hard wiring the list in the driver is not a great
> solution is that that list in itself is prone to variations, possibly
> fairly often, between boards, vendors, versions of boards, etc...

Can we separate the list of what's enabled from the details of the
registers?  Even if we put all this into DT, we may still want to have
some separation for dts file structure. Some portion of this has to be
SoC specific because you are simply exposing SoC registers.

> We can't know for sure every SoC tunable (out of the gazillions in
> those chips) are going to be needed for a given system. We know which
> ones we do use for ours, and that's a couple of handfuls, but it could
> be that Dell need a slightly different set, and so might Yadro, or so
> might our next board revision for that matter.

That's very hand wavy. Do you have some concrete examples (i.e. dts
files) showing the differences.

One problem I'm having with this is you are still going to need per
board specifics in userspace. You may be moving some of details out,
but moving to DT is not going to completely eliminate that. I agree
that not using /dev/mem is a good thing, but there are several ways
you could do that independent of any DT binding.

> Now, updating the device-tree in the board flash with whatever vendor
> specific information is needed is a LOT easier than getting the kernel
> driver constantly updated. The device-tree after all is there to
> reflect among other things system specific ways in which the SoC is
> wired and configured. This is rather close...

Sadly, updating my kernel is easier than updating my PC firmware
(though packaged firmware on my current laptop changes that). I can
assure you that ARM boards are generally much worse in that regard.
BTW, you may want to pay attention to EBBR[1][2]. Not sure how much
you care for BMCs, but there may be some interest.

Rob

[1] https://github.com/ARM-software/ebbr
[2] https://lists.linaro.org/pipermail/boot-architecture/


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Rob Herring
On Tue, Jul 17, 2018 at 5:28 PM Andrew Jeffery  wrote:
>
> On Tue, 17 Jul 2018, at 14:26, Benjamin Herrenschmidt wrote:
> > On Mon, 2018-07-16 at 07:55 -0600, Rob Herring wrote:
> > > If that data is one set per SoC, then i'm not that concerned having
> > > platform-specific data in the driver. That doesn't mean the driver is
> > > not "generic". It's still not clear to me in this thread, how much of
> > > this is board specific, but given that you've placed all the data in
> > > an SoC dtsi file it seems to be all per SoC.
> >
> > So Rob, I think that's precisely where the disconnect is.
> >
> > I think we all (well hopefully) agree that those few tunables don't fit
> > in any existing subystem and aren't likely to ever do (famous last
> > words...).
> >
> > Where we disagree is we want to make this parametrized via the DT, and
> > you want us to hard wire the list in some kind of SoC driver for a
> > given SoC family/version.
> >
> > The reason I think hard wiring the list in the driver is not a great
> > solution is that that list in itself is prone to variations, possibly
> > fairly often, between boards, vendors, versions of boards, etc...
> >
> > We can't know for sure every SoC tunable (out of the gazillions in
> > those chips) are going to be needed for a given system. We know which
> > ones we do use for ours, and that's a couple of handfuls, but it could
> > be that Dell need a slightly different set, and so might Yadro, or so
> > might our next board revision for that matter.
> >
> > Now, updating the device-tree in the board flash with whatever vendor
> > specific information is needed is a LOT easier than getting the kernel
> > driver constantly updated. The device-tree after all is there to
> > reflect among other things system specific ways in which the SoC is
> > wired and configured. This is rather close...
>
> Not sure this helps, but I feel that the proposal pretty closely matches 
> what's described in Documentation/devicetree/bindings/mfd/mfd.txt. It's 
> intended that nodes using the bindings I'm proposing are children of a 
> 'compatible = "syscon", "simple-mfd"' node (this is the case with the 
> features we're hoping to describe for our SoC). I should explicitly call that 
> out.

IMO, any binding that has only those compatibles is not correct and a
specific compatible is needed. We should be able identify a specific
h/w block.

> But to go on, "simple-mfd" is effectively an alias of "simple-bus", which 
> means its intended to match child node compatibles to drivers provided by the 
> kernel. If we shouldn't be describing minor features of a SoC in the 
> devicetree, doesn't this invalidate the case for simple-mfd? What is the 
> *correct* use of simple-mfd? When is it not used to expose minor features in 
> set of "miscellaneous system registers"? Why doesn't this proposed case fit?

I'm no fan of simple-mfd either. I think it is abused and often a sign
of bad binding design. The general problem with MFD bindings is people
define child nodes based on what drivers they happen to have for some
OS. DT is not the only way to instantiate drivers. Child nodes are
really only needed when you have resources per child that need to be
defined. For example, if the MFD has an interrupt controller and
interrupts to sub-blocks or sub-blocks have their own clocks.
"simple-mfd" was for when the parent node has no driver or the child
nodes have no dependency on the parent.

Rob


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-18 Thread Rob Herring
On Tue, Jul 17, 2018 at 5:28 PM Andrew Jeffery  wrote:
>
> On Tue, 17 Jul 2018, at 14:26, Benjamin Herrenschmidt wrote:
> > On Mon, 2018-07-16 at 07:55 -0600, Rob Herring wrote:
> > > If that data is one set per SoC, then i'm not that concerned having
> > > platform-specific data in the driver. That doesn't mean the driver is
> > > not "generic". It's still not clear to me in this thread, how much of
> > > this is board specific, but given that you've placed all the data in
> > > an SoC dtsi file it seems to be all per SoC.
> >
> > So Rob, I think that's precisely where the disconnect is.
> >
> > I think we all (well hopefully) agree that those few tunables don't fit
> > in any existing subystem and aren't likely to ever do (famous last
> > words...).
> >
> > Where we disagree is we want to make this parametrized via the DT, and
> > you want us to hard wire the list in some kind of SoC driver for a
> > given SoC family/version.
> >
> > The reason I think hard wiring the list in the driver is not a great
> > solution is that that list in itself is prone to variations, possibly
> > fairly often, between boards, vendors, versions of boards, etc...
> >
> > We can't know for sure every SoC tunable (out of the gazillions in
> > those chips) are going to be needed for a given system. We know which
> > ones we do use for ours, and that's a couple of handfuls, but it could
> > be that Dell need a slightly different set, and so might Yadro, or so
> > might our next board revision for that matter.
> >
> > Now, updating the device-tree in the board flash with whatever vendor
> > specific information is needed is a LOT easier than getting the kernel
> > driver constantly updated. The device-tree after all is there to
> > reflect among other things system specific ways in which the SoC is
> > wired and configured. This is rather close...
>
> Not sure this helps, but I feel that the proposal pretty closely matches 
> what's described in Documentation/devicetree/bindings/mfd/mfd.txt. It's 
> intended that nodes using the bindings I'm proposing are children of a 
> 'compatible = "syscon", "simple-mfd"' node (this is the case with the 
> features we're hoping to describe for our SoC). I should explicitly call that 
> out.

IMO, any binding that has only those compatibles is not correct and a
specific compatible is needed. We should be able identify a specific
h/w block.

> But to go on, "simple-mfd" is effectively an alias of "simple-bus", which 
> means its intended to match child node compatibles to drivers provided by the 
> kernel. If we shouldn't be describing minor features of a SoC in the 
> devicetree, doesn't this invalidate the case for simple-mfd? What is the 
> *correct* use of simple-mfd? When is it not used to expose minor features in 
> set of "miscellaneous system registers"? Why doesn't this proposed case fit?

I'm no fan of simple-mfd either. I think it is abused and often a sign
of bad binding design. The general problem with MFD bindings is people
define child nodes based on what drivers they happen to have for some
OS. DT is not the only way to instantiate drivers. Child nodes are
really only needed when you have resources per child that need to be
defined. For example, if the MFD has an interrupt controller and
interrupts to sub-blocks or sub-blocks have their own clocks.
"simple-mfd" was for when the parent node has no driver or the child
nodes have no dependency on the parent.

Rob


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-17 Thread Andrew Jeffery
On Tue, 17 Jul 2018, at 14:26, Benjamin Herrenschmidt wrote:
> On Mon, 2018-07-16 at 07:55 -0600, Rob Herring wrote:
> > If that data is one set per SoC, then i'm not that concerned having
> > platform-specific data in the driver. That doesn't mean the driver is
> > not "generic". It's still not clear to me in this thread, how much of
> > this is board specific, but given that you've placed all the data in
> > an SoC dtsi file it seems to be all per SoC.
> 
> So Rob, I think that's precisely where the disconnect is.
> 
> I think we all (well hopefully) agree that those few tunables don't fit
> in any existing subystem and aren't likely to ever do (famous last
> words...).
> 
> Where we disagree is we want to make this parametrized via the DT, and
> you want us to hard wire the list in some kind of SoC driver for a
> given SoC family/version.
> 
> The reason I think hard wiring the list in the driver is not a great
> solution is that that list in itself is prone to variations, possibly
> fairly often, between boards, vendors, versions of boards, etc...
> 
> We can't know for sure every SoC tunable (out of the gazillions in
> those chips) are going to be needed for a given system. We know which
> ones we do use for ours, and that's a couple of handfuls, but it could
> be that Dell need a slightly different set, and so might Yadro, or so
> might our next board revision for that matter.
> 
> Now, updating the device-tree in the board flash with whatever vendor
> specific information is needed is a LOT easier than getting the kernel
> driver constantly updated. The device-tree after all is there to
> reflect among other things system specific ways in which the SoC is
> wired and configured. This is rather close...

Not sure this helps, but I feel that the proposal pretty closely matches what's 
described in Documentation/devicetree/bindings/mfd/mfd.txt. It's intended that 
nodes using the bindings I'm proposing are children of a 'compatible = 
"syscon", "simple-mfd"' node (this is the case with the features we're hoping 
to describe for our SoC). I should explicitly call that out.

But to go on, "simple-mfd" is effectively an alias of "simple-bus", which means 
its intended to match child node compatibles to drivers provided by the kernel. 
If we shouldn't be describing minor features of a SoC in the devicetree, 
doesn't this invalidate the case for simple-mfd? What is the *correct* use of 
simple-mfd? When is it not used to expose minor features in set of 
"miscellaneous system registers"? Why doesn't this proposed case fit?

Cheers,

Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-17 Thread Andrew Jeffery
On Tue, 17 Jul 2018, at 14:26, Benjamin Herrenschmidt wrote:
> On Mon, 2018-07-16 at 07:55 -0600, Rob Herring wrote:
> > If that data is one set per SoC, then i'm not that concerned having
> > platform-specific data in the driver. That doesn't mean the driver is
> > not "generic". It's still not clear to me in this thread, how much of
> > this is board specific, but given that you've placed all the data in
> > an SoC dtsi file it seems to be all per SoC.
> 
> So Rob, I think that's precisely where the disconnect is.
> 
> I think we all (well hopefully) agree that those few tunables don't fit
> in any existing subystem and aren't likely to ever do (famous last
> words...).
> 
> Where we disagree is we want to make this parametrized via the DT, and
> you want us to hard wire the list in some kind of SoC driver for a
> given SoC family/version.
> 
> The reason I think hard wiring the list in the driver is not a great
> solution is that that list in itself is prone to variations, possibly
> fairly often, between boards, vendors, versions of boards, etc...
> 
> We can't know for sure every SoC tunable (out of the gazillions in
> those chips) are going to be needed for a given system. We know which
> ones we do use for ours, and that's a couple of handfuls, but it could
> be that Dell need a slightly different set, and so might Yadro, or so
> might our next board revision for that matter.
> 
> Now, updating the device-tree in the board flash with whatever vendor
> specific information is needed is a LOT easier than getting the kernel
> driver constantly updated. The device-tree after all is there to
> reflect among other things system specific ways in which the SoC is
> wired and configured. This is rather close...

Not sure this helps, but I feel that the proposal pretty closely matches what's 
described in Documentation/devicetree/bindings/mfd/mfd.txt. It's intended that 
nodes using the bindings I'm proposing are children of a 'compatible = 
"syscon", "simple-mfd"' node (this is the case with the features we're hoping 
to describe for our SoC). I should explicitly call that out.

But to go on, "simple-mfd" is effectively an alias of "simple-bus", which means 
its intended to match child node compatibles to drivers provided by the kernel. 
If we shouldn't be describing minor features of a SoC in the devicetree, 
doesn't this invalidate the case for simple-mfd? What is the *correct* use of 
simple-mfd? When is it not used to expose minor features in set of 
"miscellaneous system registers"? Why doesn't this proposed case fit?

Cheers,

Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-16 Thread Benjamin Herrenschmidt
On Mon, 2018-07-16 at 07:55 -0600, Rob Herring wrote:
> If that data is one set per SoC, then i'm not that concerned having
> platform-specific data in the driver. That doesn't mean the driver is
> not "generic". It's still not clear to me in this thread, how much of
> this is board specific, but given that you've placed all the data in
> an SoC dtsi file it seems to be all per SoC.

So Rob, I think that's precisely where the disconnect is.

I think we all (well hopefully) agree that those few tunables don't fit
in any existing subystem and aren't likely to ever do (famous last
words...).

Where we disagree is we want to make this parametrized via the DT, and
you want us to hard wire the list in some kind of SoC driver for a
given SoC family/version.

The reason I think hard wiring the list in the driver is not a great
solution is that that list in itself is prone to variations, possibly
fairly often, between boards, vendors, versions of boards, etc...

We can't know for sure every SoC tunable (out of the gazillions in
those chips) are going to be needed for a given system. We know which
ones we do use for ours, and that's a couple of handfuls, but it could
be that Dell need a slightly different set, and so might Yadro, or so
might our next board revision for that matter.

Now, updating the device-tree in the board flash with whatever vendor
specific information is needed is a LOT easier than getting the kernel
driver constantly updated. The device-tree after all is there to
reflect among other things system specific ways in which the SoC is
wired and configured. This is rather close...

Cheers,
Ben.



Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-16 Thread Benjamin Herrenschmidt
On Mon, 2018-07-16 at 07:55 -0600, Rob Herring wrote:
> If that data is one set per SoC, then i'm not that concerned having
> platform-specific data in the driver. That doesn't mean the driver is
> not "generic". It's still not clear to me in this thread, how much of
> this is board specific, but given that you've placed all the data in
> an SoC dtsi file it seems to be all per SoC.

So Rob, I think that's precisely where the disconnect is.

I think we all (well hopefully) agree that those few tunables don't fit
in any existing subystem and aren't likely to ever do (famous last
words...).

Where we disagree is we want to make this parametrized via the DT, and
you want us to hard wire the list in some kind of SoC driver for a
given SoC family/version.

The reason I think hard wiring the list in the driver is not a great
solution is that that list in itself is prone to variations, possibly
fairly often, between boards, vendors, versions of boards, etc...

We can't know for sure every SoC tunable (out of the gazillions in
those chips) are going to be needed for a given system. We know which
ones we do use for ours, and that's a couple of handfuls, but it could
be that Dell need a slightly different set, and so might Yadro, or so
might our next board revision for that matter.

Now, updating the device-tree in the board flash with whatever vendor
specific information is needed is a LOT easier than getting the kernel
driver constantly updated. The device-tree after all is there to
reflect among other things system specific ways in which the SoC is
wired and configured. This is rather close...

Cheers,
Ben.



Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-16 Thread Andrew Jeffery
On Mon, 16 Jul 2018, at 23:25, Rob Herring wrote:
> On Fri, Jul 13, 2018 at 12:31 AM Andrew Jeffery  wrote:
> >
> > Hi Rob, Ben,
> >
> > I've replied to you both inline below, hopefully it's clear enough from the 
> > context.
> >
> > On Fri, 13 Jul 2018, at 10:25, Benjamin Herrenschmidt wrote:
> > > On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
> > > > On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > Thanks for the response.
> > > > >
> > > > > On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> > > > > > On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > > > > > > Baseboard Management Controllers (BMCs) are embedded SoCs that 
> > > > > > > exist to
> > > > > > > provide remote management of (primarily) server platforms. BMCs 
> > > > > > > are
> > > > > > > often tightly coupled to the platform in terms of behaviour and 
> > > > > > > provide
> > > > > > > many hardware features integral to booting and running the host 
> > > > > > > system.
> > > > > > >
> > > > > > > Some of these hardware features are simple, for example scratch
> > > > > > > registers provided by the BMC that are exposed to both the host 
> > > > > > > and the
> > > > > > > BMC. In other cases there's a single bit switch to enable or 
> > > > > > > disable
> > > > > > > some of the provided functionality.
> > > > > > >
> > > > > > > The documentation defines bindings for fields in registers that 
> > > > > > > do not
> > > > > > > integrate well into other driver models yet must be described to 
> > > > > > > allow
> > > > > > > the BMC kernel to assume control of these features.
> > > > > >
> > > > > > So we'll get a new binding when that happens? That will break
> > > > > > compatibility.
> > > > >
> > > > > Can you please expand on this? I'm not following.
> > > >
> > > > If we have a subsystem in the future, then there would likely be an
> > > > associated binding which would be different. So if you update the DT,
> > > > then old kernels won't work with it.
> > >
> > > What kind of "subsystem" ? There is almost no way there could be one
> > > for that sort of BMC tunables. We've look at several BMC chips out
> > > there and requirements from several vendors, BIOS and system
> > > manufacturers and it's all over the place.
> >
> > Right - This is the fundamental principle backing these patches: There will 
> > never be a coherent subsystem catering to any of what we want to describe 
> > with these bindings.
> 
> I never said they would. Saying "do not integrate well into other
> driver models YET" implies at some point they will. No point in
> beating this any further, just remove "yet"...

Right, there should have been a comma before 'yet'. Sorry for the confusion.

*snip*

> > > > >
> > > > > Maybe this could be modelled by pinmux, but then we still need some
> > > > > way to expose the mux functions to userspace for selection
> > > > > (userspace needs to transition arbitrarily between at least options
> > > > > 0 and 1 at runtime), at which point we haven't achieved much beyond
> > > > > adding a whole heap of infrastructure in the chain.
> > > > >
> > > > > Given 0 and 1, maybe exposing attributes in relevant drivers would
> > > > > be reasonable, except 0 isn't exposed on the SoC's internal bus so
> > > > > there is no driver on the BMC-side to do so. Taking into account 2
> > > > > and 3 are also purely hardware paths further dashes the idea, as
> > > > > the configuration doesn't really "belong" to the Graphics CRT
> > > > > device more than it belongs anywhere else, except for the fact that
> > > > > there isn't anywhere else to expose it.
> > > > >
> > > > > Further, the BMC's kernel can't make the decision as to when to
> > > > > switch the mux as it knows nothing of the host's state. The BMC
> > > > > userspace is controlling the host's boot state and so *does* know
> > > > > when to flip the switch. Finally, the mux is in separate IP to the
> > > > > CRT or VGA blocks: It lives in the System Control Unit.
> > > > >
> > > > > My current point of view is the DAC mux field is effectively its
> > > > > own device, and we need to control it from userspace, so we need
> > > > > some way to describe it (i.e. not ignore it) in order for its
> > > > > capability to be exposed.
> > > > >
> > > > > I'm fully aware what I'm proposing isn't awesome as it's not
> > > > > providing any real abstraction, but the problem(s) at hand also
> > > > > seem to defy abstraction, and in order to avoid a plethora of
> > > > > bespoke bindings I thought it was reasonable to define something
> > > > > generic.
> > > > >
> > > > > All-in-all I appreciate the suggestion, but assuming you agree with
> > > > > my reasoning above do you have thoughts on other alternatives?
> > > >
> > > > Seems the controls are more fixed than I first thought. All the data
> > > > you have here could simply be within a driver.
> >
> > Rob: A driver for what though? One unique to this 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-16 Thread Andrew Jeffery
On Mon, 16 Jul 2018, at 23:25, Rob Herring wrote:
> On Fri, Jul 13, 2018 at 12:31 AM Andrew Jeffery  wrote:
> >
> > Hi Rob, Ben,
> >
> > I've replied to you both inline below, hopefully it's clear enough from the 
> > context.
> >
> > On Fri, 13 Jul 2018, at 10:25, Benjamin Herrenschmidt wrote:
> > > On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
> > > > On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > Thanks for the response.
> > > > >
> > > > > On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> > > > > > On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > > > > > > Baseboard Management Controllers (BMCs) are embedded SoCs that 
> > > > > > > exist to
> > > > > > > provide remote management of (primarily) server platforms. BMCs 
> > > > > > > are
> > > > > > > often tightly coupled to the platform in terms of behaviour and 
> > > > > > > provide
> > > > > > > many hardware features integral to booting and running the host 
> > > > > > > system.
> > > > > > >
> > > > > > > Some of these hardware features are simple, for example scratch
> > > > > > > registers provided by the BMC that are exposed to both the host 
> > > > > > > and the
> > > > > > > BMC. In other cases there's a single bit switch to enable or 
> > > > > > > disable
> > > > > > > some of the provided functionality.
> > > > > > >
> > > > > > > The documentation defines bindings for fields in registers that 
> > > > > > > do not
> > > > > > > integrate well into other driver models yet must be described to 
> > > > > > > allow
> > > > > > > the BMC kernel to assume control of these features.
> > > > > >
> > > > > > So we'll get a new binding when that happens? That will break
> > > > > > compatibility.
> > > > >
> > > > > Can you please expand on this? I'm not following.
> > > >
> > > > If we have a subsystem in the future, then there would likely be an
> > > > associated binding which would be different. So if you update the DT,
> > > > then old kernels won't work with it.
> > >
> > > What kind of "subsystem" ? There is almost no way there could be one
> > > for that sort of BMC tunables. We've look at several BMC chips out
> > > there and requirements from several vendors, BIOS and system
> > > manufacturers and it's all over the place.
> >
> > Right - This is the fundamental principle backing these patches: There will 
> > never be a coherent subsystem catering to any of what we want to describe 
> > with these bindings.
> 
> I never said they would. Saying "do not integrate well into other
> driver models YET" implies at some point they will. No point in
> beating this any further, just remove "yet"...

Right, there should have been a comma before 'yet'. Sorry for the confusion.

*snip*

> > > > >
> > > > > Maybe this could be modelled by pinmux, but then we still need some
> > > > > way to expose the mux functions to userspace for selection
> > > > > (userspace needs to transition arbitrarily between at least options
> > > > > 0 and 1 at runtime), at which point we haven't achieved much beyond
> > > > > adding a whole heap of infrastructure in the chain.
> > > > >
> > > > > Given 0 and 1, maybe exposing attributes in relevant drivers would
> > > > > be reasonable, except 0 isn't exposed on the SoC's internal bus so
> > > > > there is no driver on the BMC-side to do so. Taking into account 2
> > > > > and 3 are also purely hardware paths further dashes the idea, as
> > > > > the configuration doesn't really "belong" to the Graphics CRT
> > > > > device more than it belongs anywhere else, except for the fact that
> > > > > there isn't anywhere else to expose it.
> > > > >
> > > > > Further, the BMC's kernel can't make the decision as to when to
> > > > > switch the mux as it knows nothing of the host's state. The BMC
> > > > > userspace is controlling the host's boot state and so *does* know
> > > > > when to flip the switch. Finally, the mux is in separate IP to the
> > > > > CRT or VGA blocks: It lives in the System Control Unit.
> > > > >
> > > > > My current point of view is the DAC mux field is effectively its
> > > > > own device, and we need to control it from userspace, so we need
> > > > > some way to describe it (i.e. not ignore it) in order for its
> > > > > capability to be exposed.
> > > > >
> > > > > I'm fully aware what I'm proposing isn't awesome as it's not
> > > > > providing any real abstraction, but the problem(s) at hand also
> > > > > seem to defy abstraction, and in order to avoid a plethora of
> > > > > bespoke bindings I thought it was reasonable to define something
> > > > > generic.
> > > > >
> > > > > All-in-all I appreciate the suggestion, but assuming you agree with
> > > > > my reasoning above do you have thoughts on other alternatives?
> > > >
> > > > Seems the controls are more fixed than I first thought. All the data
> > > > you have here could simply be within a driver.
> >
> > Rob: A driver for what though? One unique to this 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-16 Thread Rob Herring
On Fri, Jul 13, 2018 at 12:31 AM Andrew Jeffery  wrote:
>
> Hi Rob, Ben,
>
> I've replied to you both inline below, hopefully it's clear enough from the 
> context.
>
> On Fri, 13 Jul 2018, at 10:25, Benjamin Herrenschmidt wrote:
> > On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
> > > On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > Thanks for the response.
> > > >
> > > > On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> > > > > On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > > > > > Baseboard Management Controllers (BMCs) are embedded SoCs that 
> > > > > > exist to
> > > > > > provide remote management of (primarily) server platforms. BMCs are
> > > > > > often tightly coupled to the platform in terms of behaviour and 
> > > > > > provide
> > > > > > many hardware features integral to booting and running the host 
> > > > > > system.
> > > > > >
> > > > > > Some of these hardware features are simple, for example scratch
> > > > > > registers provided by the BMC that are exposed to both the host and 
> > > > > > the
> > > > > > BMC. In other cases there's a single bit switch to enable or disable
> > > > > > some of the provided functionality.
> > > > > >
> > > > > > The documentation defines bindings for fields in registers that do 
> > > > > > not
> > > > > > integrate well into other driver models yet must be described to 
> > > > > > allow
> > > > > > the BMC kernel to assume control of these features.
> > > > >
> > > > > So we'll get a new binding when that happens? That will break
> > > > > compatibility.
> > > >
> > > > Can you please expand on this? I'm not following.
> > >
> > > If we have a subsystem in the future, then there would likely be an
> > > associated binding which would be different. So if you update the DT,
> > > then old kernels won't work with it.
> >
> > What kind of "subsystem" ? There is almost no way there could be one
> > for that sort of BMC tunables. We've look at several BMC chips out
> > there and requirements from several vendors, BIOS and system
> > manufacturers and it's all over the place.
>
> Right - This is the fundamental principle backing these patches: There will 
> never be a coherent subsystem catering to any of what we want to describe 
> with these bindings.

I never said they would. Saying "do not integrate well into other
driver models YET" implies at some point they will. No point in
beating this any further, just remove "yet"...

> > > > I feel like this is an argument of tradition. Maybe people have
> > > > been dissuaded from doing so when they don't have a reasonable use-
> > > > case? I'm not saying that what I'm proposing is unquestionably
> > > > reasonable, but I don't want to dismiss it out of hand.
> > >
> ...
> > >
> > > It comes up with system controller type blocks too that just have a
> > > bunch of random registers.
>
> This matches the situation at hand.
>
> > > Those change in every SoC and not in any
> > > controlled or ordered way that would make describing the individual
> > > sub-functions in DT worthwhile.
>
> "Not worthwhile" is what I'm pushing back against for our use cases. I think 
> they are narrow and limited enough to make it worthwhile.
>
> Obviously we want to avoid describing these things *badly* - you mentioned 
> the clock bindings - so I'm happy to hash out what the right representation 
> should be. But I struggle to think the solution is not describing some of our 
> hardware features at all.
>
> >
> > So what's the alternative ? Because without something like what we
> > propose, what's going to happen is /dev/mem ... that's what people do
> > today.
>
> Yep. And I've outlined in the cover letter what I think are the advantages of 
> what I'm proposing over /dev/mem. It's not an incredible gain, but has 
> several of nice-to-have properties.
>
> >
> > > > > A node per register bit doesn't scale.
> > > >
> > > > It isn't meant to scale in terms of a single system. Using it
> > > > extensively is very likely wrong. Separately, register-bit-led does
> > > > pretty much the same thing. Doesn't the scale argument apply there?
> > > > Who is to stop me from attaching an insane number of LEDs to a
> > > > system?
> > >
> > > Review.
> > >
> > > If you look, register-bit-led is rarely used outside of some ARM, Ltd.
> > > boards. It's simply quite rare to have MMIO register bits that have a
> > > fixed function of LED control.
> >
> > Well, same here, we hope to review what goes upstream to make it
> > reasonable. Otherwise it doens't matter. If a random vendor, let's say
> > IBM, chose to chip a system where they put an insane amount of cruft in
> > there, it will only affect those systems's BMC and the userspace stack
> > on it.
> >
> > Thankfully that stack is OpenBMC and IBM is aiming at having their
> > device-tree's upstream, thus reviewed, thus it won't happen.
> >
> > *Anything* can be abused. The point here is that we have a number,
> > 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-16 Thread Rob Herring
On Fri, Jul 13, 2018 at 12:31 AM Andrew Jeffery  wrote:
>
> Hi Rob, Ben,
>
> I've replied to you both inline below, hopefully it's clear enough from the 
> context.
>
> On Fri, 13 Jul 2018, at 10:25, Benjamin Herrenschmidt wrote:
> > On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
> > > On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > Thanks for the response.
> > > >
> > > > On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> > > > > On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > > > > > Baseboard Management Controllers (BMCs) are embedded SoCs that 
> > > > > > exist to
> > > > > > provide remote management of (primarily) server platforms. BMCs are
> > > > > > often tightly coupled to the platform in terms of behaviour and 
> > > > > > provide
> > > > > > many hardware features integral to booting and running the host 
> > > > > > system.
> > > > > >
> > > > > > Some of these hardware features are simple, for example scratch
> > > > > > registers provided by the BMC that are exposed to both the host and 
> > > > > > the
> > > > > > BMC. In other cases there's a single bit switch to enable or disable
> > > > > > some of the provided functionality.
> > > > > >
> > > > > > The documentation defines bindings for fields in registers that do 
> > > > > > not
> > > > > > integrate well into other driver models yet must be described to 
> > > > > > allow
> > > > > > the BMC kernel to assume control of these features.
> > > > >
> > > > > So we'll get a new binding when that happens? That will break
> > > > > compatibility.
> > > >
> > > > Can you please expand on this? I'm not following.
> > >
> > > If we have a subsystem in the future, then there would likely be an
> > > associated binding which would be different. So if you update the DT,
> > > then old kernels won't work with it.
> >
> > What kind of "subsystem" ? There is almost no way there could be one
> > for that sort of BMC tunables. We've look at several BMC chips out
> > there and requirements from several vendors, BIOS and system
> > manufacturers and it's all over the place.
>
> Right - This is the fundamental principle backing these patches: There will 
> never be a coherent subsystem catering to any of what we want to describe 
> with these bindings.

I never said they would. Saying "do not integrate well into other
driver models YET" implies at some point they will. No point in
beating this any further, just remove "yet"...

> > > > I feel like this is an argument of tradition. Maybe people have
> > > > been dissuaded from doing so when they don't have a reasonable use-
> > > > case? I'm not saying that what I'm proposing is unquestionably
> > > > reasonable, but I don't want to dismiss it out of hand.
> > >
> ...
> > >
> > > It comes up with system controller type blocks too that just have a
> > > bunch of random registers.
>
> This matches the situation at hand.
>
> > > Those change in every SoC and not in any
> > > controlled or ordered way that would make describing the individual
> > > sub-functions in DT worthwhile.
>
> "Not worthwhile" is what I'm pushing back against for our use cases. I think 
> they are narrow and limited enough to make it worthwhile.
>
> Obviously we want to avoid describing these things *badly* - you mentioned 
> the clock bindings - so I'm happy to hash out what the right representation 
> should be. But I struggle to think the solution is not describing some of our 
> hardware features at all.
>
> >
> > So what's the alternative ? Because without something like what we
> > propose, what's going to happen is /dev/mem ... that's what people do
> > today.
>
> Yep. And I've outlined in the cover letter what I think are the advantages of 
> what I'm proposing over /dev/mem. It's not an incredible gain, but has 
> several of nice-to-have properties.
>
> >
> > > > > A node per register bit doesn't scale.
> > > >
> > > > It isn't meant to scale in terms of a single system. Using it
> > > > extensively is very likely wrong. Separately, register-bit-led does
> > > > pretty much the same thing. Doesn't the scale argument apply there?
> > > > Who is to stop me from attaching an insane number of LEDs to a
> > > > system?
> > >
> > > Review.
> > >
> > > If you look, register-bit-led is rarely used outside of some ARM, Ltd.
> > > boards. It's simply quite rare to have MMIO register bits that have a
> > > fixed function of LED control.
> >
> > Well, same here, we hope to review what goes upstream to make it
> > reasonable. Otherwise it doens't matter. If a random vendor, let's say
> > IBM, chose to chip a system where they put an insane amount of cruft in
> > there, it will only affect those systems's BMC and the userspace stack
> > on it.
> >
> > Thankfully that stack is OpenBMC and IBM is aiming at having their
> > device-tree's upstream, thus reviewed, thus it won't happen.
> >
> > *Anything* can be abused. The point here is that we have a number,
> > 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-15 Thread Andrew Jeffery
Hi Alexander,

I've rearranged your reply slightly :)

On Sat, 14 Jul 2018, at 00:44, Alexander Amelkin wrote:

> 
> So I'm writing this to support your position in this discussion and to
> let Rob and other reviewers know that this feature is actually needed.

Thanks.

So to summarise some other replies so far, YADRO (Alexander) and Dell (Eugene) 
- platform vendors integrating BMCs - are interested in a solution, and the 
patches are seen useful for ASPEED and Nuvoton (Avi) BMCs.

> 
> From the discussion it looks to me like Rob is not familiar with
> specifics of BMC-managed servers and tries to apply to them the rules
> that have proven to be good for workstations and laptops.
> 

Whatever Rob's familiarity with BMCs or other systems, I'm keen to listen to, 
explore and gain from his experience. I was certainly expecting push back on 
these patches and in different circumstances I'd probably say no to them as 
well. The argument for them needs to stand up to scrutiny, and if that scrutiny 
leads to alternative solutions (that, ideally, are not /dev/mem) then that's 
fine.

Currently I think we need what I've proposed despite it looking like a 
violation of abstractions, because the hardware itself doesn't have a neat, 
abstract design. But we could be thinking about it wrong, e.g. maybe what we 
need instead are no devicetree bindings and simply some in-kernel helpers that 
allow arbitrary drivers to instantiate the sysfs interface I've proposed for 
these fields under their control. That removes the need for explicit 
description in the devicetree, though may lead to the creation of a number of 
unique drivers (and bindings) that each cover only the functions we're trying 
to generalise over here.

It would be good to have some feedback on the proposed sysfs interface as well 
- as explored above we could feasibly live without the bindings in their 
current form but taking away the sysfs interface would nuke the series.

Cheers,

Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-15 Thread Andrew Jeffery
Hi Alexander,

I've rearranged your reply slightly :)

On Sat, 14 Jul 2018, at 00:44, Alexander Amelkin wrote:

> 
> So I'm writing this to support your position in this discussion and to
> let Rob and other reviewers know that this feature is actually needed.

Thanks.

So to summarise some other replies so far, YADRO (Alexander) and Dell (Eugene) 
- platform vendors integrating BMCs - are interested in a solution, and the 
patches are seen useful for ASPEED and Nuvoton (Avi) BMCs.

> 
> From the discussion it looks to me like Rob is not familiar with
> specifics of BMC-managed servers and tries to apply to them the rules
> that have proven to be good for workstations and laptops.
> 

Whatever Rob's familiarity with BMCs or other systems, I'm keen to listen to, 
explore and gain from his experience. I was certainly expecting push back on 
these patches and in different circumstances I'd probably say no to them as 
well. The argument for them needs to stand up to scrutiny, and if that scrutiny 
leads to alternative solutions (that, ideally, are not /dev/mem) then that's 
fine.

Currently I think we need what I've proposed despite it looking like a 
violation of abstractions, because the hardware itself doesn't have a neat, 
abstract design. But we could be thinking about it wrong, e.g. maybe what we 
need instead are no devicetree bindings and simply some in-kernel helpers that 
allow arbitrary drivers to instantiate the sysfs interface I've proposed for 
these fields under their control. That removes the need for explicit 
description in the devicetree, though may lead to the creation of a number of 
unique drivers (and bindings) that each cover only the functions we're trying 
to generalise over here.

It would be good to have some feedback on the proposed sysfs interface as well 
- as explored above we could feasibly live without the bindings in their 
current form but taking away the sysfs interface would nuke the series.

Cheers,

Andrew


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-15 Thread Avi Fishman
+ my vote as Nuvoton's BMC FW provider.
On Fri, Jul 13, 2018 at 10:07 PM  wrote:
>
> >> Dell - Internal Use - Confidential
> >Really?  To a public mailing list?  odd...
>
> Ha yea sorry - I was so excited to show my support, that I jumped the gun :)
>


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-15 Thread Avi Fishman
+ my vote as Nuvoton's BMC FW provider.
On Fri, Jul 13, 2018 at 10:07 PM  wrote:
>
> >> Dell - Internal Use - Confidential
> >Really?  To a public mailing list?  odd...
>
> Ha yea sorry - I was so excited to show my support, that I jumped the gun :)
>


RE: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-13 Thread Eugene.Cho
>> Dell - Internal Use - Confidential  
>Really?  To a public mailing list?  odd...

Ha yea sorry - I was so excited to show my support, that I jumped the gun :)



RE: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-13 Thread Eugene.Cho
>> Dell - Internal Use - Confidential  
>Really?  To a public mailing list?  odd...

Ha yea sorry - I was so excited to show my support, that I jumped the gun :)



Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-13 Thread Greg KH
On Fri, Jul 13, 2018 at 06:49:04PM +, eugene@dell.com wrote:
> Dell - Internal Use - Confidential  

Really?  To a public mailing list?  odd...



Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-13 Thread Greg KH
On Fri, Jul 13, 2018 at 06:49:04PM +, eugene@dell.com wrote:
> Dell - Internal Use - Confidential  

Really?  To a public mailing list?  odd...



RE: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-13 Thread Eugene.Cho
Dell - Internal Use - Confidential  

+1 from someone using Nuvoton's BMC SoC

-Original Message-
From: Alexander Amelkin [mailto:a.amel...@yadro.com] 
Sent: Friday, July 13, 2018 10:14 AM
To: Andrew Jeffery; Benjamin Herrenschmidt; Rob Herring
Cc: Mark Rutland; devicet...@vger.kernel.org; Greg Kroah-Hartman; Cho, Eugene; 
linux-kernel@vger.kernel.org; Joel Stanley; stew...@linux.ibm.com; OpenBMC 
Maillist; moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
Subject: Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC 
control fields

Andrew, Ben, first of all let me thank you for bringing in this set of patches.

From the discussion it looks to me like Rob is not familiar with specifics of 
BMC-managed servers and tries to apply to them the rules that have proven to be 
good for workstations and laptops.

As someone using /dev/mem these days to configure those registers in BMCs, I'm 
all for this patch set as it will make BMC configuration less obscure. Writing 
1 or 0 to a named node is way clearer than writing some magic value at some 
magic offset in /dev/mem. I like the idea of having it all configurable via DT 
as it allows for only having exported the nodes that are actually needed, thus 
reducing, as you have said, the foot-gun.

So far I do not have any objections or constructive comments to the 
architecture of the proposed patches.

So I'm writing this to support your position in this discussion and to let Rob 
and other reviewers know that this feature is actually needed.

With best regards,
Alexander Amelkin

13.07.2018 09:31, Andrew Jeffery wrote:
> Hi Rob, Ben,
>
> I've replied to you both inline below, hopefully it's clear enough from the 
> context.
>
> On Fri, 13 Jul 2018, at 10:25, Benjamin Herrenschmidt wrote:
>> On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
>>> On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
>>>> Hi Rob,
>>>>
>>>> Thanks for the response.
>>>>
>>>> On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
>>>>> On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
>>>>>> Baseboard Management Controllers (BMCs) are embedded SoCs that 
>>>>>> exist to provide remote management of (primarily) server 
>>>>>> platforms. BMCs are often tightly coupled to the platform in 
>>>>>> terms of behaviour and provide many hardware features integral to 
>>>>>> booting and running the host system.
>>>>>>
>>>>>> Some of these hardware features are simple, for example scratch 
>>>>>> registers provided by the BMC that are exposed to both the host 
>>>>>> and the BMC. In other cases there's a single bit switch to enable 
>>>>>> or disable some of the provided functionality.
>>>>>>
>>>>>> The documentation defines bindings for fields in registers that 
>>>>>> do not integrate well into other driver models yet must be 
>>>>>> described to allow the BMC kernel to assume control of these features.
>>>>> So we'll get a new binding when that happens? That will break 
>>>>> compatibility.
>>>> Can you please expand on this? I'm not following.
>>> If we have a subsystem in the future, then there would likely be an 
>>> associated binding which would be different. So if you update the 
>>> DT, then old kernels won't work with it.
>> What kind of "subsystem" ? There is almost no way there could be one 
>> for that sort of BMC tunables. We've look at several BMC chips out 
>> there and requirements from several vendors, BIOS and system 
>> manufacturers and it's all over the place.
> Right - This is the fundamental principle backing these patches: There will 
> never be a coherent subsystem catering to any of what we want to describe 
> with these bindings.
>
>>>> I feel like this is an argument of tradition. Maybe people have 
>>>> been dissuaded from doing so when they don't have a reasonable use- 
>>>> case? I'm not saying that what I'm proposing is unquestionably 
>>>> reasonable, but I don't want to dismiss it out of hand.
> ...
>>> It comes up with system controller type blocks too that just have a 
>>> bunch of random registers.
> This matches the situation at hand.
>
>>> Those change in every SoC and not in any controlled or ordered way 
>>> that would make describing the individual sub-functions in DT 
>>> worthwhile.
> "Not worthwhile" is what I'm pushing back against for our use cases. I think 
> they are narrow and limited enough to make it worthwhile.
>

RE: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-13 Thread Eugene.Cho
Dell - Internal Use - Confidential  

+1 from someone using Nuvoton's BMC SoC

-Original Message-
From: Alexander Amelkin [mailto:a.amel...@yadro.com] 
Sent: Friday, July 13, 2018 10:14 AM
To: Andrew Jeffery; Benjamin Herrenschmidt; Rob Herring
Cc: Mark Rutland; devicet...@vger.kernel.org; Greg Kroah-Hartman; Cho, Eugene; 
linux-kernel@vger.kernel.org; Joel Stanley; stew...@linux.ibm.com; OpenBMC 
Maillist; moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
Subject: Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC 
control fields

Andrew, Ben, first of all let me thank you for bringing in this set of patches.

From the discussion it looks to me like Rob is not familiar with specifics of 
BMC-managed servers and tries to apply to them the rules that have proven to be 
good for workstations and laptops.

As someone using /dev/mem these days to configure those registers in BMCs, I'm 
all for this patch set as it will make BMC configuration less obscure. Writing 
1 or 0 to a named node is way clearer than writing some magic value at some 
magic offset in /dev/mem. I like the idea of having it all configurable via DT 
as it allows for only having exported the nodes that are actually needed, thus 
reducing, as you have said, the foot-gun.

So far I do not have any objections or constructive comments to the 
architecture of the proposed patches.

So I'm writing this to support your position in this discussion and to let Rob 
and other reviewers know that this feature is actually needed.

With best regards,
Alexander Amelkin

13.07.2018 09:31, Andrew Jeffery wrote:
> Hi Rob, Ben,
>
> I've replied to you both inline below, hopefully it's clear enough from the 
> context.
>
> On Fri, 13 Jul 2018, at 10:25, Benjamin Herrenschmidt wrote:
>> On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
>>> On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
>>>> Hi Rob,
>>>>
>>>> Thanks for the response.
>>>>
>>>> On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
>>>>> On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
>>>>>> Baseboard Management Controllers (BMCs) are embedded SoCs that 
>>>>>> exist to provide remote management of (primarily) server 
>>>>>> platforms. BMCs are often tightly coupled to the platform in 
>>>>>> terms of behaviour and provide many hardware features integral to 
>>>>>> booting and running the host system.
>>>>>>
>>>>>> Some of these hardware features are simple, for example scratch 
>>>>>> registers provided by the BMC that are exposed to both the host 
>>>>>> and the BMC. In other cases there's a single bit switch to enable 
>>>>>> or disable some of the provided functionality.
>>>>>>
>>>>>> The documentation defines bindings for fields in registers that 
>>>>>> do not integrate well into other driver models yet must be 
>>>>>> described to allow the BMC kernel to assume control of these features.
>>>>> So we'll get a new binding when that happens? That will break 
>>>>> compatibility.
>>>> Can you please expand on this? I'm not following.
>>> If we have a subsystem in the future, then there would likely be an 
>>> associated binding which would be different. So if you update the 
>>> DT, then old kernels won't work with it.
>> What kind of "subsystem" ? There is almost no way there could be one 
>> for that sort of BMC tunables. We've look at several BMC chips out 
>> there and requirements from several vendors, BIOS and system 
>> manufacturers and it's all over the place.
> Right - This is the fundamental principle backing these patches: There will 
> never be a coherent subsystem catering to any of what we want to describe 
> with these bindings.
>
>>>> I feel like this is an argument of tradition. Maybe people have 
>>>> been dissuaded from doing so when they don't have a reasonable use- 
>>>> case? I'm not saying that what I'm proposing is unquestionably 
>>>> reasonable, but I don't want to dismiss it out of hand.
> ...
>>> It comes up with system controller type blocks too that just have a 
>>> bunch of random registers.
> This matches the situation at hand.
>
>>> Those change in every SoC and not in any controlled or ordered way 
>>> that would make describing the individual sub-functions in DT 
>>> worthwhile.
> "Not worthwhile" is what I'm pushing back against for our use cases. I think 
> they are narrow and limited enough to make it worthwhile.
>

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-13 Thread Alexander Amelkin
Andrew, Ben, first of all let me thank you for bringing in this set of
patches.

From the discussion it looks to me like Rob is not familiar with
specifics of BMC-managed servers and tries to apply to them the rules
that have proven to be good for workstations and laptops.

As someone using /dev/mem these days to configure those registers in
BMCs, I'm all for this patch set as it will make BMC configuration less
obscure. Writing 1 or 0 to a named node is way clearer than writing some
magic value at some magic offset in /dev/mem. I like the idea of having
it all configurable via DT as it allows for only having exported the
nodes that are actually needed, thus reducing, as you have said, the
foot-gun.

So far I do not have any objections or constructive comments to the
architecture of the proposed patches.

So I'm writing this to support your position in this discussion and to
let Rob and other reviewers know that this feature is actually needed.

With best regards,
Alexander Amelkin

13.07.2018 09:31, Andrew Jeffery wrote:
> Hi Rob, Ben,
>
> I've replied to you both inline below, hopefully it's clear enough from the 
> context.
>
> On Fri, 13 Jul 2018, at 10:25, Benjamin Herrenschmidt wrote:
>> On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
>>> On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
 Hi Rob,

 Thanks for the response.

 On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
>> Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
>> provide remote management of (primarily) server platforms. BMCs are
>> often tightly coupled to the platform in terms of behaviour and provide
>> many hardware features integral to booting and running the host system.
>>
>> Some of these hardware features are simple, for example scratch
>> registers provided by the BMC that are exposed to both the host and the
>> BMC. In other cases there's a single bit switch to enable or disable
>> some of the provided functionality.
>>
>> The documentation defines bindings for fields in registers that do not
>> integrate well into other driver models yet must be described to allow
>> the BMC kernel to assume control of these features.
> So we'll get a new binding when that happens? That will break
> compatibility.
 Can you please expand on this? I'm not following.
>>> If we have a subsystem in the future, then there would likely be an
>>> associated binding which would be different. So if you update the DT,
>>> then old kernels won't work with it.
>> What kind of "subsystem" ? There is almost no way there could be one
>> for that sort of BMC tunables. We've look at several BMC chips out
>> there and requirements from several vendors, BIOS and system
>> manufacturers and it's all over the place.
> Right - This is the fundamental principle backing these patches: There will 
> never be a coherent subsystem catering to any of what we want to describe 
> with these bindings.
>
 I feel like this is an argument of tradition. Maybe people have
 been dissuaded from doing so when they don't have a reasonable use-
 case? I'm not saying that what I'm proposing is unquestionably
 reasonable, but I don't want to dismiss it out of hand.
> ...
>>> It comes up with system controller type blocks too that just have a
>>> bunch of random registers. 
> This matches the situation at hand.
>
>>> Those change in every SoC and not in any
>>> controlled or ordered way that would make describing the individual
>>> sub-functions in DT worthwhile.
> "Not worthwhile" is what I'm pushing back against for our use cases. I think 
> they are narrow and limited enough to make it worthwhile.
>
> Obviously we want to avoid describing these things *badly* - you mentioned 
> the clock bindings - so I'm happy to hash out what the right representation 
> should be. But I struggle to think the solution is not describing some of our 
> hardware features at all.
>
>> So what's the alternative ? Because without something like what we
>> propose, what's going to happen is /dev/mem ... that's what people do
>> today.
> Yep. And I've outlined in the cover letter what I think are the advantages of 
> what I'm proposing over /dev/mem. It's not an incredible gain, but has 
> several of nice-to-have properties.
>
> A node per register bit doesn't scale.
 It isn't meant to scale in terms of a single system. Using it
 extensively is very likely wrong. Separately, register-bit-led does
 pretty much the same thing. Doesn't the scale argument apply there?
 Who is to stop me from attaching an insane number of LEDs to a
 system?
>>> Review.
>>>
>>> If you look, register-bit-led is rarely used outside of some ARM, Ltd.
>>> boards. It's simply quite rare to have MMIO register bits that have a
>>> fixed function of LED control.
>> Well, same here, we hope to review what goes 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-13 Thread Alexander Amelkin
Andrew, Ben, first of all let me thank you for bringing in this set of
patches.

From the discussion it looks to me like Rob is not familiar with
specifics of BMC-managed servers and tries to apply to them the rules
that have proven to be good for workstations and laptops.

As someone using /dev/mem these days to configure those registers in
BMCs, I'm all for this patch set as it will make BMC configuration less
obscure. Writing 1 or 0 to a named node is way clearer than writing some
magic value at some magic offset in /dev/mem. I like the idea of having
it all configurable via DT as it allows for only having exported the
nodes that are actually needed, thus reducing, as you have said, the
foot-gun.

So far I do not have any objections or constructive comments to the
architecture of the proposed patches.

So I'm writing this to support your position in this discussion and to
let Rob and other reviewers know that this feature is actually needed.

With best regards,
Alexander Amelkin

13.07.2018 09:31, Andrew Jeffery wrote:
> Hi Rob, Ben,
>
> I've replied to you both inline below, hopefully it's clear enough from the 
> context.
>
> On Fri, 13 Jul 2018, at 10:25, Benjamin Herrenschmidt wrote:
>> On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
>>> On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
 Hi Rob,

 Thanks for the response.

 On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
>> Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
>> provide remote management of (primarily) server platforms. BMCs are
>> often tightly coupled to the platform in terms of behaviour and provide
>> many hardware features integral to booting and running the host system.
>>
>> Some of these hardware features are simple, for example scratch
>> registers provided by the BMC that are exposed to both the host and the
>> BMC. In other cases there's a single bit switch to enable or disable
>> some of the provided functionality.
>>
>> The documentation defines bindings for fields in registers that do not
>> integrate well into other driver models yet must be described to allow
>> the BMC kernel to assume control of these features.
> So we'll get a new binding when that happens? That will break
> compatibility.
 Can you please expand on this? I'm not following.
>>> If we have a subsystem in the future, then there would likely be an
>>> associated binding which would be different. So if you update the DT,
>>> then old kernels won't work with it.
>> What kind of "subsystem" ? There is almost no way there could be one
>> for that sort of BMC tunables. We've look at several BMC chips out
>> there and requirements from several vendors, BIOS and system
>> manufacturers and it's all over the place.
> Right - This is the fundamental principle backing these patches: There will 
> never be a coherent subsystem catering to any of what we want to describe 
> with these bindings.
>
 I feel like this is an argument of tradition. Maybe people have
 been dissuaded from doing so when they don't have a reasonable use-
 case? I'm not saying that what I'm proposing is unquestionably
 reasonable, but I don't want to dismiss it out of hand.
> ...
>>> It comes up with system controller type blocks too that just have a
>>> bunch of random registers. 
> This matches the situation at hand.
>
>>> Those change in every SoC and not in any
>>> controlled or ordered way that would make describing the individual
>>> sub-functions in DT worthwhile.
> "Not worthwhile" is what I'm pushing back against for our use cases. I think 
> they are narrow and limited enough to make it worthwhile.
>
> Obviously we want to avoid describing these things *badly* - you mentioned 
> the clock bindings - so I'm happy to hash out what the right representation 
> should be. But I struggle to think the solution is not describing some of our 
> hardware features at all.
>
>> So what's the alternative ? Because without something like what we
>> propose, what's going to happen is /dev/mem ... that's what people do
>> today.
> Yep. And I've outlined in the cover letter what I think are the advantages of 
> what I'm proposing over /dev/mem. It's not an incredible gain, but has 
> several of nice-to-have properties.
>
> A node per register bit doesn't scale.
 It isn't meant to scale in terms of a single system. Using it
 extensively is very likely wrong. Separately, register-bit-led does
 pretty much the same thing. Doesn't the scale argument apply there?
 Who is to stop me from attaching an insane number of LEDs to a
 system?
>>> Review.
>>>
>>> If you look, register-bit-led is rarely used outside of some ARM, Ltd.
>>> boards. It's simply quite rare to have MMIO register bits that have a
>>> fixed function of LED control.
>> Well, same here, we hope to review what goes 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-13 Thread Andrew Jeffery
Hi Rob, Ben,

I've replied to you both inline below, hopefully it's clear enough from the 
context.

On Fri, 13 Jul 2018, at 10:25, Benjamin Herrenschmidt wrote:
> On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
> > On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
> > > 
> > > Hi Rob,
> > > 
> > > Thanks for the response.
> > > 
> > > On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> > > > On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > > > > Baseboard Management Controllers (BMCs) are embedded SoCs that exist 
> > > > > to
> > > > > provide remote management of (primarily) server platforms. BMCs are
> > > > > often tightly coupled to the platform in terms of behaviour and 
> > > > > provide
> > > > > many hardware features integral to booting and running the host 
> > > > > system.
> > > > > 
> > > > > Some of these hardware features are simple, for example scratch
> > > > > registers provided by the BMC that are exposed to both the host and 
> > > > > the
> > > > > BMC. In other cases there's a single bit switch to enable or disable
> > > > > some of the provided functionality.
> > > > > 
> > > > > The documentation defines bindings for fields in registers that do not
> > > > > integrate well into other driver models yet must be described to allow
> > > > > the BMC kernel to assume control of these features.
> > > > 
> > > > So we'll get a new binding when that happens? That will break
> > > > compatibility.
> > > 
> > > Can you please expand on this? I'm not following.
> > 
> > If we have a subsystem in the future, then there would likely be an
> > associated binding which would be different. So if you update the DT,
> > then old kernels won't work with it.
> 
> What kind of "subsystem" ? There is almost no way there could be one
> for that sort of BMC tunables. We've look at several BMC chips out
> there and requirements from several vendors, BIOS and system
> manufacturers and it's all over the place.

Right - This is the fundamental principle backing these patches: There will 
never be a coherent subsystem catering to any of what we want to describe with 
these bindings.

> 
> > > I feel like this is an argument of tradition. Maybe people have
> > > been dissuaded from doing so when they don't have a reasonable use-
> > > case? I'm not saying that what I'm proposing is unquestionably
> > > reasonable, but I don't want to dismiss it out of hand.
> > 
...
> > 
> > It comes up with system controller type blocks too that just have a
> > bunch of random registers. 

This matches the situation at hand.

> > Those change in every SoC and not in any
> > controlled or ordered way that would make describing the individual
> > sub-functions in DT worthwhile.

"Not worthwhile" is what I'm pushing back against for our use cases. I think 
they are narrow and limited enough to make it worthwhile.

Obviously we want to avoid describing these things *badly* - you mentioned the 
clock bindings - so I'm happy to hash out what the right representation should 
be. But I struggle to think the solution is not describing some of our hardware 
features at all.

> 
> So what's the alternative ? Because without something like what we
> propose, what's going to happen is /dev/mem ... that's what people do
> today.

Yep. And I've outlined in the cover letter what I think are the advantages of 
what I'm proposing over /dev/mem. It's not an incredible gain, but has several 
of nice-to-have properties.

> 
> > > > A node per register bit doesn't scale.
> > > 
> > > It isn't meant to scale in terms of a single system. Using it
> > > extensively is very likely wrong. Separately, register-bit-led does
> > > pretty much the same thing. Doesn't the scale argument apply there?
> > > Who is to stop me from attaching an insane number of LEDs to a
> > > system?
> > 
> > Review.
> > 
> > If you look, register-bit-led is rarely used outside of some ARM, Ltd.
> > boards. It's simply quite rare to have MMIO register bits that have a
> > fixed function of LED control.
> 
> Well, same here, we hope to review what goes upstream to make it
> reasonable. Otherwise it doens't matter. If a random vendor, let's say
> IBM, chose to chip a system where they put an insane amount of cruft in
> there, it will only affect those systems's BMC and the userspace stack
> on it.
> 
> Thankfully that stack is OpenBMC and IBM is aiming at having their
> device-tree's upstream, thus reviewed, thus it won't happen.
> 
> *Anything* can be abused. The point here is that we have a number,
> thankfully rather small, maybe a dozen or two, of tunables that are
> quite specific to a combination (system vendor, bmc vendor, system
> model) which control a few HW features that essentially do *NOT* fit in
> a subsystem.

Exactly. I tried to head off the abuse vector by requiring that uses be listed 
in the bindings document, and thus enforce some level of review. It might not 
be the most effective approach at the end of the day, but at least 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-13 Thread Andrew Jeffery
Hi Rob, Ben,

I've replied to you both inline below, hopefully it's clear enough from the 
context.

On Fri, 13 Jul 2018, at 10:25, Benjamin Herrenschmidt wrote:
> On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
> > On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
> > > 
> > > Hi Rob,
> > > 
> > > Thanks for the response.
> > > 
> > > On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> > > > On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > > > > Baseboard Management Controllers (BMCs) are embedded SoCs that exist 
> > > > > to
> > > > > provide remote management of (primarily) server platforms. BMCs are
> > > > > often tightly coupled to the platform in terms of behaviour and 
> > > > > provide
> > > > > many hardware features integral to booting and running the host 
> > > > > system.
> > > > > 
> > > > > Some of these hardware features are simple, for example scratch
> > > > > registers provided by the BMC that are exposed to both the host and 
> > > > > the
> > > > > BMC. In other cases there's a single bit switch to enable or disable
> > > > > some of the provided functionality.
> > > > > 
> > > > > The documentation defines bindings for fields in registers that do not
> > > > > integrate well into other driver models yet must be described to allow
> > > > > the BMC kernel to assume control of these features.
> > > > 
> > > > So we'll get a new binding when that happens? That will break
> > > > compatibility.
> > > 
> > > Can you please expand on this? I'm not following.
> > 
> > If we have a subsystem in the future, then there would likely be an
> > associated binding which would be different. So if you update the DT,
> > then old kernels won't work with it.
> 
> What kind of "subsystem" ? There is almost no way there could be one
> for that sort of BMC tunables. We've look at several BMC chips out
> there and requirements from several vendors, BIOS and system
> manufacturers and it's all over the place.

Right - This is the fundamental principle backing these patches: There will 
never be a coherent subsystem catering to any of what we want to describe with 
these bindings.

> 
> > > I feel like this is an argument of tradition. Maybe people have
> > > been dissuaded from doing so when they don't have a reasonable use-
> > > case? I'm not saying that what I'm proposing is unquestionably
> > > reasonable, but I don't want to dismiss it out of hand.
> > 
...
> > 
> > It comes up with system controller type blocks too that just have a
> > bunch of random registers. 

This matches the situation at hand.

> > Those change in every SoC and not in any
> > controlled or ordered way that would make describing the individual
> > sub-functions in DT worthwhile.

"Not worthwhile" is what I'm pushing back against for our use cases. I think 
they are narrow and limited enough to make it worthwhile.

Obviously we want to avoid describing these things *badly* - you mentioned the 
clock bindings - so I'm happy to hash out what the right representation should 
be. But I struggle to think the solution is not describing some of our hardware 
features at all.

> 
> So what's the alternative ? Because without something like what we
> propose, what's going to happen is /dev/mem ... that's what people do
> today.

Yep. And I've outlined in the cover letter what I think are the advantages of 
what I'm proposing over /dev/mem. It's not an incredible gain, but has several 
of nice-to-have properties.

> 
> > > > A node per register bit doesn't scale.
> > > 
> > > It isn't meant to scale in terms of a single system. Using it
> > > extensively is very likely wrong. Separately, register-bit-led does
> > > pretty much the same thing. Doesn't the scale argument apply there?
> > > Who is to stop me from attaching an insane number of LEDs to a
> > > system?
> > 
> > Review.
> > 
> > If you look, register-bit-led is rarely used outside of some ARM, Ltd.
> > boards. It's simply quite rare to have MMIO register bits that have a
> > fixed function of LED control.
> 
> Well, same here, we hope to review what goes upstream to make it
> reasonable. Otherwise it doens't matter. If a random vendor, let's say
> IBM, chose to chip a system where they put an insane amount of cruft in
> there, it will only affect those systems's BMC and the userspace stack
> on it.
> 
> Thankfully that stack is OpenBMC and IBM is aiming at having their
> device-tree's upstream, thus reviewed, thus it won't happen.
> 
> *Anything* can be abused. The point here is that we have a number,
> thankfully rather small, maybe a dozen or two, of tunables that are
> quite specific to a combination (system vendor, bmc vendor, system
> model) which control a few HW features that essentially do *NOT* fit in
> a subsystem.

Exactly. I tried to head off the abuse vector by requiring that uses be listed 
in the bindings document, and thus enforce some level of review. It might not 
be the most effective approach at the end of the day, but at least 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-12 Thread Benjamin Herrenschmidt
On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
> On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
> > 
> > Hi Rob,
> > 
> > Thanks for the response.
> > 
> > On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> > > On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > > > Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
> > > > provide remote management of (primarily) server platforms. BMCs are
> > > > often tightly coupled to the platform in terms of behaviour and provide
> > > > many hardware features integral to booting and running the host system.
> > > > 
> > > > Some of these hardware features are simple, for example scratch
> > > > registers provided by the BMC that are exposed to both the host and the
> > > > BMC. In other cases there's a single bit switch to enable or disable
> > > > some of the provided functionality.
> > > > 
> > > > The documentation defines bindings for fields in registers that do not
> > > > integrate well into other driver models yet must be described to allow
> > > > the BMC kernel to assume control of these features.
> > > 
> > > So we'll get a new binding when that happens? That will break
> > > compatibility.
> > 
> > Can you please expand on this? I'm not following.
> 
> If we have a subsystem in the future, then there would likely be an
> associated binding which would be different. So if you update the DT,
> then old kernels won't work with it.

What kind of "subsystem" ? There is almost no way there could be one
for that sort of BMC tunables. We've look at several BMC chips out
there and requirements from several vendors, BIOS and system
manufacturers and it's all over the place.

> > I feel like this is an argument of tradition. Maybe people have
> > been dissuaded from doing so when they don't have a reasonable use-
> > case? I'm not saying that what I'm proposing is unquestionably
> > reasonable, but I don't want to dismiss it out of hand.
> 
> One of experience. The one that stands out is clock bindings.
> Initially we were doing a node per clock modelling which could end up
> being 100s of nodes and is difficult to get right (with DT being an
> ABI).
> 
> It comes up with system controller type blocks too that just have a
> bunch of random registers. Those change in every SoC and not in any
> controlled or ordered way that would make describing the individual
> sub-functions in DT worthwhile.

So what's the alternative ? Because without something like what we
propose, what's going to happen is /dev/mem ... that's what people do
today.

> > > A node per register bit doesn't scale.
> > 
> > It isn't meant to scale in terms of a single system. Using it
> > extensively is very likely wrong. Separately, register-bit-led does
> > pretty much the same thing. Doesn't the scale argument apply there?
> > Who is to stop me from attaching an insane number of LEDs to a
> > system?
> 
> Review.
> 
> If you look, register-bit-led is rarely used outside of some ARM, Ltd.
> boards. It's simply quite rare to have MMIO register bits that have a
> fixed function of LED control.

Well, same here, we hope to review what goes upstream to make it
reasonable. Otherwise it doens't matter. If a random vendor, let's say
IBM, chose to chip a system where they put an insane amount of cruft in
there, it will only affect those systems's BMC and the userspace stack
on it.

Thankfully that stack is OpenBMC and IBM is aiming at having their
device-tree's upstream, thus reviewed, thus it won't happen.

*Anything* can be abused. The point here is that we have a number,
thankfully rather small, maybe a dozen or two, of tunables that are
quite specific to a combination (system vendor, bmc vendor, system
model) which control a few HW features that essentially do *NOT* fit in
a subsystem.

For everything that does, we have created proper drivers (and are doing
more).


> > Obviously if there are lots of systems using it sparingly and
> > legitimately then maybe there's a scale issue, but isn't that just
> > a reality of different hardware designs? Whoever is implementing
> > support for the system is going to have to describe the hardware
> > one way or another.
> > 
> > > 
> > > Maybe this should be modelled using GPIO binding? There's a line there
> > > too as whether the signals are "general purpose" or not.
> > 
> > I don't think so, mainly because some of the things it is intended to be 
> > used for are not GPIOs. For instance, take the DAC mux I've described in 
> > the patch. It doesn't directly influence anything external to the SoC (i.e. 
> > it's certainly not a traditional GPIO in any sense). However, it does 
> > *indirectly* influence the SoC's behaviour by muxing the DAC internally 
> > between:
> > 
> > 0. VGA device exposed on the host PCIe bus
> > 1. The "Graphics CRT" controller
> > 2. VGA port A
> > 3. VGA port B
> 
> And this mux control is fixed in the SoC design?

This specific family of SoC (Aspeed) support those 4 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-12 Thread Benjamin Herrenschmidt
On Thu, 2018-07-12 at 09:11 -0600, Rob Herring wrote:
> On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
> > 
> > Hi Rob,
> > 
> > Thanks for the response.
> > 
> > On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> > > On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > > > Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
> > > > provide remote management of (primarily) server platforms. BMCs are
> > > > often tightly coupled to the platform in terms of behaviour and provide
> > > > many hardware features integral to booting and running the host system.
> > > > 
> > > > Some of these hardware features are simple, for example scratch
> > > > registers provided by the BMC that are exposed to both the host and the
> > > > BMC. In other cases there's a single bit switch to enable or disable
> > > > some of the provided functionality.
> > > > 
> > > > The documentation defines bindings for fields in registers that do not
> > > > integrate well into other driver models yet must be described to allow
> > > > the BMC kernel to assume control of these features.
> > > 
> > > So we'll get a new binding when that happens? That will break
> > > compatibility.
> > 
> > Can you please expand on this? I'm not following.
> 
> If we have a subsystem in the future, then there would likely be an
> associated binding which would be different. So if you update the DT,
> then old kernels won't work with it.

What kind of "subsystem" ? There is almost no way there could be one
for that sort of BMC tunables. We've look at several BMC chips out
there and requirements from several vendors, BIOS and system
manufacturers and it's all over the place.

> > I feel like this is an argument of tradition. Maybe people have
> > been dissuaded from doing so when they don't have a reasonable use-
> > case? I'm not saying that what I'm proposing is unquestionably
> > reasonable, but I don't want to dismiss it out of hand.
> 
> One of experience. The one that stands out is clock bindings.
> Initially we were doing a node per clock modelling which could end up
> being 100s of nodes and is difficult to get right (with DT being an
> ABI).
> 
> It comes up with system controller type blocks too that just have a
> bunch of random registers. Those change in every SoC and not in any
> controlled or ordered way that would make describing the individual
> sub-functions in DT worthwhile.

So what's the alternative ? Because without something like what we
propose, what's going to happen is /dev/mem ... that's what people do
today.

> > > A node per register bit doesn't scale.
> > 
> > It isn't meant to scale in terms of a single system. Using it
> > extensively is very likely wrong. Separately, register-bit-led does
> > pretty much the same thing. Doesn't the scale argument apply there?
> > Who is to stop me from attaching an insane number of LEDs to a
> > system?
> 
> Review.
> 
> If you look, register-bit-led is rarely used outside of some ARM, Ltd.
> boards. It's simply quite rare to have MMIO register bits that have a
> fixed function of LED control.

Well, same here, we hope to review what goes upstream to make it
reasonable. Otherwise it doens't matter. If a random vendor, let's say
IBM, chose to chip a system where they put an insane amount of cruft in
there, it will only affect those systems's BMC and the userspace stack
on it.

Thankfully that stack is OpenBMC and IBM is aiming at having their
device-tree's upstream, thus reviewed, thus it won't happen.

*Anything* can be abused. The point here is that we have a number,
thankfully rather small, maybe a dozen or two, of tunables that are
quite specific to a combination (system vendor, bmc vendor, system
model) which control a few HW features that essentially do *NOT* fit in
a subsystem.

For everything that does, we have created proper drivers (and are doing
more).


> > Obviously if there are lots of systems using it sparingly and
> > legitimately then maybe there's a scale issue, but isn't that just
> > a reality of different hardware designs? Whoever is implementing
> > support for the system is going to have to describe the hardware
> > one way or another.
> > 
> > > 
> > > Maybe this should be modelled using GPIO binding? There's a line there
> > > too as whether the signals are "general purpose" or not.
> > 
> > I don't think so, mainly because some of the things it is intended to be 
> > used for are not GPIOs. For instance, take the DAC mux I've described in 
> > the patch. It doesn't directly influence anything external to the SoC (i.e. 
> > it's certainly not a traditional GPIO in any sense). However, it does 
> > *indirectly* influence the SoC's behaviour by muxing the DAC internally 
> > between:
> > 
> > 0. VGA device exposed on the host PCIe bus
> > 1. The "Graphics CRT" controller
> > 2. VGA port A
> > 3. VGA port B
> 
> And this mux control is fixed in the SoC design?

This specific family of SoC (Aspeed) support those 4 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-12 Thread Rob Herring
On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
>
> Hi Rob,
>
> Thanks for the response.
>
> On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> > On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > > Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
> > > provide remote management of (primarily) server platforms. BMCs are
> > > often tightly coupled to the platform in terms of behaviour and provide
> > > many hardware features integral to booting and running the host system.
> > >
> > > Some of these hardware features are simple, for example scratch
> > > registers provided by the BMC that are exposed to both the host and the
> > > BMC. In other cases there's a single bit switch to enable or disable
> > > some of the provided functionality.
> > >
> > > The documentation defines bindings for fields in registers that do not
> > > integrate well into other driver models yet must be described to allow
> > > the BMC kernel to assume control of these features.
> >
> > So we'll get a new binding when that happens? That will break
> > compatibility.
>
> Can you please expand on this? I'm not following.

If we have a subsystem in the future, then there would likely be an
associated binding which would be different. So if you update the DT,
then old kernels won't work with it.


> > > Signed-off-by: Andrew Jeffery 
> > > ---
> > >
> > > Since RFC v1:
> > >
> > > * Add a commit message
> > > * Minor changes to documented labels
> > >
> > >  .../bindings/misc/bmc-misc-ctrl.txt   | 252 ++
> > >  MAINTAINERS   |   6 +
> > >  2 files changed, 258 insertions(+)
> > >  create mode 100644 
> > > Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt 
> > > b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > > new file mode 100644
> > > index ..2c869fcc7ef2
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > > @@ -0,0 +1,252 @@
> > > +BMC Miscellaneous Control Interfaces
> > > +
> > > +
> > > +Baseboard Management Controllers (BMCs) often have an array of hardware
> > > +features that need to be described but are awkward to sensibly expose.
> > > +
> > > +This bindings document provides a generic mechanism for describing such
> > > +features, covering read-only (RO), read-modify-write (RMW) and
> > > +write-1-set/write-1-clear (W1SC) semantics.
> >
> > If we wanted a generic mechanism for single register bits/fields in DT,
> > we'd have one already.
>
> I feel like this is an argument of tradition. Maybe people have been 
> dissuaded from doing so when they don't have a reasonable use-case? I'm not 
> saying that what I'm proposing is unquestionably reasonable, but I don't want 
> to dismiss it out of hand.

One of experience. The one that stands out is clock bindings.
Initially we were doing a node per clock modelling which could end up
being 100s of nodes and is difficult to get right (with DT being an
ABI).

It comes up with system controller type blocks too that just have a
bunch of random registers. Those change in every SoC and not in any
controlled or ordered way that would make describing the individual
sub-functions in DT worthwhile.

>
> > A node per register bit doesn't scale.
>
> It isn't meant to scale in terms of a single system. Using it extensively is 
> very likely wrong. Separately, register-bit-led does pretty much the same 
> thing. Doesn't the scale argument apply there? Who is to stop me from 
> attaching an insane number of LEDs to a system?

Review.

If you look, register-bit-led is rarely used outside of some ARM, Ltd.
boards. It's simply quite rare to have MMIO register bits that have a
fixed function of LED control.

> Obviously if there are lots of systems using it sparingly and legitimately 
> then maybe there's a scale issue, but isn't that just a reality of different 
> hardware designs? Whoever is implementing support for the system is going to 
> have to describe the hardware one way or another.
>
> >
> > Maybe this should be modelled using GPIO binding? There's a line there
> > too as whether the signals are "general purpose" or not.
>
> I don't think so, mainly because some of the things it is intended to be used 
> for are not GPIOs. For instance, take the DAC mux I've described in the 
> patch. It doesn't directly influence anything external to the SoC (i.e. it's 
> certainly not a traditional GPIO in any sense). However, it does *indirectly* 
> influence the SoC's behaviour by muxing the DAC internally between:
>
> 0. VGA device exposed on the host PCIe bus
> 1. The "Graphics CRT" controller
> 2. VGA port A
> 3. VGA port B

And this mux control is fixed in the SoC design?

>
> Maybe this could be modelled by pinmux, but then we still need some way to 
> expose the mux functions to userspace for selection 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-12 Thread Rob Herring
On Wed, Jul 11, 2018 at 6:54 PM Andrew Jeffery  wrote:
>
> Hi Rob,
>
> Thanks for the response.
>
> On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> > On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > > Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
> > > provide remote management of (primarily) server platforms. BMCs are
> > > often tightly coupled to the platform in terms of behaviour and provide
> > > many hardware features integral to booting and running the host system.
> > >
> > > Some of these hardware features are simple, for example scratch
> > > registers provided by the BMC that are exposed to both the host and the
> > > BMC. In other cases there's a single bit switch to enable or disable
> > > some of the provided functionality.
> > >
> > > The documentation defines bindings for fields in registers that do not
> > > integrate well into other driver models yet must be described to allow
> > > the BMC kernel to assume control of these features.
> >
> > So we'll get a new binding when that happens? That will break
> > compatibility.
>
> Can you please expand on this? I'm not following.

If we have a subsystem in the future, then there would likely be an
associated binding which would be different. So if you update the DT,
then old kernels won't work with it.


> > > Signed-off-by: Andrew Jeffery 
> > > ---
> > >
> > > Since RFC v1:
> > >
> > > * Add a commit message
> > > * Minor changes to documented labels
> > >
> > >  .../bindings/misc/bmc-misc-ctrl.txt   | 252 ++
> > >  MAINTAINERS   |   6 +
> > >  2 files changed, 258 insertions(+)
> > >  create mode 100644 
> > > Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt 
> > > b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > > new file mode 100644
> > > index ..2c869fcc7ef2
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > > @@ -0,0 +1,252 @@
> > > +BMC Miscellaneous Control Interfaces
> > > +
> > > +
> > > +Baseboard Management Controllers (BMCs) often have an array of hardware
> > > +features that need to be described but are awkward to sensibly expose.
> > > +
> > > +This bindings document provides a generic mechanism for describing such
> > > +features, covering read-only (RO), read-modify-write (RMW) and
> > > +write-1-set/write-1-clear (W1SC) semantics.
> >
> > If we wanted a generic mechanism for single register bits/fields in DT,
> > we'd have one already.
>
> I feel like this is an argument of tradition. Maybe people have been 
> dissuaded from doing so when they don't have a reasonable use-case? I'm not 
> saying that what I'm proposing is unquestionably reasonable, but I don't want 
> to dismiss it out of hand.

One of experience. The one that stands out is clock bindings.
Initially we were doing a node per clock modelling which could end up
being 100s of nodes and is difficult to get right (with DT being an
ABI).

It comes up with system controller type blocks too that just have a
bunch of random registers. Those change in every SoC and not in any
controlled or ordered way that would make describing the individual
sub-functions in DT worthwhile.

>
> > A node per register bit doesn't scale.
>
> It isn't meant to scale in terms of a single system. Using it extensively is 
> very likely wrong. Separately, register-bit-led does pretty much the same 
> thing. Doesn't the scale argument apply there? Who is to stop me from 
> attaching an insane number of LEDs to a system?

Review.

If you look, register-bit-led is rarely used outside of some ARM, Ltd.
boards. It's simply quite rare to have MMIO register bits that have a
fixed function of LED control.

> Obviously if there are lots of systems using it sparingly and legitimately 
> then maybe there's a scale issue, but isn't that just a reality of different 
> hardware designs? Whoever is implementing support for the system is going to 
> have to describe the hardware one way or another.
>
> >
> > Maybe this should be modelled using GPIO binding? There's a line there
> > too as whether the signals are "general purpose" or not.
>
> I don't think so, mainly because some of the things it is intended to be used 
> for are not GPIOs. For instance, take the DAC mux I've described in the 
> patch. It doesn't directly influence anything external to the SoC (i.e. it's 
> certainly not a traditional GPIO in any sense). However, it does *indirectly* 
> influence the SoC's behaviour by muxing the DAC internally between:
>
> 0. VGA device exposed on the host PCIe bus
> 1. The "Graphics CRT" controller
> 2. VGA port A
> 3. VGA port B

And this mux control is fixed in the SoC design?

>
> Maybe this could be modelled by pinmux, but then we still need some way to 
> expose the mux functions to userspace for selection 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-11 Thread Andrew Jeffery
Hi Rob,

Thanks for the response.

On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
> > provide remote management of (primarily) server platforms. BMCs are
> > often tightly coupled to the platform in terms of behaviour and provide
> > many hardware features integral to booting and running the host system.
> > 
> > Some of these hardware features are simple, for example scratch
> > registers provided by the BMC that are exposed to both the host and the
> > BMC. In other cases there's a single bit switch to enable or disable
> > some of the provided functionality.
> > 
> > The documentation defines bindings for fields in registers that do not
> > integrate well into other driver models yet must be described to allow
> > the BMC kernel to assume control of these features.
> 
> So we'll get a new binding when that happens? That will break 
> compatibility.

Can you please expand on this? I'm not following.

> 
> > 
> > Signed-off-by: Andrew Jeffery 
> > ---
> > 
> > Since RFC v1:
> > 
> > * Add a commit message
> > * Minor changes to documented labels
> > 
> >  .../bindings/misc/bmc-misc-ctrl.txt   | 252 ++
> >  MAINTAINERS   |   6 +
> >  2 files changed, 258 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt 
> > b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > new file mode 100644
> > index ..2c869fcc7ef2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > @@ -0,0 +1,252 @@
> > +BMC Miscellaneous Control Interfaces
> > +
> > +
> > +Baseboard Management Controllers (BMCs) often have an array of hardware
> > +features that need to be described but are awkward to sensibly expose.
> > +
> > +This bindings document provides a generic mechanism for describing such
> > +features, covering read-only (RO), read-modify-write (RMW) and
> > +write-1-set/write-1-clear (W1SC) semantics.
> 
> If we wanted a generic mechanism for single register bits/fields in DT, 
> we'd have one already.

I feel like this is an argument of tradition. Maybe people have been dissuaded 
from doing so when they don't have a reasonable use-case? I'm not saying that 
what I'm proposing is unquestionably reasonable, but I don't want to dismiss it 
out of hand.

> A node per register bit doesn't scale.

It isn't meant to scale in terms of a single system. Using it extensively is 
very likely wrong. Separately, register-bit-led does pretty much the same 
thing. Doesn't the scale argument apply there? Who is to stop me from attaching 
an insane number of LEDs to a system?

Obviously if there are lots of systems using it sparingly and legitimately then 
maybe there's a scale issue, but isn't that just a reality of different 
hardware designs? Whoever is implementing support for the system is going to 
have to describe the hardware one way or another.

> 
> Maybe this should be modelled using GPIO binding? There's a line there 
> too as whether the signals are "general purpose" or not.

I don't think so, mainly because some of the things it is intended to be used 
for are not GPIOs. For instance, take the DAC mux I've described in the patch. 
It doesn't directly influence anything external to the SoC (i.e. it's certainly 
not a traditional GPIO in any sense). However, it does *indirectly* influence 
the SoC's behaviour by muxing the DAC internally between:

0. VGA device exposed on the host PCIe bus
1. The "Graphics CRT" controller 
2. VGA port A
3. VGA port B

Maybe this could be modelled by pinmux, but then we still need some way to 
expose the mux functions to userspace for selection (userspace needs to 
transition arbitrarily between at least options 0 and 1 at runtime), at which 
point we haven't achieved much beyond adding a whole heap of infrastructure in 
the chain.

Given 0 and 1, maybe exposing attributes in relevant drivers would be 
reasonable, except 0 isn't exposed on the SoC's internal bus so there is no 
driver on the BMC-side to do so. Taking into account 2 and 3 are also purely 
hardware paths further dashes the idea, as the configuration doesn't really 
"belong" to the Graphics CRT device more than it belongs anywhere else, except 
for the fact that there isn't anywhere else to expose it.

Further, the BMC's kernel can't make the decision as to when to switch the mux 
as it knows nothing of the host's state. The BMC userspace is controlling the 
host's boot state and so *does* know when to flip the switch. Finally, the mux 
is in separate IP to the CRT or VGA blocks: It lives in the System Control Unit.

My current point of view is the DAC mux field is effectively its own device, 
and we need to control it from 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-11 Thread Andrew Jeffery
Hi Rob,

Thanks for the response.

On Thu, 12 Jul 2018, at 05:34, Rob Herring wrote:
> On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
> > provide remote management of (primarily) server platforms. BMCs are
> > often tightly coupled to the platform in terms of behaviour and provide
> > many hardware features integral to booting and running the host system.
> > 
> > Some of these hardware features are simple, for example scratch
> > registers provided by the BMC that are exposed to both the host and the
> > BMC. In other cases there's a single bit switch to enable or disable
> > some of the provided functionality.
> > 
> > The documentation defines bindings for fields in registers that do not
> > integrate well into other driver models yet must be described to allow
> > the BMC kernel to assume control of these features.
> 
> So we'll get a new binding when that happens? That will break 
> compatibility.

Can you please expand on this? I'm not following.

> 
> > 
> > Signed-off-by: Andrew Jeffery 
> > ---
> > 
> > Since RFC v1:
> > 
> > * Add a commit message
> > * Minor changes to documented labels
> > 
> >  .../bindings/misc/bmc-misc-ctrl.txt   | 252 ++
> >  MAINTAINERS   |   6 +
> >  2 files changed, 258 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt 
> > b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > new file mode 100644
> > index ..2c869fcc7ef2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > @@ -0,0 +1,252 @@
> > +BMC Miscellaneous Control Interfaces
> > +
> > +
> > +Baseboard Management Controllers (BMCs) often have an array of hardware
> > +features that need to be described but are awkward to sensibly expose.
> > +
> > +This bindings document provides a generic mechanism for describing such
> > +features, covering read-only (RO), read-modify-write (RMW) and
> > +write-1-set/write-1-clear (W1SC) semantics.
> 
> If we wanted a generic mechanism for single register bits/fields in DT, 
> we'd have one already.

I feel like this is an argument of tradition. Maybe people have been dissuaded 
from doing so when they don't have a reasonable use-case? I'm not saying that 
what I'm proposing is unquestionably reasonable, but I don't want to dismiss it 
out of hand.

> A node per register bit doesn't scale.

It isn't meant to scale in terms of a single system. Using it extensively is 
very likely wrong. Separately, register-bit-led does pretty much the same 
thing. Doesn't the scale argument apply there? Who is to stop me from attaching 
an insane number of LEDs to a system?

Obviously if there are lots of systems using it sparingly and legitimately then 
maybe there's a scale issue, but isn't that just a reality of different 
hardware designs? Whoever is implementing support for the system is going to 
have to describe the hardware one way or another.

> 
> Maybe this should be modelled using GPIO binding? There's a line there 
> too as whether the signals are "general purpose" or not.

I don't think so, mainly because some of the things it is intended to be used 
for are not GPIOs. For instance, take the DAC mux I've described in the patch. 
It doesn't directly influence anything external to the SoC (i.e. it's certainly 
not a traditional GPIO in any sense). However, it does *indirectly* influence 
the SoC's behaviour by muxing the DAC internally between:

0. VGA device exposed on the host PCIe bus
1. The "Graphics CRT" controller 
2. VGA port A
3. VGA port B

Maybe this could be modelled by pinmux, but then we still need some way to 
expose the mux functions to userspace for selection (userspace needs to 
transition arbitrarily between at least options 0 and 1 at runtime), at which 
point we haven't achieved much beyond adding a whole heap of infrastructure in 
the chain.

Given 0 and 1, maybe exposing attributes in relevant drivers would be 
reasonable, except 0 isn't exposed on the SoC's internal bus so there is no 
driver on the BMC-side to do so. Taking into account 2 and 3 are also purely 
hardware paths further dashes the idea, as the configuration doesn't really 
"belong" to the Graphics CRT device more than it belongs anywhere else, except 
for the fact that there isn't anywhere else to expose it.

Further, the BMC's kernel can't make the decision as to when to switch the mux 
as it knows nothing of the host's state. The BMC userspace is controlling the 
host's boot state and so *does* know when to flip the switch. Finally, the mux 
is in separate IP to the CRT or VGA blocks: It lives in the System Control Unit.

My current point of view is the DAC mux field is effectively its own device, 
and we need to control it from 

Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-11 Thread Benjamin Herrenschmidt
On Wed, 2018-07-11 at 14:04 -0600, Rob Herring wrote:
> On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
> > provide remote management of (primarily) server platforms. BMCs are
> > often tightly coupled to the platform in terms of behaviour and provide
> > many hardware features integral to booting and running the host system.
> > 
> > Some of these hardware features are simple, for example scratch
> > registers provided by the BMC that are exposed to both the host and the
> > BMC. In other cases there's a single bit switch to enable or disable
> > some of the provided functionality.
> > 
> > The documentation defines bindings for fields in registers that do not
> > integrate well into other driver models yet must be described to allow
> > the BMC kernel to assume control of these features.
> 
> So we'll get a new binding when that happens? That will break 
> compatibility.

What do you mean ? The binding provides a way to describe arbitrary
register fields and expose them to userspace. I'm not sure what you
mean by backward compatibility.

There is simply no way these things can be "abstracted" via some kind
of nice layered Linux subsystem of some sort. It's just a whole bunch
of knobs that control various things integral to the operation of the
host system. Andrew can provide a more precise list if you need to.

> 
> > 
> > Signed-off-by: Andrew Jeffery 
> > ---
> > 
> > Since RFC v1:
> > 
> > * Add a commit message
> > * Minor changes to documented labels
> > 
> >  .../bindings/misc/bmc-misc-ctrl.txt   | 252 ++
> >  MAINTAINERS   |   6 +
> >  2 files changed, 258 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt 
> > b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > new file mode 100644
> > index ..2c869fcc7ef2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > @@ -0,0 +1,252 @@
> > +BMC Miscellaneous Control Interfaces
> > +
> > +
> > +Baseboard Management Controllers (BMCs) often have an array of hardware
> > +features that need to be described but are awkward to sensibly expose.
> > +
> > +This bindings document provides a generic mechanism for describing such
> > +features, covering read-only (RO), read-modify-write (RMW) and
> > +write-1-set/write-1-clear (W1SC) semantics.
> 
> If we wanted a generic mechanism for single register bits/fields in DT, 
> we'd have one already. A node per register bit doesn't scale.

Register *field*. I think you are looking at this from the wrong angle.
This isn't about exposing 236821643 SoC registers in an embedded
product. This is about exposing a dozen or so tunables that don't
control the SoC from the perspective of the OS running on it, but
controls how various features of the SoC are exposed to the *host*
system. Examples could be which of the SoC internal PCIe devices
exposed to the host is enabled (the SoC can appear as a GPU, a BMC misc
device or both or neither, it can have a DMA controller or not,
etc...). Other examples are scratch registers that need to be set to
system specific values by userspace, which the BIOS of the host will
read to determine some configuration information. That sort of thing.

There isn't that many, scalability isn't a problem. Both the list of
registers of interest and what needs to go in them is very much
system/vendor specific. This is a way for the kernel to act as a
"conduit" that doesn't need to know the specifics of a given
system/vendor/BIOS combination.

Your response smells like yet another case of trying to apply one of
those general "blanket" rules to something where it's completely
unapplicable.

> Maybe this should be modelled using GPIO binding? There's a line there 
> too as whether the signals are "general purpose" or not.

GPIOs are binary values right ? These are arbitrary fields. We want
arbitrary fields. This is really needed Rob, otherwise we'll NEVER have
BMC support upstream. 

The other option will be a dozen random ad-hoc char-devs that would
have to be updated every time a new bit needs to be added or changed.

Ben.


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-11 Thread Benjamin Herrenschmidt
On Wed, 2018-07-11 at 14:04 -0600, Rob Herring wrote:
> On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> > Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
> > provide remote management of (primarily) server platforms. BMCs are
> > often tightly coupled to the platform in terms of behaviour and provide
> > many hardware features integral to booting and running the host system.
> > 
> > Some of these hardware features are simple, for example scratch
> > registers provided by the BMC that are exposed to both the host and the
> > BMC. In other cases there's a single bit switch to enable or disable
> > some of the provided functionality.
> > 
> > The documentation defines bindings for fields in registers that do not
> > integrate well into other driver models yet must be described to allow
> > the BMC kernel to assume control of these features.
> 
> So we'll get a new binding when that happens? That will break 
> compatibility.

What do you mean ? The binding provides a way to describe arbitrary
register fields and expose them to userspace. I'm not sure what you
mean by backward compatibility.

There is simply no way these things can be "abstracted" via some kind
of nice layered Linux subsystem of some sort. It's just a whole bunch
of knobs that control various things integral to the operation of the
host system. Andrew can provide a more precise list if you need to.

> 
> > 
> > Signed-off-by: Andrew Jeffery 
> > ---
> > 
> > Since RFC v1:
> > 
> > * Add a commit message
> > * Minor changes to documented labels
> > 
> >  .../bindings/misc/bmc-misc-ctrl.txt   | 252 ++
> >  MAINTAINERS   |   6 +
> >  2 files changed, 258 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt 
> > b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > new file mode 100644
> > index ..2c869fcc7ef2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> > @@ -0,0 +1,252 @@
> > +BMC Miscellaneous Control Interfaces
> > +
> > +
> > +Baseboard Management Controllers (BMCs) often have an array of hardware
> > +features that need to be described but are awkward to sensibly expose.
> > +
> > +This bindings document provides a generic mechanism for describing such
> > +features, covering read-only (RO), read-modify-write (RMW) and
> > +write-1-set/write-1-clear (W1SC) semantics.
> 
> If we wanted a generic mechanism for single register bits/fields in DT, 
> we'd have one already. A node per register bit doesn't scale.

Register *field*. I think you are looking at this from the wrong angle.
This isn't about exposing 236821643 SoC registers in an embedded
product. This is about exposing a dozen or so tunables that don't
control the SoC from the perspective of the OS running on it, but
controls how various features of the SoC are exposed to the *host*
system. Examples could be which of the SoC internal PCIe devices
exposed to the host is enabled (the SoC can appear as a GPU, a BMC misc
device or both or neither, it can have a DMA controller or not,
etc...). Other examples are scratch registers that need to be set to
system specific values by userspace, which the BIOS of the host will
read to determine some configuration information. That sort of thing.

There isn't that many, scalability isn't a problem. Both the list of
registers of interest and what needs to go in them is very much
system/vendor specific. This is a way for the kernel to act as a
"conduit" that doesn't need to know the specifics of a given
system/vendor/BIOS combination.

Your response smells like yet another case of trying to apply one of
those general "blanket" rules to something where it's completely
unapplicable.

> Maybe this should be modelled using GPIO binding? There's a line there 
> too as whether the signals are "general purpose" or not.

GPIOs are binary values right ? These are arbitrary fields. We want
arbitrary fields. This is really needed Rob, otherwise we'll NEVER have
BMC support upstream. 

The other option will be a dozen random ad-hoc char-devs that would
have to be updated every time a new bit needs to be added or changed.

Ben.


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-11 Thread Rob Herring
On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
> provide remote management of (primarily) server platforms. BMCs are
> often tightly coupled to the platform in terms of behaviour and provide
> many hardware features integral to booting and running the host system.
> 
> Some of these hardware features are simple, for example scratch
> registers provided by the BMC that are exposed to both the host and the
> BMC. In other cases there's a single bit switch to enable or disable
> some of the provided functionality.
> 
> The documentation defines bindings for fields in registers that do not
> integrate well into other driver models yet must be described to allow
> the BMC kernel to assume control of these features.

So we'll get a new binding when that happens? That will break 
compatibility.

> 
> Signed-off-by: Andrew Jeffery 
> ---
> 
> Since RFC v1:
> 
> * Add a commit message
> * Minor changes to documented labels
> 
>  .../bindings/misc/bmc-misc-ctrl.txt   | 252 ++
>  MAINTAINERS   |   6 +
>  2 files changed, 258 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt 
> b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> new file mode 100644
> index ..2c869fcc7ef2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> @@ -0,0 +1,252 @@
> +BMC Miscellaneous Control Interfaces
> +
> +
> +Baseboard Management Controllers (BMCs) often have an array of hardware
> +features that need to be described but are awkward to sensibly expose.
> +
> +This bindings document provides a generic mechanism for describing such
> +features, covering read-only (RO), read-modify-write (RMW) and
> +write-1-set/write-1-clear (W1SC) semantics.

If we wanted a generic mechanism for single register bits/fields in DT, 
we'd have one already. A node per register bit doesn't scale.

Maybe this should be modelled using GPIO binding? There's a line there 
too as whether the signals are "general purpose" or not.

Rob


Re: [RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-11 Thread Rob Herring
On Wed, Jul 11, 2018 at 03:01:19PM +0930, Andrew Jeffery wrote:
> Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
> provide remote management of (primarily) server platforms. BMCs are
> often tightly coupled to the platform in terms of behaviour and provide
> many hardware features integral to booting and running the host system.
> 
> Some of these hardware features are simple, for example scratch
> registers provided by the BMC that are exposed to both the host and the
> BMC. In other cases there's a single bit switch to enable or disable
> some of the provided functionality.
> 
> The documentation defines bindings for fields in registers that do not
> integrate well into other driver models yet must be described to allow
> the BMC kernel to assume control of these features.

So we'll get a new binding when that happens? That will break 
compatibility.

> 
> Signed-off-by: Andrew Jeffery 
> ---
> 
> Since RFC v1:
> 
> * Add a commit message
> * Minor changes to documented labels
> 
>  .../bindings/misc/bmc-misc-ctrl.txt   | 252 ++
>  MAINTAINERS   |   6 +
>  2 files changed, 258 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt 
> b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> new file mode 100644
> index ..2c869fcc7ef2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
> @@ -0,0 +1,252 @@
> +BMC Miscellaneous Control Interfaces
> +
> +
> +Baseboard Management Controllers (BMCs) often have an array of hardware
> +features that need to be described but are awkward to sensibly expose.
> +
> +This bindings document provides a generic mechanism for describing such
> +features, covering read-only (RO), read-modify-write (RMW) and
> +write-1-set/write-1-clear (W1SC) semantics.

If we wanted a generic mechanism for single register bits/fields in DT, 
we'd have one already. A node per register bit doesn't scale.

Maybe this should be modelled using GPIO binding? There's a line there 
too as whether the signals are "general purpose" or not.

Rob


[RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-10 Thread Andrew Jeffery
Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
provide remote management of (primarily) server platforms. BMCs are
often tightly coupled to the platform in terms of behaviour and provide
many hardware features integral to booting and running the host system.

Some of these hardware features are simple, for example scratch
registers provided by the BMC that are exposed to both the host and the
BMC. In other cases there's a single bit switch to enable or disable
some of the provided functionality.

The documentation defines bindings for fields in registers that do not
integrate well into other driver models yet must be described to allow
the BMC kernel to assume control of these features.

Signed-off-by: Andrew Jeffery 
---

Since RFC v1:

* Add a commit message
* Minor changes to documented labels

 .../bindings/misc/bmc-misc-ctrl.txt   | 252 ++
 MAINTAINERS   |   6 +
 2 files changed, 258 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt

diff --git a/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt 
b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
new file mode 100644
index ..2c869fcc7ef2
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
@@ -0,0 +1,252 @@
+BMC Miscellaneous Control Interfaces
+
+
+Baseboard Management Controllers (BMCs) often have an array of hardware
+features that need to be described but are awkward to sensibly expose.
+
+This bindings document provides a generic mechanism for describing such
+features, covering read-only (RO), read-modify-write (RMW) and
+write-1-set/write-1-clear (W1SC) semantics.
+
+All uses of bmc-misc-ctrl must be documented under Valid Uses below.
+
+The bindings are similar in nature to register-bit-led.
+
+Required Properties
+---
+
+compatible:Must be "bmc-misc-ctrl"
+offset:A one or three cell property describing the registers
+   associated with the field.
+
+   If the optional property 'set-clear' is not present then the
+   node describes a register with read-modify-write semantics. The
+   offset property has one cell describing the register of
+   interest.
+
+   If the optional property 'set-clear' is present then the node
+   describes a register set that together implement read,
+   write-1-set and write-1-clear semantics. The offset property
+   must be three cells, the first is the address of the register
+   to read from, the second the write-1-set register and the third
+   write-1-clear.
+
+mask:  A mask whose set bits represent the bits of the field.
+label: The name of the field
+
+Optional Properties
+---
+
+read-only: Define a read-only field (RMW/W1SC irrelevant).
+set-clear: Define whether the field exists in a RMW or W1SC register set
+default-value: Single cell applicable to RMW. The field will be updated to the
+   cell's value.
+default-set:   For W1SC, set all bits in the field
+default-clear: For W1SC, clear all bits in the field
+
+Valid Uses
+--
+
+Description:   Control bit for switching the video display DAC mux between
+   host VGA and BMC CRT mode
+Machines:  aspeed,ast2500
+Parent:compatible = "aspeed,ast2500-scu", "syscon", 
"simple-mfd";
+Node:
+   field@2c.16 {
+   compatible = "bmc-misc-ctrl";
+   offset = <0x2c>;
+   mask = <0x0003>;
+   label = "dac-mux";
+   };
+
+Description:   Host VGA scratch registers
+Machines:  aspeed,ast2500
+Parent:compatible = "aspeed,ast2500-scu", "syscon", 
"simple-mfd";
+Node:
+   field@50.0 {
+   compatible = "bmc-misc-ctrl";
+   offset = <0x50>;
+   mask = <0x>;
+   label = "vga0";
+   read-only;
+   };
+
+   field@54.0 {
+   compatible = "bmc-misc-ctrl";
+   offset = <0x54>;
+   mask = <0x>;
+   label = "vga1";
+   read-only;
+   };
+
+   field@58.0 {
+   compatible = "bmc-misc-ctrl";
+   offset = <0x58>;
+   mask = <0x>;
+   label = "vga2";
+   read-only;
+   };
+
+   field@5c.0 {
+   compatible = "bmc-misc-ctrl";
+   offset = <0x5c>;
+   mask = <0x>;
+   label = "vga3";
+   read-only;
+

[RFC PATCH v2 1/4] dt-bindings: misc: Add bindings for misc. BMC control fields

2018-07-10 Thread Andrew Jeffery
Baseboard Management Controllers (BMCs) are embedded SoCs that exist to
provide remote management of (primarily) server platforms. BMCs are
often tightly coupled to the platform in terms of behaviour and provide
many hardware features integral to booting and running the host system.

Some of these hardware features are simple, for example scratch
registers provided by the BMC that are exposed to both the host and the
BMC. In other cases there's a single bit switch to enable or disable
some of the provided functionality.

The documentation defines bindings for fields in registers that do not
integrate well into other driver models yet must be described to allow
the BMC kernel to assume control of these features.

Signed-off-by: Andrew Jeffery 
---

Since RFC v1:

* Add a commit message
* Minor changes to documented labels

 .../bindings/misc/bmc-misc-ctrl.txt   | 252 ++
 MAINTAINERS   |   6 +
 2 files changed, 258 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt

diff --git a/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt 
b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
new file mode 100644
index ..2c869fcc7ef2
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/bmc-misc-ctrl.txt
@@ -0,0 +1,252 @@
+BMC Miscellaneous Control Interfaces
+
+
+Baseboard Management Controllers (BMCs) often have an array of hardware
+features that need to be described but are awkward to sensibly expose.
+
+This bindings document provides a generic mechanism for describing such
+features, covering read-only (RO), read-modify-write (RMW) and
+write-1-set/write-1-clear (W1SC) semantics.
+
+All uses of bmc-misc-ctrl must be documented under Valid Uses below.
+
+The bindings are similar in nature to register-bit-led.
+
+Required Properties
+---
+
+compatible:Must be "bmc-misc-ctrl"
+offset:A one or three cell property describing the registers
+   associated with the field.
+
+   If the optional property 'set-clear' is not present then the
+   node describes a register with read-modify-write semantics. The
+   offset property has one cell describing the register of
+   interest.
+
+   If the optional property 'set-clear' is present then the node
+   describes a register set that together implement read,
+   write-1-set and write-1-clear semantics. The offset property
+   must be three cells, the first is the address of the register
+   to read from, the second the write-1-set register and the third
+   write-1-clear.
+
+mask:  A mask whose set bits represent the bits of the field.
+label: The name of the field
+
+Optional Properties
+---
+
+read-only: Define a read-only field (RMW/W1SC irrelevant).
+set-clear: Define whether the field exists in a RMW or W1SC register set
+default-value: Single cell applicable to RMW. The field will be updated to the
+   cell's value.
+default-set:   For W1SC, set all bits in the field
+default-clear: For W1SC, clear all bits in the field
+
+Valid Uses
+--
+
+Description:   Control bit for switching the video display DAC mux between
+   host VGA and BMC CRT mode
+Machines:  aspeed,ast2500
+Parent:compatible = "aspeed,ast2500-scu", "syscon", 
"simple-mfd";
+Node:
+   field@2c.16 {
+   compatible = "bmc-misc-ctrl";
+   offset = <0x2c>;
+   mask = <0x0003>;
+   label = "dac-mux";
+   };
+
+Description:   Host VGA scratch registers
+Machines:  aspeed,ast2500
+Parent:compatible = "aspeed,ast2500-scu", "syscon", 
"simple-mfd";
+Node:
+   field@50.0 {
+   compatible = "bmc-misc-ctrl";
+   offset = <0x50>;
+   mask = <0x>;
+   label = "vga0";
+   read-only;
+   };
+
+   field@54.0 {
+   compatible = "bmc-misc-ctrl";
+   offset = <0x54>;
+   mask = <0x>;
+   label = "vga1";
+   read-only;
+   };
+
+   field@58.0 {
+   compatible = "bmc-misc-ctrl";
+   offset = <0x58>;
+   mask = <0x>;
+   label = "vga2";
+   read-only;
+   };
+
+   field@5c.0 {
+   compatible = "bmc-misc-ctrl";
+   offset = <0x5c>;
+   mask = <0x>;
+   label = "vga3";
+   read-only;
+