[PATCH 1/2] staging: bcm2835-audio: Removed spaces before ')'

2017-02-05 Thread Abhijit Naik
bcm2835-vchiq.c and vc_vchi_audioserv_defs.h:
fixing ERROR: space prohibited before that close parenthesis

Signed-off-by: Abhijit Naik 
---
 drivers/staging/bcm2835-audio/bcm2835-vchiq.c  | 16 
 drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/bcm2835-audio/bcm2835-vchiq.c 
b/drivers/staging/bcm2835-audio/bcm2835-vchiq.c
index e8fd9c79bcfc..99dc8c4e0185 100644
--- a/drivers/staging/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/bcm2835-audio/bcm2835-vchiq.c
@@ -44,15 +44,15 @@

 /* Logging macros (for remapping to other logging mechanisms, i.e., printf) */
 #ifdef AUDIO_DEBUG_ENABLE
-#define LOG_ERR( fmt, arg... )   pr_err( "%s:%d " fmt, __func__, __LINE__, 
##arg)
-#define LOG_WARN( fmt, arg... )  pr_info( "%s:%d " fmt, __func__, __LINE__, 
##arg)
-#define LOG_INFO( fmt, arg... )  pr_info( "%s:%d " fmt, __func__, __LINE__, 
##arg)
-#define LOG_DBG( fmt, arg... )   pr_info( "%s:%d " fmt, __func__, __LINE__, 
##arg)
+#define LOG_ERR( fmt, arg...)   pr_err( "%s:%d " fmt, __func__, __LINE__, 
##arg)
+#define LOG_WARN( fmt, arg...)  pr_info( "%s:%d " fmt, __func__, __LINE__, 
##arg)
+#define LOG_INFO( fmt, arg...)  pr_info( "%s:%d " fmt, __func__, __LINE__, 
##arg)
+#define LOG_DBG( fmt, arg...)   pr_info( "%s:%d " fmt, __func__, __LINE__, 
##arg)
 #else
