Re: [EXT] Re: [PATCH v2 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K dpi driver

[Greg KH]

On Mon, Feb 19, 2024 at 05:03:38AM +0000, Vamsi Krishna Attunuru wrote:
> > -----Original Message-----
> > From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Sent: Saturday, February 17, 2024 1:44 PM
> > To: Vamsi Krishna Attunuru <vattunuru@xxxxxxxxxxx>
> > Cc: arnd@xxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > Subject: [EXT] Re: [PATCH v2 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K
> > dpi driver

Why is this here?

> > External Email
> > 
> > ----------------------------------------------------------------------
> > On Fri, Feb 16, 2024 at 02:32:25AM -0800, Vamsi Attunuru wrote:
> > > Adds a driver for Marvell CN10K DPI(DMA Engine) device's physical
> > > function which initializes DPI DMA hardware's global configuration and
> > > enables hardware mailbox channels between physical function (PF) and
> > > it's virtual functions (VF). VF device drivers (User space drivers)
> > > use this hw mailbox to communicate any required device configuration
> > > on it's respective VF device. Accordingly, this DPI PF driver provision the VF
> > device resources.
> > >
> > > At the hardware level, the DPI physical function (PF) acts as a
> > > management interface to setup the VF device resources, VF devices are
> > > only provisioned to handle or control the actual DMA Engine's data transfer
> > capabilities.
> > >
> > > Signed-off-by: Vamsi Attunuru <vattunuru@xxxxxxxxxxx>
> > > ---
> > >
> > > Changes V1 -> V2:
> > > - Fixed return values and busy-wait loops
> > > - Merged .h file into .c file
> > > - Fixed directory structure
> > > - Removed module params
> > > - Registered the device as misc device
> > 
> > Why register as a misc device if you don't actually use it at all?  That feels
> > pointless and extra code and confusion for everyone as you have created a
> > device node in the system that will just fail all operations made on it.
> > 
> > confused,
> > 
> 
> Module params are removed in V2, planning to use device node to pass the device configuration tuning parameters and other ops, can you please also share other v2 review comments, I will plan to address it in V3.

How is a reviewer supposed to know that you are going to use unused code
sometime in the future when you do not say that here?

What would you do if you had to review such code?  You would stop there
and wait for it all to make sense.

Please try to get some internal review next time before you send out
your next version so that you don't waste other's time reviewing dead
code like this.

thanks,

greg k-h