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

[Greg Kroah-Hartman]

On Thu, Dec 07, 2017 at 10:30:45AM +, Dhaval Rajeshbhai Shah wrote:
> 
> 
> > -Original Message-
> > From: Philippe Ombredanne [mailto:pombreda...@nexb.com]
> > Sent: Thursday, December 07, 2017 2:07 AM
> > To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>
> > Cc: Arnd Bergmann <a...@arndb.de>; Greg Kroah-Hartman
> > <gre...@linuxfoundation.org>; LKML <linux-kernel@vger.kernel.org>;
> > michal.si...@xilinx.com; Hyun Kwon <hy...@xilinx.com>; Dhaval
> > Rajeshbhai Shah <ds...@xilinx.com>
> > Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> > logicoreIP init driver
> > 
> > Daval,
> > 
> > On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <dhaval.s...@xilinx.com>
> > wrote:
> > > Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> > > created. This driver will provide the api which can be used by the
> > > encoder and decoder driver to get the configured value.
> > >
> > > Signed-off-by: Dhaval Shah <ds...@xilinx.com>
> > []
> > > diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> > > file mode 100644 index 000..373f7f9
> > > --- /dev/null
> > > +++ b/drivers/misc/xlnx_vcu.c
> > > @@ -0,0 +1,664 @@
> > > +/*
> > > + * Xilinx VCU Init
> > > + *
> > > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > > + *
> > > + * Contacts   Dhaval Shah <ds...@xilinx.com>
> > > + *
> > > + * SPDX-License-Identifier: GPL-2.0
> > > + */
> > 
> > The SPDX id should be on the first line with a C++ // style comment instead 
> > as
> > requested by Linus.
> I will take care of this Changes as also suggested by the Greg k-h as well 
> once get the ACK for the remaining reply from the Greg k-h.

Don't wait for me, my queue is really long, if you have fixes that you
know you need to make, make them!

greg k-h

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

[Greg Kroah-Hartman]

On Thu, Dec 07, 2017 at 10:30:45AM +, Dhaval Rajeshbhai Shah wrote:
> 
> 
> > -Original Message-
> > From: Philippe Ombredanne [mailto:pombreda...@nexb.com]
> > Sent: Thursday, December 07, 2017 2:07 AM
> > To: Dhaval Rajeshbhai Shah 
> > Cc: Arnd Bergmann ; Greg Kroah-Hartman
> > ; LKML ;
> > michal.si...@xilinx.com; Hyun Kwon ; Dhaval
> > Rajeshbhai Shah 
> > Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> > logicoreIP init driver
> > 
> > Daval,
> > 
> > On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah 
> > wrote:
> > > Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> > > created. This driver will provide the api which can be used by the
> > > encoder and decoder driver to get the configured value.
> > >
> > > Signed-off-by: Dhaval Shah 
> > []
> > > diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> > > file mode 100644 index 000..373f7f9
> > > --- /dev/null
> > > +++ b/drivers/misc/xlnx_vcu.c
> > > @@ -0,0 +1,664 @@
> > > +/*
> > > + * Xilinx VCU Init
> > > + *
> > > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > > + *
> > > + * Contacts   Dhaval Shah 
> > > + *
> > > + * SPDX-License-Identifier: GPL-2.0
> > > + */
> > 
> > The SPDX id should be on the first line with a C++ // style comment instead 
> > as
> > requested by Linus.
> I will take care of this Changes as also suggested by the Greg k-h as well 
> once get the ACK for the remaining reply from the Greg k-h.

Don't wait for me, my queue is really long, if you have fixes that you
know you need to make, make them!

greg k-h

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

[Dhaval Rajeshbhai Shah]