-#define LOG_ERR( fmt, arg... )   pr_err( "%s:%d " fmt, __func__, __LINE__, 
##arg)
-#define LOG_WARN( fmt, arg... )
-#define LOG_INFO( fmt, arg... )
-#define LOG_DBG( fmt, arg... )
+#define LOG_ERR( fmt, arg...)   pr_err( "%s:%d " fmt, __func__, __LINE__, 
##arg)
+#define LOG_WARN( fmt, arg...)
+#define LOG_INFO( fmt, arg...)
+#define LOG_DBG( fmt, arg...)
 #endif

 struct bcm2835_audio_instance {
diff --git a/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h 
b/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h
index 09f07fd891bb..f88c5632918b 100644
--- a/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h
+++ b/drivers/staging/bcm2835-audio/vc_vchi_audioserv_defs.h
@@ -22,7 +22,7 @@
 #define VC_AUDIO_SERVER_NAME  MAKE_FOURCC("AUDS")

 /* Maximum message length */
-#define VC_AUDIO_MAX_MSG_LEN  (sizeof( VC_AUDIO_MSG_T ))
+#define VC_AUDIO_MAX_MSG_LEN  (sizeof( VC_AUDIO_MSG_T))

 /*
  *  List of screens that are currently supported
--
2.11.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCHv1] net-next: treewide use is_vlan_dev() helper function.

2017-02-05 Thread Johannes Thumshirn


On 02/04/2017 06:00 PM, Parav Pandit wrote:

This patch makes use of is_vlan_dev() function instead of flag
comparison which is exactly done by is_vlan_dev() helper function.

Signed-off-by: Parav Pandit 
Reviewed-by: Daniel Jurgens 
---



For drivers/scsi/fcoe/fcoe.c:
Acked-by: Johannes Thumshirn 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: iio: addac: adt7316.c - style fix, octal permission

2017-02-05 Thread Derek Robson
Changed file permissions to octal.
Found with checkpatch.

Signed-off-by: Derek Robson 
---
 drivers/staging/iio/addac/adt7316.c | 108 ++--
 1 file changed, 54 insertions(+), 54 deletions(-)

diff --git a/drivers/staging/iio/addac/adt7316.c 
b/drivers/staging/iio/addac/adt7316.c
index 6054c7298fce..aa251c245981 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -267,7 +267,7 @@ static ssize_t adt7316_store_enabled(struct device *dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(enabled, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(enabled, 0644,
adt7316_show_enabled,
adt7316_store_enabled,
0);
@@ -311,7 +311,7 @@ static ssize_t adt7316_store_select_ex_temp(struct device 
*dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(select_ex_temp, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(select_ex_temp, 0644,
adt7316_show_select_ex_temp,
adt7316_store_select_ex_temp,
0);
@@ -352,7 +352,7 @@ static ssize_t adt7316_store_mode(struct device *dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(mode, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(mode, 0644,
adt7316_show_mode,
adt7316_store_mode,
0);
@@ -364,7 +364,7 @@ static ssize_t adt7316_show_all_modes(struct device *dev,
return sprintf(buf, "single_channel\nround_robin\n");
 }
 
-static IIO_DEVICE_ATTR(all_modes, S_IRUGO, adt7316_show_all_modes, NULL, 0);
+static IIO_DEVICE_ATTR(all_modes, 0444, adt7316_show_all_modes, NULL, 0);
 
 static ssize_t adt7316_show_ad_channel(struct device *dev,
struct device_attribute *attr,
@@ -446,7 +446,7 @@ static ssize_t adt7316_store_ad_channel(struct device *dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(ad_channel, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(ad_channel, 0644,
adt7316_show_ad_channel,
adt7316_store_ad_channel,
0);
@@ -469,7 +469,7 @@ static ssize_t adt7316_show_all_ad_channels(struct device 
*dev,
"2 - External Temperature\n");
 }
 
-static IIO_DEVICE_ATTR(all_ad_channels, S_IRUGO,
+static IIO_DEVICE_ATTR(all_ad_channels, 0444,
adt7316_show_all_ad_channels, NULL, 0);
 
 static ssize_t adt7316_show_disable_averaging(struct device *dev,
@@ -506,7 +506,7 @@ static ssize_t adt7316_store_disable_averaging(struct 
device *dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(disable_averaging, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(disable_averaging, 0644,
adt7316_show_disable_averaging,
adt7316_store_disable_averaging,
0);
@@ -545,7 +545,7 @@ static ssize_t adt7316_store_enable_smbus_timeout(struct 
device *dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(enable_smbus_timeout, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(enable_smbus_timeout, 0644,
adt7316_show_enable_smbus_timeout,
adt7316_store_enable_smbus_timeout,
0);
@@ -583,7 +583,7 @@ static ssize_t adt7316_store_powerdown(struct device *dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(powerdown, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(powerdown, 0644,
adt7316_show_powerdown,
adt7316_store_powerdown,
0);
@@ -621,7 +621,7 @@ static ssize_t adt7316_store_fast_ad_clock(struct device 
*dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(fast_ad_clock, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(fast_ad_clock, 0644,
adt7316_show_fast_ad_clock,
adt7316_store_fast_ad_clock,
0);
@@ -674,7 +674,7 @@ static ssize_t adt7316_store_da_high_resolution(struct 
device *dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(da_high_resolution, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(da_high_resolution, 0644,
adt7316_show_da_high_resolution,
adt7316_store_da_high_resolution,
0);
@@ -720,7 +720,7 @@ static ssize_t adt7316_store_AIN_internal_Vref(struct 
device *dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(AIN_internal_Vref, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(AIN_internal_Vref, 0644,
adt7316_show_AIN_internal_Vref,
adt7316_store_AIN_internal_Vref,
0);
@@ -760,7 +760,7 @@ static ssize_t adt7316_store_enable_prop_DACA(struct device 
*dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(enable_proportion_DACA, S_IRUGO | S_IWUSR,
+static IIO_DEVICE_ATTR(enable_proportion_DACA, 0644,
adt7316_show_enable_prop_DACA,
adt7316_store_enable_prop_DACA,
0);
@@ -799,7 +799,7 @@ static ssize_t adt7316_store_enable_prop_DACB(struct device 
*dev,
return len;
 }
 
-static IIO_DEVICE_ATTR(enable_proportion_DACB, S_IRUGO | S_IWUSR,
+static 

Re: Is it time to move drivers/staging/netlogic/ out of staging?

2017-02-05 Thread Florian Fainelli
Le 02/04/17 à 00:08, Greg KH a écrit :
> On Fri, Feb 03, 2017 at 06:37:02PM -0800, Florian Fainelli wrote:
>>
>>
>> On 02/03/2017 12:44 PM, Greg KH wrote:
>>> On Fri, Feb 03, 2017 at 12:38:45PM -0800, Florian Fainelli wrote:
 On 02/03/2017 12:36 PM, Greg KH wrote:
> On Fri, Feb 03, 2017 at 10:57:16AM -0800, Joe Perches wrote:
>> On Fri, 2017-02-03 at 10:50 -0800, Florian Fainelli wrote:
>>> (with JC's other email)
>>
>> And now with Greg's proper email too
>>
>>> On 02/03/2017 10:47 AM, Joe Perches wrote:
 64 bit stats isn't implemented, but is that really necessary?
 Anything else?
>>>
>>> Joe, do you have such hardware that you are interested in getting
>>> supported, or was that just to reduce the amount of drivers in staging?
>>> I am really not clear about what happened to that entire product line,
>>> and whether there is any interest in having anything supported these 
>>> days...
>>
>> No hardware.  Just to reduce staging driver count.
>
> Without hardware or a "real" maintainer, it shouldn't be moved.
>
> Heck, if no one has the hardware, let's just delete the thing.

 I do have one, and other colleagues have some too, but I am not heavily
 using it, nor do I have many cycles to spend on that... sounds like we
 could keep it in staging for another 6 months and see what happens then?
>>>
>>> Well, if it works for you, want to maintain it?  :)
>>
>> I'd have to locate the documentation first, and you would have to reply
>> to my patch series about DSA ;)
> 
> I don't have any patch series in my queue, sorry, so I have no idea what
> you are talking about...

You don't really? How about this patch series:

https://www.mail-archive.com/netdev@vger.kernel.org/msg147917.html

and me asking you another time to provide feedback:

https://www.mail-archive.com/netdev@vger.kernel.org/msg150885.html
-- 
Florian
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: speakup: speakup.h - remove unused define

2017-02-05 Thread Derek Robson
As part of cleaning up symbolic permissions found define that is not used.

Signed-off-by: Derek Robson 
---
 drivers/staging/speakup/speakup.h | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/speakup/speakup.h 
b/drivers/staging/speakup/speakup.h
index d5bd9b0a5c95..b203f0f883a9 100644
--- a/drivers/staging/speakup/speakup.h
+++ b/drivers/staging/speakup/speakup.h
@@ -9,10 +9,6 @@
 #define SHIFT_TBL_SIZE 64
 #define MAX_DESC_LEN 72
 
-/* proc permissions */
-#define USER_R (S_IFREG | 0444)
-#define USER_W (S_IFREG | 0666)
-
 #define TOGGLE_0 .u.n = {NULL, 0, 0, 1, 0, 0, NULL }
 #define TOGGLE_1 .u.n = {NULL, 1, 0, 1, 0, 0, NULL }
 #define MAXVARLEN 15
-- 
2.11.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCHv1] net-next: treewide use is_vlan_dev() helper function.

2017-02-05 Thread Jonathan Maxwell
On Sun, Feb 5, 2017 at 4:00 AM, Parav Pandit  wrote:
> This patch makes use of is_vlan_dev() function instead of flag
> comparison which is exactly done by is_vlan_dev() helper function.
>
> Signed-off-by: Parav Pandit 
> Reviewed-by: Daniel Jurgens 
> ---
>  drivers/infiniband/core/cma.c|  6 ++
>  drivers/infiniband/sw/rxe/rxe_net.c  |  2 +-
>  drivers/net/ethernet/broadcom/cnic.c |  2 +-
>  drivers/net/ethernet/chelsio/cxgb3/l2t.c |  2 +-
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c  |  4 ++--
>  drivers/net/ethernet/chelsio/cxgb4/l2t.c |  2 +-
>  drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c |  8 
>  drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c |  4 ++--
>  drivers/net/hyperv/netvsc_drv.c  |  2 +-
>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c|  6 +++---
>  drivers/scsi/cxgbi/libcxgbi.c|  6 +++---
>  drivers/scsi/fcoe/fcoe.c | 13 ++---
>  include/rdma/ib_addr.h   |  6 ++
>  net/hsr/hsr_slave.c  |  3 ++-
>  14 files changed, 31 insertions(+), 35 deletions(-)
>

