Re: [PATCH linux dev-5.1 v1 1/1] ARM: dts: aspeed: Add s7106 BMC Machine

[Andrew Jeffery]

Hi Ray,

On Wed, 29 May 2019, at 19:45, ray.lue at mic.com.tw wrote:
> Hi Andrew, 
> 
> Thank you for reviewing this dts, I will test and update it based on 
> your recommendation. 

Great!

> 
> Regarding the "slave-mqueue"
> 
> > 
> > > +    status = "okay";
> > > +    spsfw1 at 10 {
> > > +        compatible = "slave-mqueue";
> > 
> > This compatible isn't documented anywhere. Do you have a bindings
> > document for it? We'll need that before we can merge the devicetree.
> 
> it's an Intel developed driver for IPMB, but yet upstream to kernel. 
> 
> https://lore.kernel.org/patchwork/patch/934565/
> https://github.com/Intel-BMC/openbmc/blob/a7715486507e75e4a7cee843a48067b15595defa/meta-openbmc-mods/meta-common/recipes-kernel/linux/linux-aspeed/0019-Add-I2C-IPMB-support.patch
> 
> However, it's a necessary driver for OpenBMC ipmbbridge service like below.  
> 
> https://github.com/openbmc/ipmbbridge/blob/master/ipmb-channels.json
> {
>   "channels": [
>     {
>       "type": "me",
>       "master-path": "/dev/i2c-5",
>       "slave-path": "/sys/bus/i2c/devices/5-1010/slave-mqueue",
>       "bmc-addr": 32,
>       "remote-addr": 44
>     },
>     {
>       "type": "ipmb",
>       "master-path": "/dev/i2c-0",
>       "slave-path": "/sys/bus/i2c/devices/0-1010/slave-mqueue",
>       "bmc-addr": 32,
>       "remote-addr": 88
>     }
>   ]
> }
> 
> The IPMB to ME is a very important channel for Intel based platform to 
> retrieve critical sensors like CPU, DIMM, IO ...etc. What's your 
> recommendation? should I remove it from dts until this driver approved 
> by upstream? 

My recommendation here is to split the patch. In the first patch, add everything
you have here except for the IPMB node. Then in the following patch, add the
IPMB node. This way you can send the first patch upstream while you harass
Intel to upstream the IPMB bindings and driver.

Cheers,

Andrew