> -Original Message-
> From: Philippe Ombredanne [mailto:pombreda...@nexb.com]
> Sent: Thursday, December 07, 2017 2:07 AM
> To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>
> Cc: Arnd Bergmann <a...@arndb.de>; Greg Kroah-Hartman
> <gre...@linuxfoundation.org>; LKML <linux-kernel@vger.kernel.org>;
> michal.si...@xilinx.com; Hyun Kwon <hy...@xilinx.com>; Dhaval
> Rajeshbhai Shah <ds...@xilinx.com>
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> logicoreIP init driver
> 
> Daval,
> 
> On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <dhaval.s...@xilinx.com>
> wrote:
> > Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> > created. This driver will provide the api which can be used by the
> > encoder and decoder driver to get the configured value.
> >
> > Signed-off-by: Dhaval Shah <ds...@xilinx.com>
> []
> > diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> > file mode 100644 index 000..373f7f9
> > --- /dev/null
> > +++ b/drivers/misc/xlnx_vcu.c
> > @@ -0,0 +1,664 @@
> > +/*
> > + * Xilinx VCU Init
> > + *
> > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > + *
> > + * Contacts   Dhaval Shah <ds...@xilinx.com>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0
> > + */
> 
> The SPDX id should be on the first line with a C++ // style comment instead as
> requested by Linus.
I will take care of this Changes as also suggested by the Greg k-h as well once 
get the ACK for the remaining reply from the Greg k-h.

Thanks & Regards,
Dhaval
> --
> Cordially
> Philippe Ombredanne

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

[Dhaval Rajeshbhai Shah]

> -Original Message-
> From: Philippe Ombredanne [mailto:pombreda...@nexb.com]
> Sent: Thursday, December 07, 2017 2:07 AM
> To: Dhaval Rajeshbhai Shah 
> Cc: Arnd Bergmann ; Greg Kroah-Hartman
> ; LKML ;
> michal.si...@xilinx.com; Hyun Kwon ; Dhaval
> Rajeshbhai Shah 
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> logicoreIP init driver
> 
> Daval,
> 
> On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah 
> wrote:
> > Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> > created. This driver will provide the api which can be used by the
> > encoder and decoder driver to get the configured value.
> >
> > Signed-off-by: Dhaval Shah 
> []
> > diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> > file mode 100644 index 000..373f7f9
> > --- /dev/null
> > +++ b/drivers/misc/xlnx_vcu.c
> > @@ -0,0 +1,664 @@
> > +/*
> > + * Xilinx VCU Init
> > + *
> > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > + *
> > + * Contacts   Dhaval Shah 
> > + *
> > + * SPDX-License-Identifier: GPL-2.0
> > + */
> 
> The SPDX id should be on the first line with a C++ // style comment instead as
> requested by Linus.
I will take care of this Changes as also suggested by the Greg k-h as well once 
get the ACK for the remaining reply from the Greg k-h.

Thanks & Regards,
Dhaval
> --
> Cordially
> Philippe Ombredanne

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

[Philippe Ombredanne]

Daval,

On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah  wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> created. This driver will provide the api which can be used
> by the encoder and decoder driver to get the configured value.
>
> Signed-off-by: Dhaval Shah 
[]
> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c
> new file mode 100644
> index 000..373f7f9
> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,664 @@
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts   Dhaval Shah 
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */

The SPDX id should be on the first line with a C++ // style comment
instead as requested by Linus.
-- 
Cordially
Philippe Ombredanne

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

[Philippe Ombredanne]

Daval,

On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah  wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> created. This driver will provide the api which can be used
> by the encoder and decoder driver to get the configured value.
>
> Signed-off-by: Dhaval Shah 
[]
> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c
> new file mode 100644
> index 000..373f7f9
> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,664 @@
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts   Dhaval Shah 
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */

The SPDX id should be on the first line with a C++ // style comment
instead as requested by Linus.
-- 
Cordially
Philippe Ombredanne

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

[Dhaval Rajeshbhai Shah]

> -Original Message-
> From: 'Greg KH' [mailto:gre...@linuxfoundation.org]
> Sent: Wednesday, December 06, 2017 1:16 AM
> To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>
> Cc: a...@arndb.de; linux-kernel@vger.kernel.org; michal.si...@xilinx.com;
> Hyun Kwon <hy...@xilinx.com>
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> logicoreIP init driver
> 
> On Wed, Dec 06, 2017 at 09:05:51AM +, Dhaval Rajeshbhai Shah wrote:
> > > 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.
> > Can I remove the export api and header file and make this driver generic
> for the logicoreIP? No other driver will depend on that. This will help us to
> remove the isolation between the Programmable system and Programmable
> logic by configuring the logicoreIp register set.
> 
> I don't know, let's see what that patch submission looks like.  If it doesn't 
> do
> anything, that's not really a good idea :)
>
This will do the isolation enable and removal related stuff along with the set 
the proper frequency based on the clock information get from the logicoreIp. In 
this way, This is very helpful driver.
 