Neatens the code up nicely.

Acked-by: Jon Maxwell 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH 05/14] netvsc: remove no longer needed receive staging buffers

2017-02-05 Thread KY Srinivasan


> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Wednesday, February 1, 2017 8:29 AM
> To: KY Srinivasan ; gre...@linuxfoundation.org
> Cc: de...@linuxdriverproject.org; virtualizat...@lists.linux-foundation.org;
> Stephen Hemminger 
> Subject: [PATCH 05/14] netvsc: remove no longer needed receive staging
> buffers
> 
> Since commit aed8c164ca5199 ("Drivers: hv: ring_buffer: count on wrap
> around mappings") it is no longer necessary to handle ring wrapping
> by having a special receive buffer.
> 
> Signed-off-by: Stephen Hemminger 
> ---
>  drivers/net/hyperv/hyperv_net.h   |  5 ---
>  drivers/net/hyperv/netvsc.c   | 83 
> ++-
>  drivers/net/hyperv/rndis_filter.c | 11 --
>  3 files changed, 11 insertions(+), 88 deletions(-)
> 
> diff --git a/drivers/net/hyperv/hyperv_net.h
> b/drivers/net/hyperv/hyperv_net.h
> index 3958adade7eb..cce70ceba6d5 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -748,11 +748,6 @@ struct netvsc_device {
> 
>   int ring_size;
> 
> - /* The primary channel callback buffer */
> - unsigned char *cb_buffer;
> - /* The sub channel callback buffer */
> - unsigned char *sub_cb_buf;
> -
>   struct multi_send_data msd[VRSS_CHANNEL_MAX];
>   u32 max_pkt; /* max number of pkt in one send, e.g. 8 */
>   u32 pkt_align; /* alignment bytes, e.g. 8 */
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index e326e68f9f6d..7487498b663c 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -67,12 +67,6 @@ static struct netvsc_device *alloc_net_device(void)
>   if (!net_device)
>   return NULL;
> 
> - net_device->cb_buffer = kzalloc(NETVSC_PACKET_SIZE,
> GFP_KERNEL);
> - if (!net_device->cb_buffer) {
> - kfree(net_device);
> - return NULL;
> - }
> -
>   net_device->mrc[0].buf = vzalloc(NETVSC_RECVSLOT_MAX *
>sizeof(struct recv_comp_data));
> 
> @@ -93,7 +87,6 @@ static void free_netvsc_device(struct netvsc_device
> *nvdev)
>   for (i = 0; i < VRSS_CHANNEL_MAX; i++)
>   vfree(nvdev->mrc[i].buf);
> 
> - kfree(nvdev->cb_buffer);
>   kfree(nvdev);
>  }
> 
> @@ -584,7 +577,6 @@ void netvsc_device_remove(struct hv_device
> *device)
>   vmbus_close(device->channel);
> 
>   /* Release all resources */
> - vfree(net_device->sub_cb_buf);
>   free_netvsc_device(net_device);
>  }
> 
> @@ -1256,16 +1248,11 @@ static void netvsc_process_raw_pkt(struct
> hv_device *device,
> 
>  void netvsc_channel_cb(void *context)
>  {
> - int ret;
> - struct vmbus_channel *channel = (struct vmbus_channel *)context;
> + struct vmbus_channel *channel = context;
>   u16 q_idx = channel->offermsg.offer.sub_channel_index;
>   struct hv_device *device;
>   struct netvsc_device *net_device;
> - u32 bytes_recvd;
> - u64 request_id;
>   struct vmpacket_descriptor *desc;
> - unsigned char *buffer;
> - int bufferlen = NETVSC_PACKET_SIZE;
>   struct net_device *ndev;
>   bool need_to_commit = false;
> 
> @@ -1277,65 +1264,19 @@ void netvsc_channel_cb(void *context)
>   net_device = get_inbound_net_device(device);
>   if (!net_device)
>   return;
> +
>   ndev = hv_get_drvdata(device);
> - buffer = get_per_channel_state(channel);
> -
> - do {
> - desc = get_next_pkt_raw(channel);
> - if (desc != NULL) {
> - netvsc_process_raw_pkt(device,
> -channel,
> -net_device,
> -ndev,
> -desc->trans_id,
> -desc);
> -
> - put_pkt_raw(channel, desc);
> - need_to_commit = true;
> - continue;
> - }
> - if (need_to_commit) {
> - need_to_commit = false;
> - commit_rd_index(channel);
> - }
> 
> - ret = vmbus_recvpacket_raw(channel, buffer, bufferlen,
> -_recvd, _id);
> - if (ret == 0) {
> - if (bytes_recvd > 0) {
> - desc = (struct vmpacket_descriptor *)buffer;
> - netvsc_process_raw_pkt(device,
> -channel,
> -net_device,
> -ndev,
> -request_id,
> -desc);
> - 

Re: [PATCH 1/6] staging: Import the BCM2835 MMAL-based V4L2 camera driver.

2017-02-05 Thread Michael Zoran
Dave: I'd personally like to see the current version of vchi stabbed,
poisoned, beaten, shot, chained and thrown in a river.:)

Would it be possible to find another transport for this driver to
remove the dependency on VCHI?  Given the process of improving drivers
in stagging, I don't expect VCHI to ever make it out of staging in it's
current form either. One possibility is mbox, but it has the limitation
of one one request at a time.

Also, you mention protecting IP and that V4L expects everything to be
exposed.  How does V4L2 work if I have a hybrid software/hardware video
capture device? 

In the case of other drivers in linux that upload a large firmware blob
into who knows where, how does the open source part of the driver hook
into the firmware blob?

On Sun, 2017-02-05 at 22:15 +, Dave Stevenson wrote:
> Hi Mauro.
> 
> I'm going to stick my head above the parapet as one of the original 
> authors back when I worked at Broadcom.
> As it happens I started working at Raspberry Pi last Monday, so that 
> puts me in a place where I can work on this again a bit more. (The
> last 
> two years have been just a spare time support role).
> Whilst I have done kernel development work in various roles, it's
> all 
> been downstream so I've not been that active on these lists before.
> 
> All formatting/checkpatch comments noted.
> Checkpatch was whinging when this was first written around December
> 2013 
> about long lines, so many got broken up to shut it up. Views on code 
> style and checkpatch seem to have changed a little since then.
> I thought we had made checkpatch happy before the driver was pushed,
> but 
> with some of the comments still having // style I guess some slipped 
> through the net.
> Yes chunks of this could do with refactoring to reduce the levels of 
> indentation - always more to do.
> If I've removed any formatting/style type comments in my cuts it's
> not 
> because I'm ignoring them, just that they're not something that
> needs 
> discussion (just fixing). I've only taken out the really big lumps
> of 
> code with no comments on.
> 
> Newbie question: if this has already been merged to staging, where am
> I 
> looking for the relevant tree to add patches on top of? 
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
> branch 
> staging-next?
> 
> Responses to the rest inline.
> TL;DR answer is that you are seeing the top edge of a full ISP 
> processing pipe and optional encoders running on the GPU, mainly as 
> there are blocks that can't be exposed for IP reasons (Raspberry Pi
> only 
> being the customer not silicon vendor constrains what can and can't
> be 
> made public).
> That doesn't seem to fit very well into V4L2 which expects that it
> can 
> see all the detail, so there are a few nasty spots to shoe-horn it
> in. 
> If there are better ways to solve the problems, then I'm open to
> them.
> 
> Thanks
>    Dave
> 
> 
> On 03/02/17 18:59, Mauro Carvalho Chehab wrote:
> > HI Eric,
> > 
> > Em Fri, 27 Jan 2017 13:54:58 -0800
> > Eric Anholt  escreveu:
> > 
> > > - Supports raw YUV capture, preview, JPEG and H264.
> > > - Uses videobuf2 for data transfer, using dma_buf.
> > > - Uses 3.6.10 timestamping
> > > - Camera power based on use
> > > - Uses immutable input mode on video encoder
> > > 
> > > This code comes from the Raspberry Pi kernel tree (rpi-4.9.y) as
> > > of
> > > a15ba877dab4e61ea3fc7b006e2a73828b083c52.
> > 
> > First of all, thanks for that! Having an upstream driver for the
> > RPi camera is something that has been long waited!
> > 
> > Greg was kick on merging it on staging ;) Anyway, the real review
> > will happen when the driver becomes ready to be promoted out of
> > staging. When you address the existing issues and get it ready to
> > merge, please send the patch with such changes to linux-media ML.
> > I'll do a full review on it by then.
> 
> Is that even likely given the dependence on VCHI? I wasn't expecting 
> VCHI to leave staging, which would force this to remain too.
> 
> > Still, let me do a quick review on this driver, specially at the
> > non-MMAL code.
> > 
> > > 
> > > Signed-off-by: Eric Anholt 
> > > ---
> > >  .../media/platform/bcm2835/bcm2835-camera.c| 2016
> > > 
> > >  .../media/platform/bcm2835/bcm2835-camera.h|  145 ++
> > >  drivers/staging/media/platform/bcm2835/controls.c  | 1345
> > > +
> > >  .../staging/media/platform/bcm2835/mmal-common.h   |   53 +
> > >  .../media/platform/bcm2835/mmal-encodings.h|  127 ++
> > >  .../media/platform/bcm2835/mmal-msg-common.h   |   50 +
> > >  .../media/platform/bcm2835/mmal-msg-format.h   |   81 +
> > >  .../staging/media/platform/bcm2835/mmal-msg-port.h |  107 ++
> > >  drivers/staging/media/platform/bcm2835/mmal-msg.h  |  404 
> > >  .../media/platform/bcm2835/mmal-parameters.h   |  689
> > > +++
> > >  .../staging/media/platform/bcm2835/mmal-vchiq.c| 1916

Re: [PATCH 1/6] staging: Import the BCM2835 MMAL-based V4L2 camera driver.

2017-02-05 Thread Dave Stevenson

Hi Mauro.

I'm going to stick my head above the parapet as one of the original 
authors back when I worked at Broadcom.
As it happens I started working at Raspberry Pi last Monday, so that 
puts me in a place where I can work on this again a bit more. (The last 
two years have been just a spare time support role).
Whilst I have done kernel development work in various roles, it's all 
been downstream so I've not been that active on these lists before.


All formatting/checkpatch comments noted.
Checkpatch was whinging when this was first written around December 2013 
about long lines, so many got broken up to shut it up. Views on code 
style and checkpatch seem to have changed a little since then.
I thought we had made checkpatch happy before the driver was pushed, but 
with some of the comments still having // style I guess some slipped 
through the net.
Yes chunks of this could do with refactoring to reduce the levels of 
indentation - always more to do.
If I've removed any formatting/style type comments in my cuts it's not 
because I'm ignoring them, just that they're not something that needs 
discussion (just fixing). I've only taken out the really big lumps of 
code with no comments on.


Newbie question: if this has already been merged to staging, where am I 
looking for the relevant tree to add patches on top of? 
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git branch 
staging-next?


Responses to the rest inline.
TL;DR answer is that you are seeing the top edge of a full ISP 
processing pipe and optional encoders running on the GPU, mainly as 
there are blocks that can't be exposed for IP reasons (Raspberry Pi only 
being the customer not silicon vendor constrains what can and can't be 
made public).
That doesn't seem to fit very well into V4L2 which expects that it can 
see all the detail, so there are a few nasty spots to shoe-horn it in. 
If there are better ways to solve the problems, then I'm open to them.


Thanks
  Dave


On 03/02/17 18:59, Mauro Carvalho Chehab wrote:

HI Eric,

Em Fri, 27 Jan 2017 13:54:58 -0800
Eric Anholt  escreveu:


- Supports raw YUV capture, preview, JPEG and H264.
- Uses videobuf2 for data transfer, using dma_buf.
- Uses 3.6.10 timestamping
- Camera power based on use
- Uses immutable input mode on video encoder

This code comes from the Raspberry Pi kernel tree (rpi-4.9.y) as of
a15ba877dab4e61ea3fc7b006e2a73828b083c52.


First of all, thanks for that! Having an upstream driver for the
RPi camera is something that has been long waited!

Greg was kick on merging it on staging ;) Anyway, the real review
will happen when the driver becomes ready to be promoted out of
staging. When you address the existing issues and get it ready to
merge, please send the patch with such changes to linux-media ML.
I'll do a full review on it by then.


