Re: [PATCH 11/11] ipmi: Add NVIDIA LSTP IPMI BMC driver

[Corey Minyard]

On Fri, Oct 16, 2026 at 11:19:14AM +0300, Adrian Ambrozewicz wrote:
> Hi Corey,
> 
> Thanks for the review.
> 
> On Mon, Oct 12, 2026 at 04:10:32PM -0500, Corey Minyard wrote:
> > Can you name the file usb_lstp.c, or something like that, to match
> > the other BMC side devices?
> 
> Sure. Would lstp_bmc.c work, next to ssif_bmc.c and kcs_bmc.c?
> Otherwise usb_lstp.c it is.

Either name works.  Just want it to be consistent, and your suggestion
is better than mine.

> 
> >> +static DEFINE_IDA(lstp_ipmi_ida);
> >
> > Wouldn't you want to assign the numbers in the device tree so they
> > always have the same number?
> 
> I'd rather keep them out of DT. The binding rules ask not to add
> instance index properties or custom aliases
> (Documentation/devicetree/bindings/writing-bindings.rst): DT describes
> the hardware, not how Linux numbers its device nodes. Our out-of-tree
> driver used to take the node name from a "label" property, and we
> dropped it for that reason.
> 
> A stable name comes from udev instead. The node's parent is the
> channel's auxiliary device under the USB device, so a rule matching the
> USB port path and the channel can add a symlink with whatever name the
> BMC userspace expects.

Yes, that's the right way to do this, since udev has something to
differentiate the device.

> 
> > It seems a little strange to set this up here and then register it in
> > the next section.  Why not set it up right before the register?
> >
> > The breakup of the probe function seems a little unnatural.  Not a
> > bit deal, but it makes the code a little harder to follow.
> 
> Agreed. The next version fills in the miscdevice right before
> misc_register() and folds the init and start helpers into one probe
> function.

Going a little higher level, I'm wondering if there is any advantage to
having this in the kernel.  Since it's USB you could do this all in
userland.  There's no kernel subsystem that this would tie in to.

I suppose the big advantage is it looks like the other BMC interfaces.
But you can maintain it more easily if it's in userland.  I don't know a
lot about USB, though, and how all that works.  Just something to
think about, perhaps.

-corey

> 
> Thanks,
> Adrian