> What's wrong with just submitting your entire driver series?  Why break it up
> like this?  That's normally not a good idea, you want us to review it in a
> "whole" idea, not piece-meal.
Codecs are owned by different team. when they will submit their code they will 
extend this driver.

Thanks,
Dhaval
> 
> thanks,
> 
> greg k-h

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

[Dhaval Rajeshbhai Shah]

> -Original Message-
> From: 'Greg KH' [mailto:gre...@linuxfoundation.org]
> Sent: Wednesday, December 06, 2017 1:16 AM
> To: Dhaval Rajeshbhai Shah 
> Cc: a...@arndb.de; linux-kernel@vger.kernel.org; michal.si...@xilinx.com;
> Hyun Kwon 
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> logicoreIP init driver
> 
> On Wed, Dec 06, 2017 at 09:05:51AM +, Dhaval Rajeshbhai Shah wrote:
> > > 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.
> > Can I remove the export api and header file and make this driver generic
> for the logicoreIP? No other driver will depend on that. This will help us to
> remove the isolation between the Programmable system and Programmable
> logic by configuring the logicoreIp register set.
> 
> I don't know, let's see what that patch submission looks like.  If it doesn't 
> do
> anything, that's not really a good idea :)
>
This will do the isolation enable and removal related stuff along with the set 
the proper frequency based on the clock information get from the logicoreIp. In 
this way, This is very helpful driver.
 
> What's wrong with just submitting your entire driver series?  Why break it up
> like this?  That's normally not a good idea, you want us to review it in a
> "whole" idea, not piece-meal.
Codecs are owned by different team. when they will submit their code they will 
extend this driver.

Thanks,
Dhaval
> 
> thanks,
> 
> greg k-h

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

['Greg KH']

On Wed, Dec 06, 2017 at 09:05:51AM +, Dhaval Rajeshbhai Shah wrote:
> > 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.
> Can I remove the export api and header file and make this driver generic for 
> the logicoreIP? No other driver will depend on that. This will help us to 
> remove the isolation between the Programmable system and Programmable logic 
> by configuring the logicoreIp register set.

I don't know, let's see what that patch submission looks like.  If it
doesn't do anything, that's not really a good idea :)

What's wrong with just submitting your entire driver series?  Why break
it up like this?  That's normally not a good idea, you want us to review
it in a "whole" idea, not piece-meal.

thanks,

greg k-h

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

['Greg KH']

On Wed, Dec 06, 2017 at 09:05:51AM +, Dhaval Rajeshbhai Shah wrote:
> > 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.
> Can I remove the export api and header file and make this driver generic for 
> the logicoreIP? No other driver will depend on that. This will help us to 
> remove the isolation between the Programmable system and Programmable logic 
> by configuring the logicoreIp register set.

I don't know, let's see what that patch submission looks like.  If it
doesn't do anything, that's not really a good idea :)

What's wrong with just submitting your entire driver series?  Why break
it up like this?  That's normally not a good idea, you want us to review
it in a "whole" idea, not piece-meal.

thanks,

greg k-h

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

[Dhaval Rajeshbhai Shah]