Is that even likely given the dependence on VCHI? I wasn't expecting 
VCHI to leave staging, which would force this to remain too.



Still, let me do a quick review on this driver, specially at the
non-MMAL code.



Signed-off-by: Eric Anholt 
---
 .../media/platform/bcm2835/bcm2835-camera.c| 2016 
 .../media/platform/bcm2835/bcm2835-camera.h|  145 ++
 drivers/staging/media/platform/bcm2835/controls.c  | 1345 +
 .../staging/media/platform/bcm2835/mmal-common.h   |   53 +
 .../media/platform/bcm2835/mmal-encodings.h|  127 ++
 .../media/platform/bcm2835/mmal-msg-common.h   |   50 +
 .../media/platform/bcm2835/mmal-msg-format.h   |   81 +
 .../staging/media/platform/bcm2835/mmal-msg-port.h |  107 ++
 drivers/staging/media/platform/bcm2835/mmal-msg.h  |  404 
 .../media/platform/bcm2835/mmal-parameters.h   |  689 +++
 .../staging/media/platform/bcm2835/mmal-vchiq.c| 1916 +++
 .../staging/media/platform/bcm2835/mmal-vchiq.h|  178 ++
 12 files changed, 7111 insertions(+)
 create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.c
 create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.h
 create mode 100644 drivers/staging/media/platform/bcm2835/controls.c
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-common.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-encodings.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-common.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-format.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-port.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-parameters.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.c
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.h

diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c 
b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
new file mode 100644

Re: [PATCH v3 15/24] media: Add userspace header file for i.MX

2017-02-05 Thread Laurent Pinchart
Hi Steve,

On Friday 13 Jan 2017 15:13:33 Steve Longerbeam wrote:
> On 01/13/2017 04:05 AM, Philipp Zabel wrote:
> > Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> >> This adds a header file for use by userspace programs wanting to interact
> >> with the i.MX media driver. It defines custom v4l2 controls and events
> >> generated by the i.MX v4l2 subdevices.
> >> 
> >> Signed-off-by: Steve Longerbeam 
> >> ---
> >> 
> >>   include/uapi/media/Kbuild |  1 +
> >>   include/uapi/media/imx.h  | 30 ++
> >>   2 files changed, 31 insertions(+)
> >>   create mode 100644 include/uapi/media/imx.h
> >> 
> >> diff --git a/include/uapi/media/Kbuild b/include/uapi/media/Kbuild
> >> index aafaa5a..fa78958 100644
> >> --- a/include/uapi/media/Kbuild
> >> +++ b/include/uapi/media/Kbuild
> >> @@ -1 +1,2 @@
> >> 
> >>   # UAPI Header export list
> >> 
> >> +header-y += imx.h
> >> diff --git a/include/uapi/media/imx.h b/include/uapi/media/imx.h
> >> new file mode 100644
> >> index 000..2421d9c
> >> --- /dev/null
> >> +++ b/include/uapi/media/imx.h
> >> @@ -0,0 +1,30 @@
> >> +/*
> >> + * Copyright (c) 2014-2015 Mentor Graphics Inc.
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License as published by
> >> the + * Free Software Foundation; either version 2 of the
> >> + * License, or (at your option) any later version
> >> + */
> >> +
> >> +#ifndef __UAPI_MEDIA_IMX_H__
> >> +#define __UAPI_MEDIA_IMX_H__
> >> +
> >> +/*
> >> + * events from the subdevs
> >> + */
> >> +#define V4L2_EVENT_IMX_CLASS  V4L2_EVENT_PRIVATE_START
> >> +#define V4L2_EVENT_IMX_NFB4EOF(V4L2_EVENT_IMX_CLASS + 1)
> >> +#define V4L2_EVENT_IMX_EOF_TIMEOUT(V4L2_EVENT_IMX_CLASS + 2)
> >> +#define V4L2_EVENT_IMX_FRAME_INTERVAL (V4L2_EVENT_IMX_CLASS + 3)
> > 
> > Aren't these generic enough to warrant common events? I would think
> > there have to be other capture IP cores that can signal aborted frames
> > or frame timeouts.
> 
> Yes, agreed. A frame capture timeout, or frame interval error, are
> both generic concepts. At some point it would be great to make the
> Frame Interval Monitor generally available under v4l2-core. As for the
> EOF timeout event, I'll look into moving that into a generic V4L2 event.

I'd prefer generic events if possible, but regardless of whether that's 
possible, the events must be documented in the V4L2 specification.

-- 
Regards,

Laurent Pinchart

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3 13/24] platform: add video-multiplexer subdevice driver

2017-02-05 Thread Laurent Pinchart
Hi Steve,

