Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

From: 'Greg KH'
Date: Wed Dec 06 2017 - 02:56:00 EST


On Wed, Dec 06, 2017 at 06:01:37AM +0000, Dhaval Rajeshbhai Shah wrote:
> Hi Greg k-h,
>
> Thanks a lot for the review.
>
> Replies inline.

As they should be, perhaps you need a better email client :)

>
> > +config XILINX_VCU
> > + tristate "Xilinx VCU Init"
> > + default n
>
> That's always the default, no need for this.
> [Dhaval ] : I will remove that.

This style of replying is very odd, please use the normal format in the
future.

> > + help
> > + Driver for the Xilinx VCU Init based on the logicoreIP.
>
> You need a lot more help text here to explain what this driver is, what it is for, and who would need it.
> [Dhaval ] : I will provide more help text to provide more help on driver.
>
> Also, why is this a misc driver?
> [Dhaval ] : this driver is for the logicoreIP which is created to support the Processing system and Programmable logic isolation and to provide the clock related information. So this is not a VCU driver and but just a intermediate driver which supports logicoreIP. That's why no subsystem for this.

Then you need to explain this a lot better, posting a random driver
for submission that is expected to be used by another one isn't ok.
Post the whole patch series please, we do not add infrastructure to the
kernel that is not used right then.

thanks,

greg k-h