> -Original Message-
> From: 'Greg KH' [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, December 05, 2017 11:56 PM
> To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>
> Cc: a...@arndb.de; linux-kernel@vger.kernel.org; michal.si...@xilinx.com;
> Hyun Kwon <hy...@xilinx.com>
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> logicoreIP init driver
> 
> On Wed, Dec 06, 2017 at 06:01:37AM +, 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.
I have updated that in this reply.
> 
> > > +   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.
Can I remove the export api and header file and make this driver generic for 
the logicoreIP? No other driver will depend on that. This will help us to 
remove the isolation between the Programmable system and Programmable logic by 
configuring the logicoreIp register set.

Thanks 
Dhaval
> 
> thanks,
> 
> greg k-h

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

[Dhaval Rajeshbhai Shah]

> -Original Message-
> From: 'Greg KH' [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, December 05, 2017 11:56 PM
> To: Dhaval Rajeshbhai Shah 
> Cc: a...@arndb.de; linux-kernel@vger.kernel.org; michal.si...@xilinx.com;
> Hyun Kwon 
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU
> logicoreIP init driver
> 
> On Wed, Dec 06, 2017 at 06:01:37AM +, 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.
I have updated that in this reply.
> 
> > > +   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.
Can I remove the export api and header file and make this driver generic for 
the logicoreIP? No other driver will depend on that. This will help us to 
remove the isolation between the Programmable system and Programmable logic by 
configuring the logicoreIp register set.

Thanks 
Dhaval
> 
> thanks,
> 
> greg k-h

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

['Greg KH']

On Wed, Dec 06, 2017 at 06:01:37AM +, 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

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

['Greg KH']

On Wed, Dec 06, 2017 at 06:01:37AM +, 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

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

[Dhaval Rajeshbhai Shah]

Hi Greg k-h,

Thanks a lot for the review.

Replies inline.

-Original Message-
From: Greg KH [mailto:gre...@linuxfoundation.org] 
Sent: Tuesday, December 05, 2017 5:30 AM
To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>
Cc: a...@arndb.de; linux-kernel@vger.kernel.org; michal.si...@xilinx.com; Hyun 
Kwon <hy...@xilinx.com>; Dhaval Rajeshbhai Shah <ds...@xilinx.com>
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP 
init driver

On Tue, Dec 05, 2017 at 03:43:32AM -0800, Dhaval Shah wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design 
> created. This driver will provide the api which can be used by the 
> encoder and decoder driver to get the configured value.

Your subject has a lot of [] in it, none of that is needed except the [PATCH] 
one :)
[Dhaval ] : I will take care from next version. 

> 
> Signed-off-by: Dhaval Shah <ds...@xilinx.com>
> ---
>  drivers/misc/Kconfig|   6 +
>  drivers/misc/Makefile   |   1 +
>  drivers/misc/xlnx_vcu.c | 664 
> 
>  include/misc/xlnx_vcu.h |  18 ++
>  4 files changed, 689 insertions(+)
>  create mode 100644 drivers/misc/xlnx_vcu.c  create mode 100644 
> include/misc/xlnx_vcu.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 
> f1a5c23..3b7c796 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -496,6 +496,12 @@ config PCI_ENDPOINT_TEST
> Enable this configuration option to enable the host side test 
> driver
> for PCI Endpoint.
>  
> +config XILINX_VCU
> +   tristate "Xilinx VCU Init"
> +   default n

That's always the default, no need for this.
[Dhaval ] : I will remove that. 

> +   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.

> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,664 @@
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts   Dhaval Shah <ds...@xilinx.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0

That line goes at the top of the file with // in front of it.
[Dhaval ] : I will update that SPDX license as you said. 


> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 

Why do you need a .h file for a single driver?
[Dhaval ] : There are few APIs and structure which are provided in the .h file 
which will be used by the other driver on the Xilinx based system. I have 
exported few API as well because of this reason. Those API will share the 
information from the logicoreIP register set.

> +/**
> + * xvcu_get_color_depth - read the color depth register
> + * @xvcu:Pointer to the xvcu_device structure
> + *
> + * Return:   Returns 32bit value
> + *
> + */
> +u32 xvcu_get_color_depth(struct xvcu_device *xvcu) {
> + return xvcu_read(xvcu->logicore_reg_ba, VCU_ENC_COLOR_DEPTH); } 
> +EXPORT_SYMBOL_GPL(xvcu_get_color_depth);

Why is your driver exporting symbols that no one uses?

This feels very odd...
[Dhaval ] :  There are few information from the logicoreIp register set which 
needs to share with other driver. That's why those API are exported to use by 
other driver to get those information based on the requirement.

greg k-h

Regards,
Dhaval

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

[Dhaval Rajeshbhai Shah]

Hi Greg k-h,

Thanks a lot for the review.

Replies inline.

-Original Message-
From: Greg KH [mailto:gre...@linuxfoundation.org] 
Sent: Tuesday, December 05, 2017 5:30 AM
To: Dhaval Rajeshbhai Shah 
Cc: a...@arndb.de; linux-kernel@vger.kernel.org; michal.si...@xilinx.com; Hyun 
Kwon ; Dhaval Rajeshbhai Shah 
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP 
init driver

On Tue, Dec 05, 2017 at 03:43:32AM -0800, Dhaval Shah wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design 
> created. This driver will provide the api which can be used by the 
> encoder and decoder driver to get the configured value.

Your subject has a lot of [] in it, none of that is needed except the [PATCH] 
one :)
[Dhaval ] : I will take care from next version. 

> 
> Signed-off-by: Dhaval Shah 
> ---
>  drivers/misc/Kconfig|   6 +
>  drivers/misc/Makefile   |   1 +
>  drivers/misc/xlnx_vcu.c | 664 
> 
>  include/misc/xlnx_vcu.h |  18 ++
>  4 files changed, 689 insertions(+)
>  create mode 100644 drivers/misc/xlnx_vcu.c  create mode 100644 
> include/misc/xlnx_vcu.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 
> f1a5c23..3b7c796 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -496,6 +496,12 @@ config PCI_ENDPOINT_TEST
> Enable this configuration option to enable the host side test 
> driver
> for PCI Endpoint.
>  
> +config XILINX_VCU
> +   tristate "Xilinx VCU Init"
> +   default n

That's always the default, no need for this.
[Dhaval ] : I will remove that. 

> +   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.

> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,664 @@
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts   Dhaval Shah 
> + *
> + * SPDX-License-Identifier: GPL-2.0

That line goes at the top of the file with // in front of it.
[Dhaval ] : I will update that SPDX license as you said. 


> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 

Why do you need a .h file for a single driver?
[Dhaval ] : There are few APIs and structure which are provided in the .h file 
which will be used by the other driver on the Xilinx based system. I have 
exported few API as well because of this reason. Those API will share the 
information from the logicoreIP register set.

> +/**
> + * xvcu_get_color_depth - read the color depth register
> + * @xvcu:Pointer to the xvcu_device structure
> + *
> + * Return:   Returns 32bit value
> + *
> + */
> +u32 xvcu_get_color_depth(struct xvcu_device *xvcu) {
> + return xvcu_read(xvcu->logicore_reg_ba, VCU_ENC_COLOR_DEPTH); } 
> +EXPORT_SYMBOL_GPL(xvcu_get_color_depth);

Why is your driver exporting symbols that no one uses?

This feels very odd...
[Dhaval ] :  There are few information from the logicoreIp register set which 
needs to share with other driver. That's why those API are exported to use by 
other driver to get those information based on the requirement.

greg k-h

Regards,
Dhaval

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

[Dhaval Rajeshbhai Shah]

Hi Arnd,

Replies inline.

-Original Message-
From: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] On Behalf Of Arnd 
Bergmann
Sent: Tuesday, December 05, 2017 5:32 AM
To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>
Cc: gregkh <gre...@linuxfoundation.org>; Linux Kernel Mailing List 
<linux-kernel@vger.kernel.org>; Michal Simek <michal.si...@xilinx.com>; Hyun 
Kwon <hy...@xilinx.com>
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP 
init driver

On Tue, Dec 5, 2017 at 1:38 PM, Dhaval Rajeshbhai Shah <ds...@xilinx.com> wrote:
> From: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] On Behalf 
> Of Arnd Bergmann
> Sent: Tuesday, December 05, 2017 4:19 AM
> To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>
> Cc: gregkh <gre...@linuxfoundation.org>; Linux Kernel Mailing List 
> <linux-kernel@vger.kernel.org>; Michal Simek 
> <michal.si...@xilinx.com>; Hyun Kwon <hy...@xilinx.com>; Dhaval 
> Rajeshbhai Shah <ds...@xilinx.com>
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU 
> logicoreIP init driver
>
> On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <dhaval.s...@xilinx.com> wrote:
>> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design 
>> created. This driver will provide the api which can be used by the 
>> encoder and decoder driver to get the configured value.
>>
>> Signed-off-by: Dhaval Shah <ds...@xilinx.com>
>
> Can you explain what a "VCU" is and why there is no existing subsystem for it?
>
> [Dhaval] : This VCU means Video codec unit. Here, this driver is for the 
> logicoreIP and not the VCU which is created to support the PS and PL 
> 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.

What are PS and PL then?
[Dhaval ] :  PS is the Processing system. PL is the Programmable logic. There 
are few register in the logicoreIP which provides the isolation between this 
two in the chip. 

> If this is the "video codec unit", I'd suggest moving it to 
> drivers/media/platform and sending the patch to the 
> linux-me...@vger.kernel.org list instead.
> [Dhaval] : This is not the Video codec unit but the driver to provide the 
> support the IP created for the PS and PL isolation and clock related 
> information. That why I have placed this under the "drivers/misc/".

Is the driver useful without a codec driver then? (I assume this will be 
answered when you explain PS and PL). If not, just put it in the same directory 
as the codec.
[Dhaval ] : Yes. Driver is useful without a codec driver as well. No dependency 
on the Codec driver.

   Arnd

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

[Dhaval Rajeshbhai Shah]

Hi Arnd,

Replies inline.

-Original Message-
From: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] On Behalf Of Arnd 
Bergmann
Sent: Tuesday, December 05, 2017 5:32 AM
To: Dhaval Rajeshbhai Shah 
Cc: gregkh ; Linux Kernel Mailing List 
; Michal Simek ; Hyun 
Kwon 
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP 
init driver

On Tue, Dec 5, 2017 at 1:38 PM, Dhaval Rajeshbhai Shah  wrote:
> From: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] On Behalf 
> Of Arnd Bergmann
> Sent: Tuesday, December 05, 2017 4:19 AM
> To: Dhaval Rajeshbhai Shah 
> Cc: gregkh ; Linux Kernel Mailing List 
> ; Michal Simek 
> ; Hyun Kwon ; Dhaval 
> Rajeshbhai Shah 
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU 
> logicoreIP init driver
>
> On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah  wrote:
>> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design 
>> created. This driver will provide the api which can be used by the 
>> encoder and decoder driver to get the configured value.
>>
>> Signed-off-by: Dhaval Shah 
>
> Can you explain what a "VCU" is and why there is no existing subsystem for it?
>
> [Dhaval] : This VCU means Video codec unit. Here, this driver is for the 
> logicoreIP and not the VCU which is created to support the PS and PL 
> 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.

What are PS and PL then?
[Dhaval ] :  PS is the Processing system. PL is the Programmable logic. There 
are few register in the logicoreIP which provides the isolation between this 
two in the chip. 

> If this is the "video codec unit", I'd suggest moving it to 
> drivers/media/platform and sending the patch to the 
> linux-me...@vger.kernel.org list instead.
> [Dhaval] : This is not the Video codec unit but the driver to provide the 
> support the IP created for the PS and PL isolation and clock related 
> information. That why I have placed this under the "drivers/misc/".

Is the driver useful without a codec driver then? (I assume this will be 
answered when you explain PS and PL). If not, just put it in the same directory 
as the codec.
[Dhaval ] : Yes. Driver is useful without a codec driver as well. No dependency 
on the Codec driver.

   Arnd

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

[Arnd Bergmann]

On Tue, Dec 5, 2017 at 1:38 PM, Dhaval Rajeshbhai Shah <ds...@xilinx.com> wrote:
> From: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] On Behalf Of 
> Arnd Bergmann
> Sent: Tuesday, December 05, 2017 4:19 AM
> To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>
> Cc: gregkh <gre...@linuxfoundation.org>; Linux Kernel Mailing List 
> <linux-kernel@vger.kernel.org>; Michal Simek <michal.si...@xilinx.com>; Hyun 
> Kwon <hy...@xilinx.com>; Dhaval Rajeshbhai Shah <ds...@xilinx.com>
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU 
> logicoreIP init driver
>
> On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <dhaval.s...@xilinx.com> wrote:
>> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
>> created. This driver will provide the api which can be used by the
>> encoder and decoder driver to get the configured value.
>>
>> Signed-off-by: Dhaval Shah <ds...@xilinx.com>
>
> Can you explain what a "VCU" is and why there is no existing subsystem for it?
>
> [Dhaval] : This VCU means Video codec unit. Here, this driver is for the 
> logicoreIP and not the VCU which is created to support the PS and PL 
> 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.

What are PS and PL then?

> If this is the "video codec unit", I'd suggest moving it to 
> drivers/media/platform and sending the patch to the 
> linux-me...@vger.kernel.org list instead.
> [Dhaval] : This is not the Video codec unit but the driver to provide the 
> support the IP created for the PS and PL isolation and clock related 
> information. That why I have placed this under the "drivers/misc/".

Is the driver useful without a codec driver then? (I assume this will
be answered when
you explain PS and PL). If not, just put it in the same directory as the codec.

   Arnd

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

[Arnd Bergmann]