On Tuesday 24 Jan 2017 18:07:55 Steve Longerbeam wrote:
> On 01/24/2017 04:02 AM, Philipp Zabel wrote:
> > On Fri, 2017-01-20 at 15:03 +0100, Hans Verkuil wrote:
> >>> +
> >>> +int vidsw_g_mbus_config(struct v4l2_subdev *sd, struct v4l2_mbus_config
> >>> *cfg)
> >>> +{
> >>> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
> >>> + struct media_pad *pad;
> >>> + int ret;
> >>> +
> >>> + if (vidsw->active == -1) {
> >>> + dev_err(sd->dev, "no configuration for inactive mux\n");
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + /*
> >>> +  * Retrieve media bus configuration from the entity connected to the
> >>> +  * active input
> >>> +  */
> >>> + pad = media_entity_remote_pad(>pads[vidsw->active]);
> >>> + if (pad) {
> >>> + sd = media_entity_to_v4l2_subdev(pad->entity);
> >>> + ret = v4l2_subdev_call(sd, video, g_mbus_config, cfg);
> >>> + if (ret == -ENOIOCTLCMD)
> >>> + pad = NULL;
> >>> + else if (ret < 0) {
> >>> + dev_err(sd->dev, "failed to get source 
configuration\n");
> >>> + return ret;
> >>> + }
> >>> + }
> >>> + if (!pad) {
> >>> + /* Mirror the input side on the output side */
> >>> + cfg->type = vidsw->endpoint[vidsw->active].bus_type;
> >>> + if (cfg->type == V4L2_MBUS_PARALLEL ||
> >>> + cfg->type == V4L2_MBUS_BT656)
> >>> + cfg->flags = vidsw->endpoint[vidsw-
>active].bus.parallel.flags;
> >>> + }
> >>> +
> >>> + return 0;
> >>> +}
> >> 
> >> I am not certain this op is needed at all. In the current kernel this op
> >> is only used by soc_camera, pxa_camera and omap3isp (somewhat dubious).
> >> Normally this information should come from the device tree and there
> >> should be no need for this op.
> >> 
> >> My (tentative) long-term plan was to get rid of this op.
> >> 
> >> If you don't need it, then I recommend it is removed.
> 
> Hi Hans, the imx-media driver was only calling g_mbus_config to the camera
> sensor, and it was doing that to determine the sensor's bus type. This info
> was already available from parsing a v4l2_of_endpoint from the sensor node.
> So it was simple to remove the g_mbus_config calls, and instead rely on the
> parsed sensor v4l2_of_endpoint.

That's not a good point. The imx-media driver must not parse the sensor DT 
node as it is not aware of what bindings the sensor is compatible with. 
Information must instead be queried from the sensor subdev at runtime, through 
the g_mbus_config() operation.

Of course, if you can get the information from the imx-media DT node, that's 
certainly an option. It's only information provided by the sensor driver that 
you have no choice but query using a subdev operation.

> > We currently use this to make the CSI capture interface understand
> > whether its source from the MIPI CSI-2 or from the parallel bus. That is
> > probably something that should be fixed, but I'm not quite sure how.
> > 
> > The Synopsys DesignWare MIPI CSI-2 reciever turns the incoming MIPI
> > CSI-2 signal into a 32-bit parallel pixel bus plus some signals for the
> > MIPI specific metadata (virtual channel, data type).
> > 
> > Then the CSI2IPU gasket turns this input bus into four separate parallel
> > 16-bit pixel buses plus an 8-bit "mct_di" bus for each of them, that
> > carries the MIPI metadata. The incoming data is split into the four
> > outputs according to the MIPI virtual channel.
> > 
> > Two of these 16-bit + 8-bit parallel buses are routed through a
> > multiplexer before finally arriving at the CSI on the other side.
> > 
> > We need to configure the CSI to either use or ignore the data from the
> > 8-bit mct_di bus depending on whether the source of the mux is
> > configured to the MIPI CSI-2 receiver / CSI2IPU gasket, or to a parallel
> > input.
> 
> Philipp, from my experience, the CSI_MIPI_DI register (configured
> by ipu_csi_set_mipi_datatype()) can only be given a virtual channel 0,
> otherwise no data is received from the MIPI CSI-2 sensor, regardless
> of the virtual channel the sensor is transmitting over.
> 
> So it seems the info on the 8-bit mct_di buses generated by the CSI2IPU
> gasket are ignored by the CSI's, at least the virtual channel number is
> ignored.
> 
> For example, if the sensor is transmitting on vc 1, the gasket routes
> the sensor data to the parallel bus to CSI1. But if CSI_MIPI_DI on CSI1
> is written with vc 1, no data is received.
> 
> Steve
> 
> > Currently we let g_mbus_config pretend that even the internal 32-bit +
> > metadata and 16-bit + 8-bit metadata parallel buses are of type
> > V4L2_MBUS_CSI so that the CSI can ask the mux, which propagates to the
> > CSI-2 receiver, if connected.
> > 
> > Without g_mbus_config we'd need to get that information from somewhere
> > else. One possibility would be to extend MEDIA_BUS formats to describe
> > these "parallelized MIPI data" buses separately.

-- 
Regards,

Laurent Pinchart


Re: [PATCH v3 06/24] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors

2017-02-05 Thread Russell King - ARM Linux
On Sun, Feb 05, 2017 at 05:24:30PM +0200, Laurent Pinchart wrote:
> Hi Russell,
> 
> On Monday 30 Jan 2017 22:51:33 Russell King - ARM Linux wrote:
> > > + ov5640_to_mipi_csi: endpoint@1 {
> > > + reg = <1>;
> > > + remote-endpoint = 
> <_csi_from_mipi_sensor>;
> > > + data-lanes = <0 1>;
> > > + clock-lanes = <2>;
> > 
> > How do you envision a four-lane sensor being described?
> > 
> > data-lanes = <0 1 3 4>;
> > clock-lanes = <2>;
> > 
> > ?
> > 
> > The binding document for video-interfaces.txt says:
> > 
> > - clock-lanes: an array of physical clock lane indexes. Position of an entry
> > determines the logical lane number, while the value of an entry indicates
> > physical lane, e.g. for a MIPI CSI-2 bus we could have "clock-lanes =
> > <0>;", which places the clock lane on hardware lane 0. This property is
> > valid for serial busses only (e.g. MIPI CSI-2). Note that for the MIPI
> > CSI-2 bus this array contains only one entry.
> > 
> > So I think you need to have a good reason to make the clock lane non-zero.
> 
> The purpose of the data-lanes and clock-lanes properties is to describe lane 
> assignment for hardware that supports lane routing. As far as I know the 
> OV5640 doesn't support lane routing and has dedicated pins for the clock and 
> data lanes. The data-lanes and clock-lanes properties should probably not be 
> specified at all.

You need at least data-lanes so you know how many data lanes are wired
between the camera and the mipi csi2 receiver.  Just because a camera
has (eg) four lanes does not mean you need to wire all four, and in
some cases less than four will be wired.

If data-lanes does not describe that, then all existing users of the
binding are abusing it:

$ grep data_lanes drivers/media/i2c -r
drivers/media/i2c/s5k5baf.c:state->nlanes = 
ep.bus.mipi_csi2.num_data_lanes;
drivers/media/i2c/s5c73m3/s5c73m3-core.c:   if (ep.bus.mipi_csi2.num_data_lanes 
!= S5C73M3_MIPI_DATA_LANES)
drivers/media/i2c/tc358743.c:   endpoint->bus.mipi_csi2.num_data_lanes 
== 0 ||
drivers/media/i2c/smiapp/smiapp-core.c: hwcfg->lanes = 
bus_cfg->bus.mipi_csi2.num_data_lanes;

So all those drivers are using it for the _number_ of CSI2 lanes, and
are not touching the mapping in any way (not even checking that it is
an identity mapping.)  You could specify any mapping to these drivers,
as long as num_data_lanes came out right.

And... there's no point having a property in a binding if no one is
using it... and even more silly not to have a property that everyone
needs...

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3 12/24] add mux and video interface bridge entity functions

2017-02-05 Thread Laurent Pinchart
Hi Steve,

Thank you for the patch

On Friday 06 Jan 2017 18:11:30 Steve Longerbeam wrote:
> From: Philipp Zabel 
> 
> Signed-off-by: Philipp Zabel 
> ---
>  Documentation/media/uapi/mediactl/media-types.rst | 22 
>  include/uapi/linux/media.h|  6 ++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/media/uapi/mediactl/media-types.rst
> b/Documentation/media/uapi/mediactl/media-types.rst index 3e03dc2..023be29
> 100644
> --- a/Documentation/media/uapi/mediactl/media-types.rst
> +++ b/Documentation/media/uapi/mediactl/media-types.rst
> @@ -298,6 +298,28 @@ Types and flags used to represent the media graph
> elements received on its sink pad and outputs the statistics data on
> its source pad.
> 
> +-  ..  row 29
> +
> +   ..  _MEDIA-ENT-F-MUX:
> +
> +   -  ``MEDIA_ENT_F_MUX``
> +
> +   - Video multiplexer. An entity capable of multiplexing must have at
> + least two sink pads and one source pad, and must pass the video
> + frame(s) received from the active sink pad to the source pad.
> Video
> + frame(s) from the inactive sink pads are discarded.

Apart from the comment made by Hans regarding the macro name, this looks good 
to me.

> +-  ..  row 30
> +
> +   ..  _MEDIA-ENT-F-VID-IF-BRIDGE:
> +
> +   -  ``MEDIA_ENT_F_VID_IF_BRIDGE``
> +
> +   - Video interface bridge. A video interface bridge entity must have
> at
> + least one sink pad and one source pad. It receives video frame(s)
> on
> + its sink pad in one bus format (HDMI, eDP, MIPI CSI-2, ...) and
> + converts them and outputs them on its source pad in another bus
> format
> + (eDP, MIPI CSI-2, parallel, ...).


The first sentence mentions *at least* one sink pad and one source pad, and 
the second one refers to "its sink pad" and "its source pad". This is a bit 
confusing. An easy option would be to require a single sink and a single 
source pad, but that would exclude bridges that combine a multiplexer.

I also wonder whether "bridge" is the appropriate word here. Transceiver might 
be a better choice, to insist on the fact that one of the two pads corresponds 
to a physical interface that has special electrical properties (such as HDMI, 
eDP, or CSI-2 that all require PHYs).

>  ..  tabularcolumns:: |p{5.5cm}|p{12.0cm}|
> 
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 4890787..08a8bfa 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -105,6 +105,12 @@ struct media_device_info {
>  #define MEDIA_ENT_F_PROC_VIDEO_STATISTICS(MEDIA_ENT_F_BASE + 0x4006)
> 
>  /*
> + * Switch and bridge entitites
> + */
> +#define MEDIA_ENT_F_MUX  (MEDIA_ENT_F_BASE + 
0x5001)
> +#define MEDIA_ENT_F_VID_IF_BRIDGE(MEDIA_ENT_F_BASE + 0x5002)
> +
> +/*
>   * Connectors
>   */
>  /* It is a responsibility of the entity drivers to add connectors and links
> */

-- 
Regards,

Laurent Pinchart

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3 06/24] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors

2017-02-05 Thread Laurent Pinchart
Hi Russell,

On Monday 30 Jan 2017 22:51:33 Russell King - ARM Linux wrote:
> On Fri, Jan 06, 2017 at 06:11:24PM -0800, Steve Longerbeam wrote:
> > +   ov5640: camera@40 {
> > +   compatible = "ovti,ov5640";
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_ov5640>;
> > +   clocks = <_xclk>;
> > +   clock-names = "xclk";
> > +   reg = <0x40>;
> > +   xclk = <2200>;
> > +   reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* NANDF_D5 */
> > +   pwdn-gpios = < 9 GPIO_ACTIVE_HIGH>; /* NANDF_WP_B */
> > +
> > +   port {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   ov5640_to_mipi_csi: endpoint@1 {
> > +   reg = <1>;
> > +   remote-endpoint = 
<_csi_from_mipi_sensor>;
> > +   data-lanes = <0 1>;
> > +   clock-lanes = <2>;
> 
> How do you envision a four-lane sensor being described?
> 
>   data-lanes = <0 1 3 4>;
>   clock-lanes = <2>;
> 
> ?
> 
> The binding document for video-interfaces.txt says:
> 
> - clock-lanes: an array of physical clock lane indexes. Position of an entry
> determines the logical lane number, while the value of an entry indicates
> physical lane, e.g. for a MIPI CSI-2 bus we could have "clock-lanes =
> <0>;", which places the clock lane on hardware lane 0. This property is
> valid for serial busses only (e.g. MIPI CSI-2). Note that for the MIPI
> CSI-2 bus this array contains only one entry.
> 
> So I think you need to have a good reason to make the clock lane non-zero.

The purpose of the data-lanes and clock-lanes properties is to describe lane 
assignment for hardware that supports lane routing. As far as I know the 
OV5640 doesn't support lane routing and has dedicated pins for the clock and 
data lanes. The data-lanes and clock-lanes properties should probably not be 
specified at all.

-- 
Regards,

Laurent Pinchart

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3 06/24] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors

2017-02-05 Thread Laurent Pinchart
On Monday 16 Jan 2017 13:55:23 Philipp Zabel wrote:
> On Fri, 2017-01-13 at 15:04 -0800, Steve Longerbeam wrote:
> > On 01/13/2017 04:03 AM, Philipp Zabel wrote:
> > > Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> > >> Enables the OV5642 parallel-bus sensor, and the OV5640 MIPI CSI-2
> > >> sensor.
> > >> Both hang off the same i2c2 bus, so they require different (and non-
> > >> default) i2c slave addresses.
> > >> 
> > >> The OV5642 connects to the parallel-bus mux input port on
> > >> ipu1_csi0_mux.
> > >> 
> > >> The OV5640 connects to the input port on the MIPI CSI-2 receiver on
> > >> mipi_csi. It is set to transmit over MIPI virtual channel 1.
> > >> 
> > >> Signed-off-by: Steve Longerbeam 
> > >> ---
> > >> 
> > >>   arch/arm/boot/dts/imx6dl-sabrelite.dts   |   5 ++
> > >>   arch/arm/boot/dts/imx6q-sabrelite.dts|   6 ++
> > >>   arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 118 ++
> > >>   3 files changed, 129 insertions(+)
> > >> 
> > >> diff --git a/arch/arm/boot/dts/imx6dl-sabrelite.dts
> > >> b/arch/arm/boot/dts/imx6dl-sabrelite.dts index 0f06ca5..fec2524 100644
> > >> --- a/arch/arm/boot/dts/imx6dl-sabrelite.dts
> > >> +++ b/arch/arm/boot/dts/imx6dl-sabrelite.dts
> 
> [...]
> 
> > >> @@ -299,6 +326,52 @@
> > >> 
> > >>  pinctrl-names = "default";
> > >>  pinctrl-0 = <_i2c2>;
> > >>  status = "okay";
> > >> 
> > >> +
> > >> +ov5640: camera@40 {
> > >> +compatible = "ovti,ov5640";
> > >> +pinctrl-names = "default";
> > >> +pinctrl-0 = <_ov5640>;
> > >> +clocks = <_xclk>;
> > >> +clock-names = "xclk";
> > >> +reg = <0x40>;
> > >> +xclk = <2200>;
> > > 
> > > This is superfluous, you can use clk_get_rate on mipi_xclk.
> > 
> > This property is actually there to tell the driver what to set the
> > rate to, with clk_set_rate(). So you are saying it would be better
> > to set the rate in the device tree and the driver should only
> > retrieve the rate?
> 
> Yes. Given that this is a reference clock input that is constant on a
> given board and never changes during runtime, I think this is the
> correct way. The clock will be fixed rate on most boards, I assume.

I think it's a bit worse than that. The ov5640 and ov5642 drivers should 
retrieve the clock rate and compute register values accordingly (PLL 
configuration parameters for instance, but most probably other values as 
well). Unfortunately, as usual with Omnivision, the lack of public and even 
non-public information results in drivers hardcoding large lists of 
register/value pairs that have been computed for a specific clock frequency. 
The drivers will thus not operate correctly if the clock is running at a 
different rate. Until that can be fixed, the best option is probably to assign 
the rate in the device tree as Philipp proposed, and to use clk_get_rate() in 
the driver to reject any rate other than 22MHz.

-- 
Regards,

Laurent Pinchart

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: bcm2835-audio: remove unused macro

2017-02-05 Thread Greg Kroah-Hartman
On Fri, Feb 03, 2017 at 07:50:09PM +0100, Hendrik v. Raven wrote:
> The VC_AUDIO_MAX_MSG_LEN macro is not used anywhere and has coding style
> violations.
> 
> Signed-off-by: Hendrik v. Raven 

Does not apply to my tree :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: fbtft: fbtft-bus.c: Fix checkpatch error

2017-02-05 Thread Greg KH
On Sat, Feb 04, 2017 at 01:42:48PM +0900, Youngdo Lee wrote:
> Fix checkpatch error:
> ERROR: space prohibited before that close parenthesis ')'
> 
> Signed-off-by: Youngdo Lee 
> ---
>  drivers/staging/fbtft/fbtft-bus.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/fbtft/fbtft-bus.c 
> b/drivers/staging/fbtft/fbtft-bus.c
> index ec45043..3ce265d 100644
> --- a/drivers/staging/fbtft/fbtft-bus.c
> +++ b/drivers/staging/fbtft/fbtft-bus.c
> @@ -10,6 +10,7 @@
>   *
>   
> */
>  
> +#define  nop_modifier(expr)  (expr)

What?

>  #define define_fbtft_write_reg(func, type, modifier) 
>  \
>  void func(struct fbtft_par *par, int len, ...)   
>  \
>  {
>  \
> @@ -68,9 +69,9 @@ void func(struct fbtft_par *par, int len, ...)  
>   \
>  }
>  \
>  EXPORT_SYMBOL(func);
>  
> -define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, )
> +define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, nop_modifier)

That's really odd.  Don't do crazy things like this to fix odd code.
Fix up the code to not do such looney things in the first place.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/1] iio: fixed symbolic permission references 'S_IRUGO | S_IWUSR'

2017-02-05 Thread Jonathan Cameron
On 04/02/17 19:11, Artur Lorincz wrote:
> Replaced the symbolic permission references S_IRUGO and S_IWUSR with their
> octal counterparts.
> 
> Signed-off-by: Artur Lorincz 
Applied with a small change to the description to mention which driver we
are dealing with.

Also, for IIO patches please cc linux-iio as now done.

Jonathan
> ---
>  drivers/staging/iio/adc/ad7192.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c 
> b/drivers/staging/iio/adc/ad7192.c
> index 1fb68c0..4fc8588 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -342,9 +342,9 @@ static int ad7192_setup(struct ad7192_state *st,
> 
>  static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available,
>in_voltage-voltage_scale_available,
> -  S_IRUGO, ad7192_show_scale_available, NULL, 0);
> +  0444, ad7192_show_scale_available, NULL, 0);
> 
> -static IIO_DEVICE_ATTR(in_voltage_scale_available, S_IRUGO,
> +static IIO_DEVICE_ATTR(in_voltage_scale_available, 0444,
>  ad7192_show_scale_available, NULL, 0);
> 
>  static ssize_t ad7192_show_ac_excitation(struct device *dev,
> @@ -412,11 +412,11 @@ static ssize_t ad7192_set(struct device *dev,
>   return ret ? ret : len;
>  }
> 
> -static IIO_DEVICE_ATTR(bridge_switch_en, S_IRUGO | S_IWUSR,
> +static IIO_DEVICE_ATTR(bridge_switch_en, 0644,
>  ad7192_show_bridge_switch, ad7192_set,
>  AD7192_REG_GPOCON);
> 
> -static IIO_DEVICE_ATTR(ac_excitation_en, S_IRUGO | S_IWUSR,
> +static IIO_DEVICE_ATTR(ac_excitation_en, 0644,
>  ad7192_show_ac_excitation, ad7192_set,
>  AD7192_REG_MODE);
> 
> --
> 1.9.1
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel