linux-kernel - Re: [PATCH v7 1/3] mfd: ls2kbmc: Introduce Loongson-2K BMC core driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

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

Message-ID: <20250718140206.GI11056@google.com>
Date: Fri, 18 Jul 2025 15:02:06 +0100
From: Lee Jones <lee@...nel.org>
To: Binbin Zhou <zhoubb.aaron@...il.com>
Cc: Binbin Zhou <zhoubinbin@...ngson.cn>,
	Huacai Chen <chenhuacai@...ngson.cn>,
	Corey Minyard <minyard@....org>,
	Huacai Chen <chenhuacai@...nel.org>,
	Xuerui Wang <kernel@...0n.name>, loongarch@...ts.linux.dev,
	linux-kernel@...r.kernel.org,
	openipmi-developer@...ts.sourceforge.net, jeffbai@...c.io,
	kexybiscuit@...c.io, wangyao@...ote.com,
	Chong Qiao <qiaochong@...ngson.cn>,
	Corey Minyard <corey@...yard.net>
Subject: Re: [PATCH v7 1/3] mfd: ls2kbmc: Introduce Loongson-2K BMC core
 driver

On Fri, 11 Jul 2025, Binbin Zhou wrote:

> Hi Lee:
> 
> Thanks for your review.
> 
> On Thu, Jul 10, 2025 at 5:56 PM Lee Jones <lee@...nel.org> wrote:
> >
> > On Fri, 04 Jul 2025, Binbin Zhou wrote:
> >
> > > The Loongson-2K Board Management Controller provides an PCIe interface
> > > to the host to access the feature implemented in the BMC.
> > >
> > > The BMC is assembled on a server similar to the server machine with
> > > Loongson-3 CPU. It supports multiple sub-devices like DRM and IPMI.
> > >
> > > Co-developed-by: Chong Qiao <qiaochong@...ngson.cn>
> > > Signed-off-by: Chong Qiao <qiaochong@...ngson.cn>
> > > Reviewed-by: Huacai Chen <chenhuacai@...ngson.cn>
> > > Acked-by: Corey Minyard <corey@...yard.net>
> > > Signed-off-by: Binbin Zhou <zhoubinbin@...ngson.cn>
> > > ---
> > >  MAINTAINERS                 |   6 ++
> > >  drivers/mfd/Kconfig         |  13 +++
> > >  drivers/mfd/Makefile        |   2 +
> > >  drivers/mfd/ls2k-bmc-core.c | 156 ++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 177 insertions(+)

[...]

> > > +static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
> > > +{
> > > +     struct simplefb_platform_data pd;
> > > +     resource_size_t base;
> > > +     int ret;
> > > +
> > > +     ret = pci_enable_device(dev);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = ls2k_bmc_parse_mode(dev, &pd);
> > > +     if (ret)
> > > +             goto disable_pci;
> > > +
> > > +     ls2k_bmc_cells[0].platform_data = &pd;
> > > +     ls2k_bmc_cells[0].pdata_size = sizeof(pd);
> >
> > This is fragile.
> >
> > Please identify the elements in ls2k_bmc_cells and use it to index here.
> >
> > See: `static struct mfd_cell as3711_subdevs`
> 
> How about this:
> 
> enum {
>         LS2K_BMC_DISPLAY,
>         LS2k_BMC_IPMI0,
>         LS2k_BMC_IPMI1,
>         LS2k_BMC_IPMI2,
>         LS2k_BMC_IPMI3,
>         LS2k_BMC_IPMI4,
> };
> 
> static struct mfd_cell ls2k_bmc_cells[] = {
>         [LS2K_BMC_DISPLAY] = {
>                 .name = "simple-framebuffer",
>                 .num_resources = ARRAY_SIZE(ls2k_display_resources),
>                 .resources = ls2k_display_resources
>         },
>         [LS2k_BMC_IPMI0] = {
>                 .name = "ls2k-ipmi-si",
>                 .num_resources = ARRAY_SIZE(ls2k_ipmi0_resources),
>                 .resources = ls2k_ipmi0_resources
>         },
>         [LS2k_BMC_IPMI1] = {
>                 .name = "ls2k-ipmi-si",
>                 .num_resources = ARRAY_SIZE(ls2k_ipmi1_resources),
>                 .resources = ls2k_ipmi1_resources
>         },
>         [LS2k_BMC_IPMI2] = {
>                 .name = "ls2k-ipmi-si",
>                 .num_resources = ARRAY_SIZE(ls2k_ipmi2_resources),
>                 .resources = ls2k_ipmi2_resources
>         },
>         [LS2k_BMC_IPMI3] = {
>                 .name = "ls2k-ipmi-si",
>                 .num_resources = ARRAY_SIZE(ls2k_ipmi3_resources),
>                 .resources = ls2k_ipmi3_resources
>         },
>         [LS2k_BMC_IPMI4] = {
>                 .name = "ls2k-ipmi-si",
>                 .num_resources = ARRAY_SIZE(ls2k_ipmi4_resources),
>                 .resources = ls2k_ipmi4_resources
>         },
> };
> 
> and
> 
>         ls2k_bmc_cells[LS2K_BMC_DISPLAY].platform_data = &pd;
>         ls2k_bmc_cells[LS2K_BMC_DISPLAY].pdata_size = sizeof(pd);

Yes, that's it.

-- 
Lee Jones [李琼斯]