On Tue, Dec 5, 2017 at 1:38 PM, Dhaval Rajeshbhai Shah  wrote:
> From: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] On Behalf Of 
> Arnd Bergmann
> Sent: Tuesday, December 05, 2017 4:19 AM
> To: Dhaval Rajeshbhai Shah 
> Cc: gregkh ; Linux Kernel Mailing List 
> ; Michal Simek ; Hyun 
> Kwon ; Dhaval Rajeshbhai Shah 
> Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU 
> logicoreIP init driver
>
> On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah  wrote:
>> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
>> created. This driver will provide the api which can be used by the
>> encoder and decoder driver to get the configured value.
>>
>> Signed-off-by: Dhaval Shah 
>
> Can you explain what a "VCU" is and why there is no existing subsystem for it?
>
> [Dhaval] : This VCU means Video codec unit. Here, this driver is for the 
> logicoreIP and not the VCU which is created to support the PS and PL 
> 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.

What are PS and PL then?

> If this is the "video codec unit", I'd suggest moving it to 
> drivers/media/platform and sending the patch to the 
> linux-me...@vger.kernel.org list instead.
> [Dhaval] : This is not the Video codec unit but the driver to provide the 
> support the IP created for the PS and PL isolation and clock related 
> information. That why I have placed this under the "drivers/misc/".

Is the driver useful without a codec driver then? (I assume this will
be answered when
you explain PS and PL). If not, just put it in the same directory as the codec.

   Arnd

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

[Greg KH]

On Tue, Dec 05, 2017 at 03:43:32AM -0800, Dhaval Shah wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> created. This driver will provide the api which can be used
> by the encoder and decoder driver to get the configured value.

Your subject has a lot of [] in it, none of that is needed except the
[PATCH] one :)

> 
> Signed-off-by: Dhaval Shah 
> ---
>  drivers/misc/Kconfig|   6 +
>  drivers/misc/Makefile   |   1 +
>  drivers/misc/xlnx_vcu.c | 664 
> 
>  include/misc/xlnx_vcu.h |  18 ++
>  4 files changed, 689 insertions(+)
>  create mode 100644 drivers/misc/xlnx_vcu.c
>  create mode 100644 include/misc/xlnx_vcu.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f1a5c23..3b7c796 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -496,6 +496,12 @@ config PCI_ENDPOINT_TEST
> Enable this configuration option to enable the host side test 
> driver
> for PCI Endpoint.
>  
> +config XILINX_VCU
> +   tristate "Xilinx VCU Init"
> +   default n

That's always the default, no need for this.

> +   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.

Also, why is this a misc driver?

> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,664 @@
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts   Dhaval Shah 
> + *
> + * SPDX-License-Identifier: GPL-2.0

That line goes at the top of the file with // in front of it.


> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 

Why do you need a .h file for a single driver?

> +/**
> + * xvcu_get_color_depth - read the color depth register
> + * @xvcu:Pointer to the xvcu_device structure
> + *
> + * Return:   Returns 32bit value
> + *
> + */
> +u32 xvcu_get_color_depth(struct xvcu_device *xvcu)
> +{
> + return xvcu_read(xvcu->logicore_reg_ba, VCU_ENC_COLOR_DEPTH);
> +}
> +EXPORT_SYMBOL_GPL(xvcu_get_color_depth);

Why is your driver exporting symbols that no one uses?

This feels very odd...

greg k-h

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

[Greg KH]

On Tue, Dec 05, 2017 at 03:43:32AM -0800, Dhaval Shah wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> created. This driver will provide the api which can be used
> by the encoder and decoder driver to get the configured value.

Your subject has a lot of [] in it, none of that is needed except the
[PATCH] one :)

> 
> Signed-off-by: Dhaval Shah 
> ---
>  drivers/misc/Kconfig|   6 +
>  drivers/misc/Makefile   |   1 +
>  drivers/misc/xlnx_vcu.c | 664 
> 
>  include/misc/xlnx_vcu.h |  18 ++
>  4 files changed, 689 insertions(+)
>  create mode 100644 drivers/misc/xlnx_vcu.c
>  create mode 100644 include/misc/xlnx_vcu.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f1a5c23..3b7c796 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -496,6 +496,12 @@ config PCI_ENDPOINT_TEST
> Enable this configuration option to enable the host side test 
> driver
> for PCI Endpoint.
>  
> +config XILINX_VCU
> +   tristate "Xilinx VCU Init"
> +   default n

That's always the default, no need for this.

> +   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.

Also, why is this a misc driver?

> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,664 @@
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts   Dhaval Shah 
> + *
> + * SPDX-License-Identifier: GPL-2.0

That line goes at the top of the file with // in front of it.


> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 

Why do you need a .h file for a single driver?

> +/**
> + * xvcu_get_color_depth - read the color depth register
> + * @xvcu:Pointer to the xvcu_device structure
> + *
> + * Return:   Returns 32bit value
> + *
> + */
> +u32 xvcu_get_color_depth(struct xvcu_device *xvcu)
> +{
> + return xvcu_read(xvcu->logicore_reg_ba, VCU_ENC_COLOR_DEPTH);
> +}
> +EXPORT_SYMBOL_GPL(xvcu_get_color_depth);

Why is your driver exporting symbols that no one uses?

This feels very odd...

greg k-h

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

[Dhaval Rajeshbhai Shah]

Hi Arnd,

Thanks a lot for the review.
Replies inline.

-Original Message-
From: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] On Behalf Of Arnd 
Bergmann
Sent: Tuesday, December 05, 2017 4:19 AM
To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>
Cc: gregkh <gre...@linuxfoundation.org>; Linux Kernel Mailing List 
<linux-kernel@vger.kernel.org>; Michal Simek <michal.si...@xilinx.com>; Hyun 
Kwon <hy...@xilinx.com>; Dhaval Rajeshbhai Shah <ds...@xilinx.com>
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP 
init driver

On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah <dhaval.s...@xilinx.com> wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design 
> created. This driver will provide the api which can be used by the 
> encoder and decoder driver to get the configured value.
>
> Signed-off-by: Dhaval Shah <ds...@xilinx.com>

Can you explain what a "VCU" is and why there is no existing subsystem for it?

[Dhaval] : This VCU means Video codec unit. Here, this driver is for the 
logicoreIP and not the VCU which is created to support the PS and PL 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.

If this is the "video codec unit", I'd suggest moving it to 
drivers/media/platform and sending the patch to the linux-me...@vger.kernel.org 
list instead.
[Dhaval] : This is not the Video codec unit but the driver to provide the 
support the IP created for the PS and PL isolation and clock related 
information. That why I have placed this under the "drivers/misc/".


  Arnd

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

[Dhaval Rajeshbhai Shah]

Hi Arnd,

Thanks a lot for the review.
Replies inline.

-Original Message-
From: arndbergm...@gmail.com [mailto:arndbergm...@gmail.com] On Behalf Of Arnd 
Bergmann
Sent: Tuesday, December 05, 2017 4:19 AM
To: Dhaval Rajeshbhai Shah 
Cc: gregkh ; Linux Kernel Mailing List 
; Michal Simek ; Hyun 
Kwon ; Dhaval Rajeshbhai Shah 
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP 
init driver

On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah  wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design 
> created. This driver will provide the api which can be used by the 
> encoder and decoder driver to get the configured value.
>
> Signed-off-by: Dhaval Shah 

Can you explain what a "VCU" is and why there is no existing subsystem for it?

[Dhaval] : This VCU means Video codec unit. Here, this driver is for the 
logicoreIP and not the VCU which is created to support the PS and PL 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.

If this is the "video codec unit", I'd suggest moving it to 
drivers/media/platform and sending the patch to the linux-me...@vger.kernel.org 
list instead.
[Dhaval] : This is not the Video codec unit but the driver to provide the 
support the IP created for the PS and PL isolation and clock related 
information. That why I have placed this under the "drivers/misc/".


  Arnd

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

[Arnd Bergmann]

On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah  wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> created. This driver will provide the api which can be used
> by the encoder and decoder driver to get the configured value.
>
> Signed-off-by: Dhaval Shah 

Can you explain what a "VCU" is and why there is no existing subsystem for it?

If this is the "video codec unit", I'd suggest moving it to
drivers/media/platform
and sending the patch to the linux-me...@vger.kernel.org list instead.

  Arnd

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

[Arnd Bergmann]

On Tue, Dec 5, 2017 at 12:43 PM, Dhaval Shah  wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design
> created. This driver will provide the api which can be used
> by the encoder and decoder driver to get the configured value.
>
> Signed-off-by: Dhaval Shah 

Can you explain what a "VCU" is and why there is no existing subsystem for it?

If this is the "video codec unit", I'd suggest moving it to
drivers/media/platform
and sending the patch to the linux-me...@vger.kernel.org list instead.

  Arnd