Re: [PATCH v1 1/9] staging: android: binder: Move some of the logic into subfunction
On Wed, Dec 04, 2013 at 06:09:33PM +, Serban Constantinescu wrote:
> +static void bc_dead_binder_done(struct binder_proc *proc,
> + struct binder_thread *thread, void __user *cookie)
> +{
> + struct binder_work *w;
> + struct binder_ref_death *death = NULL;
> +
> + list_for_each_entry(w, &proc->delivered_death, entry) {
> + struct binder_ref_death *tmp_death = container_of(w, struct
> binder_ref_death, work);
Put a blank line after the variable declaration block. Also break it up
into two lines instead of having the lines be a million characters long.
list_for_each_entry(w, &proc->delivered_death, entry) {
struct binder_ref_death *tmp_death;
tmp_death = container_of(w, struct binder_ref_death, work);
> + if (tmp_death->cookie == cookie) {
> + death = tmp_death;
> + return;
Should be a break here. This function wasn't tested.
> + }
> + }
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 1/9] staging: android: binder: Move some of the logic into subfunction
On Wed, Dec 04, 2013 at 06:09:33PM +, Serban Constantinescu wrote:
> +static void bc_increfs_done(struct binder_proc *proc,
> + struct binder_thread *thread, uint32_t cmd,
> + void __user *node_ptr, void __user *cookie)
> +{
> + struct binder_node *node;
> +
> + node = binder_get_node(proc, node_ptr);
> + if (node == NULL) {
> + binder_user_error("%d:%d %s u%p no match\n",
> + proc->pid, thread->pid,
> + cmd == BC_INCREFS_DONE ?
> + "BC_INCREFS_DONE" :
> + "BC_ACQUIRE_DONE",
> + node_ptr);
> + return;
> + }
> + if (cookie != node->cookie) {
> + binder_user_error("%d:%d %s u%p node %d cookie mismatch %p !=
> %p\n",
> + proc->pid, thread->pid,
> + cmd == BC_INCREFS_DONE ?
> + "BC_INCREFS_DONE" : "BC_ACQUIRE_DONE",
> + node_ptr, node->debug_id,
> + cookie, node->cookie);
> + return;
> + }
> + if (cmd == BC_ACQUIRE_DONE) {
> + if (node->pending_strong_ref == 0) {
> + binder_user_error("%d:%d BC_ACQUIRE_DONE node %d has no
> pending acquire request\n",
> + proc->pid, thread->pid,
> + node->debug_id);
> + return;
> + }
> + node->pending_strong_ref = 0;
> + } else {
> + if (node->pending_weak_ref == 0) {
> + binder_user_error("%d:%d BC_INCREFS_DONE node %d has no
> pending increfs request\n",
> + proc->pid, thread->pid,
> + node->debug_id);
> + return;
> + }
> + node->pending_weak_ref = 0;
> + }
> + binder_dec_node(node, cmd == BC_ACQUIRE_DONE, 0);
> + binder_debug(BINDER_DEBUG_USER_REFS,
> + "%d:%d %s node %d ls %d lw %d\n",
> + proc->pid, thread->pid,
> + cmd == BC_INCREFS_DONE ?
> + "BC_INCREFS_DONE" :
> + "BC_ACQUIRE_DONE",
> + node->debug_id, node->local_strong_refs,
> + node->local_weak_refs);
> + return;
> +}
This function is 52 lines long but at least 32 of those lines are
debug code.
Just extra of lines of code means you have increased the number of bugs
150%. But what I know is that from a static analysis perspective, debug
code has more defects per line then regular code it's worse than that.
Plus all the extra noise obscures the actual logic and makes the real
code annoying to look at so it's worse still.
If you want to move this stuff out of staging, then remove all the extra
crap so it doesn't look like barf.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 2/9] staging: android: binder: Add binder_copy_to_user()
On Wed, Dec 04, 2013 at 06:09:34PM +, Serban Constantinescu wrote:
> This patch adds binder_copy_to_user() to be used for copying binder
> commands to user address space. This way we can abstract away the
> copy_to_user() calls and add separate handling for the compat layer.
>
> Signed-off-by: Serban Constantinescu
> ---
> drivers/staging/android/binder.c | 39
> --
> 1 file changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/staging/android/binder.c
> b/drivers/staging/android/binder.c
> index 233889c..6fbb340 100644
> --- a/drivers/staging/android/binder.c
> +++ b/drivers/staging/android/binder.c
> @@ -2117,6 +2117,18 @@ static int binder_has_thread_work(struct binder_thread
> *thread)
> (thread->looper & BINDER_LOOPER_STATE_NEED_RETURN);
> }
>
> +static int binder_copy_to_user(uint32_t cmd, void *parcel,
> +void __user **ptr, size_t size)
> +{
Bike shedding: Put the **ptr as the first argument.
binder_copy_to_user(dest, cmd, src, size);
> + if (put_user(cmd, (uint32_t __user *)*ptr))
> + return -EFAULT;
> + *ptr += sizeof(uint32_t);
> + if (copy_to_user(*ptr, parcel, size))
> + return -EFAULT;
> + *ptr += size;
> + return 0;
> +}
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 3/9] staging: android: binder: Add cmd == CMD_NAME handling
On Wed, Dec 04, 2013 at 06:09:35PM +, Serban Constantinescu wrote: > This patch modifies the functions that need to be passed the explicit > command to use a boolean flag. This way we can reuse the code for 64bit > compat commands. > I don't understand this at all. cmd seems like it should be 32 bits on both arches. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 4/9] staging: android: binder: Add align_helper() macro
On Wed, Dec 04, 2013 at 06:09:36PM +, Serban Constantinescu wrote: > This patch adds align_helper() macro that will be used for enforcing > the desired alignment on 64bit systems where the alignment will differ > depending on the userspace used (32bit /64bit). > > This patch is a temporary patch that will be extended with 32bit compat > handling. > > Signed-off-by: Serban Constantinescu > --- > drivers/staging/android/binder.c |9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/android/binder.c > b/drivers/staging/android/binder.c > index 16109cd..6719a53 100644 > --- a/drivers/staging/android/binder.c > +++ b/drivers/staging/android/binder.c > @@ -140,6 +140,8 @@ module_param_call(stop_on_user_error, > binder_set_stop_on_user_error, > binder_stop_on_user_error = 2; \ > } while (0) > > +#define align_helper(ptr)ALIGN(ptr, sizeof(void *)) > + This is a terrible name. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/5] Staging: bcm: DDRInit: Fixed coding style issue, replaced spaces w/ tabs.(patch set)
These five patches all do the same thing. It's ok to merge them into one patch. The subject lines are all the same and that's not ok. The subject lines are too long as well. On Wed, Dec 04, 2013 at 07:26:19PM -0500, Gary Rookard wrote: > This is the first patch of a series. Don't put this in the description. > Replaced spaces in margin w/ 1 tab for lines: > 11-15, 17-23, 25-58, 60, 62, 64 > No signed off by line. > On branch staging-next Put this between the --- and the diffstat. > --- <-- here. > drivers/staging/bcm/DDRInit.c | 98 > +-- > 1 file changed, 49 insertions(+), 49 deletions(-) regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 34/39] vme: remove DEFINE_PCI_DEVICE_TABLE macro
On Tue, Dec 3, 2013 at 7:29 AM, Jingoo Han wrote:
> Don't use DEFINE_PCI_DEVICE_TABLE macro, because this macro
> is not preferred.
>
> Signed-off-by: Jingoo Han
> ---
Greg, this patch should be reverted. It do the opposite things.
> drivers/vme/boards/vme_vmivme7805.c |2 +-
> drivers/vme/bridges/vme_ca91cx42.c |2 +-
> drivers/vme/bridges/vme_tsi148.c|2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vme/boards/vme_vmivme7805.c
> b/drivers/vme/boards/vme_vmivme7805.c
> index cf74aee..ac42212 100644
> --- a/drivers/vme/boards/vme_vmivme7805.c
> +++ b/drivers/vme/boards/vme_vmivme7805.c
> @@ -27,7 +27,7 @@ static void __iomem *vmic_base;
>
> static const char driver_name[] = "vmivme_7805";
>
> -static DEFINE_PCI_DEVICE_TABLE(vmic_ids) = {
> +static const struct pci_device_id vmic_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_VMIC, PCI_DEVICE_ID_VTIMR) },
> { },
> };
> diff --git a/drivers/vme/bridges/vme_ca91cx42.c
> b/drivers/vme/bridges/vme_ca91cx42.c
> index f844857..a06edbf 100644
> --- a/drivers/vme/bridges/vme_ca91cx42.c
> +++ b/drivers/vme/bridges/vme_ca91cx42.c
> @@ -42,7 +42,7 @@ static int geoid;
>
> static const char driver_name[] = "vme_ca91cx42";
>
> -static DEFINE_PCI_DEVICE_TABLE(ca91cx42_ids) = {
> +static const struct pci_device_id ca91cx42_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_TUNDRA, PCI_DEVICE_ID_TUNDRA_CA91C142) },
> { },
> };
> diff --git a/drivers/vme/bridges/vme_tsi148.c
> b/drivers/vme/bridges/vme_tsi148.c
> index 9cf8833..16830d8 100644
> --- a/drivers/vme/bridges/vme_tsi148.c
> +++ b/drivers/vme/bridges/vme_tsi148.c
> @@ -45,7 +45,7 @@ static int geoid;
>
> static const char driver_name[] = "vme_tsi148";
>
> -static DEFINE_PCI_DEVICE_TABLE(tsi148_ids) = {
> +static const struct pci_device_id tsi148_ids[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_TUNDRA, PCI_DEVICE_ID_TUNDRA_TSI148) },
> { },
> };
> --
> 1.7.10.4
>
>
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 28/39] staging: remove DEFINE_PCI_DEVICE_TABLE macro
On Tue, Dec 3, 2013 at 7:26 AM, Jingoo Han wrote: > Don't use DEFINE_PCI_DEVICE_TABLE macro, because this macro > is not preferred. > > Signed-off-by: Jingoo Han > I think you misunderstood the checkpatch.pl warning, it tells you what to do, not what not to do. WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id This means use DEFINE_PCI_DEVICE_TABLE to replace struct pci_device_id, not reverse. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/11] staging: et131x: drop packet when error occurs in et131x_tx
On Wed, Dec 04, 2013 at 03:23:29PM -0800, Greg Kroah-Hartman wrote: > On Wed, Dec 04, 2013 at 03:24:14PM +0800, ZHAO Gang wrote: > > As TODO file suggested, drop packet instead of return NETDEV_TX_BUSY > > when tx failed. > > Really? That's ok to do? Seems like you are changing the logic of the > function a lot here, how does the code let userspace know packets were > dropped then? The TODO file item was a suggestion by David Miller from the last review - http://lkml.org/lkml/2013/1/28/756 Mark ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 1/9] staging: android: binder: Move some of the logic into subfunction
On Thu, Dec 05, 2013 at 11:18:22AM +0300, Dan Carpenter wrote:
> On Wed, Dec 04, 2013 at 06:09:33PM +, Serban Constantinescu wrote:
> > +static void bc_increfs_done(struct binder_proc *proc,
> > + struct binder_thread *thread, uint32_t cmd,
> > + void __user *node_ptr, void __user *cookie)
> > +{
> > + struct binder_node *node;
> > +
> > + node = binder_get_node(proc, node_ptr);
> > + if (node == NULL) {
> > + binder_user_error("%d:%d %s u%p no match\n",
> > + proc->pid, thread->pid,
> > + cmd == BC_INCREFS_DONE ?
> > + "BC_INCREFS_DONE" :
> > + "BC_ACQUIRE_DONE",
> > + node_ptr);
> > + return;
> > + }
> > + if (cookie != node->cookie) {
> > + binder_user_error("%d:%d %s u%p node %d cookie mismatch %p !=
> > %p\n",
> > + proc->pid, thread->pid,
> > + cmd == BC_INCREFS_DONE ?
> > + "BC_INCREFS_DONE" : "BC_ACQUIRE_DONE",
> > + node_ptr, node->debug_id,
> > + cookie, node->cookie);
> > + return;
> > + }
> > + if (cmd == BC_ACQUIRE_DONE) {
> > + if (node->pending_strong_ref == 0) {
> > + binder_user_error("%d:%d BC_ACQUIRE_DONE node %d has no
> > pending acquire request\n",
> > + proc->pid, thread->pid,
> > + node->debug_id);
> > + return;
> > + }
> > + node->pending_strong_ref = 0;
> > + } else {
> > + if (node->pending_weak_ref == 0) {
> > + binder_user_error("%d:%d BC_INCREFS_DONE node %d has no
> > pending increfs request\n",
> > + proc->pid, thread->pid,
> > + node->debug_id);
> > + return;
> > + }
> > + node->pending_weak_ref = 0;
> > + }
> > + binder_dec_node(node, cmd == BC_ACQUIRE_DONE, 0);
> > + binder_debug(BINDER_DEBUG_USER_REFS,
> > +"%d:%d %s node %d ls %d lw %d\n",
> > +proc->pid, thread->pid,
> > +cmd == BC_INCREFS_DONE ?
> > +"BC_INCREFS_DONE" :
> > +"BC_ACQUIRE_DONE",
> > +node->debug_id, node->local_strong_refs,
> > +node->local_weak_refs);
> > + return;
> > +}
>
> This function is 52 lines long but at least 32 of those lines are
> debug code.
>
> Just extra of lines of code means you have increased the number of bugs
> 150%. But what I know is that from a static analysis perspective, debug
> code has more defects per line then regular code it's worse than that.
> Plus all the extra noise obscures the actual logic and makes the real
> code annoying to look at so it's worse still.
>
> If you want to move this stuff out of staging, then remove all the extra
> crap so it doesn't look like barf.
I don't ever think the binder code will be moved out of staging for a
variety of reasons, but that doesn't mean it shouldn't be cleaned up :)
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 34/39] vme: remove DEFINE_PCI_DEVICE_TABLE macro
On Thu, Dec 05, 2013 at 05:14:37PM +0800, ZHAO Gang wrote: > On Tue, Dec 3, 2013 at 7:29 AM, Jingoo Han wrote: > > Don't use DEFINE_PCI_DEVICE_TABLE macro, because this macro > > is not preferred. > > > > Signed-off-by: Jingoo Han > > --- > > Greg, this patch should be reverted. It do the opposite things. Not at all, it is correct. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 28/39] staging: remove DEFINE_PCI_DEVICE_TABLE macro
On Thu, Dec 05, 2013 at 05:06:33PM +0800, ZHAO Gang wrote: > On Tue, Dec 3, 2013 at 7:26 AM, Jingoo Han wrote: > > Don't use DEFINE_PCI_DEVICE_TABLE macro, because this macro > > is not preferred. > > > > Signed-off-by: Jingoo Han > > > > I think you misunderstood the checkpatch.pl warning, it tells you what > to do, not what not to do. > > WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id > > This means use DEFINE_PCI_DEVICE_TABLE to replace struct > pci_device_id, not reverse. No, checkpatch is wrong, and is being fixed, this patch is correct. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 00/51] staging: comedi: cleanup irq requests
On Wednesday, December 04, 2013 4:47 AM, Ian Abbott wrote: > On 2013-12-03 19:07, H Hartley Sweeten wrote: >> The comedi subsystem only requires the drivers to support interrupts if one >> or more of the subdevices support async commands. Since this is optional: >> >> 1) don't fail the attach if the irq is not available >> 2) only hookup the async command support if the irq is available >> 3) remove any async command init from subdevices that don't need it >> 4) remove any unnecessary sanity checks in the async command functions >> >> Make use of the comedi_device 'read_subdev' and 'write_subdev' pointers >> instead of accessing the dev->subdevices array directly with "magic" numbers >> to the correct subdevice. >> >> Also, remove any unnecessary debug noise associated with the irq requests. > > I have issues with patches 18, 19, 22, and 49, although patch 19 is > fixed by patch 20. > > Patch 27 may be considered for stable kernels 3.7 and later if it > applies cleanly (I need to check). Ian, Greg has applied patches 1-17 to his tree. I will drop the offending pieces in patches 18, 19, 22, and 49 and repost the series later today. I'll leave the backport of patch 27 to you. Thanks, Hartley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv5][ 2/8] staging: imx-drm: Add RGB666 support for parallel display.
Cc: Rob Herring
Cc: Pawel Moll
Cc: Mark Rutland
Cc: Stephen Warren
Cc: Ian Campbell
Cc: devicet...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: driverdev-devel@linuxdriverproject.org
Cc: David Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: Mauro Carvalho Chehab
Cc: Laurent Pinchart
Cc: linux-me...@vger.kernel.org
Cc: Sascha Hauer
Cc: Shawn Guo
Cc: linux-arm-ker...@lists.infradead.org
Cc: Eric Bénard
Signed-off-by: Denis Carikli
---
ChangeLog v3->v5:
- Use the correct RGB order.
ChangeLog v2->v3:
- Added some interested people in the Cc list.
- Removed the commit message long desciption that was just a copy of the short
description.
- Rebased the patch.
- Fixed a copy-paste error in the ipu_dc_map_clear parameter.
---
.../bindings/staging/imx-drm/fsl-imx-drm.txt |2 +-
drivers/staging/imx-drm/ipu-v3/ipu-dc.c|9 +
drivers/staging/imx-drm/parallel-display.c |2 ++
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
index b876d49..2d24425 100644
--- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
+++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
@@ -29,7 +29,7 @@ Required properties:
- crtc: the crtc this display is connected to, see below
Optional properties:
- interface_pix_fmt: How this display is connected to the
- crtc. Currently supported types: "rgb24", "rgb565", "bgr666"
+ crtc. Currently supported types: "rgb24", "rgb565", "bgr666", "rgb666"
- edid: verbatim EDID data block describing attached display.
- ddc: phandle describing the i2c bus handling the display data
channel
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
index d0e3bc3..617e65b 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
@@ -92,6 +92,7 @@ enum ipu_dc_map {
IPU_DC_MAP_GBR24, /* TVEv2 */
IPU_DC_MAP_BGR666,
IPU_DC_MAP_BGR24,
+ IPU_DC_MAP_RGB666,
};
struct ipu_dc {
@@ -155,6 +156,8 @@ static int ipu_pixfmt_to_map(u32 fmt)
return IPU_DC_MAP_BGR666;
case V4L2_PIX_FMT_BGR24:
return IPU_DC_MAP_BGR24;
+ case V4L2_PIX_FMT_RGB666:
+ return IPU_DC_MAP_RGB666;
default:
return -EINVAL;
}
@@ -404,6 +407,12 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 1, 15, 0xff); /* green */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 0, 23, 0xff); /* blue */
+ /* rgb666 */
+ ipu_dc_map_clear(priv, IPU_DC_MAP_RGB666);
+ ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */
+ ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */
+ ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */
+
return 0;
}
diff --git a/drivers/staging/imx-drm/parallel-display.c
b/drivers/staging/imx-drm/parallel-display.c
index 24aa9be..bb71d6d 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -222,6 +222,8 @@ static int imx_pd_probe(struct platform_device *pdev)
imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB565;
else if (!strcmp(fmt, "bgr666"))
imxpd->interface_pix_fmt = V4L2_PIX_FMT_BGR666;
+ else if (!strcmp(fmt, "rgb666"))
+ imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB666;
}
imxpd->dev = &pdev->dev;
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv5][ 3/8] staging: imx-drm: Correct BGR666 and the board's dts that use them.
Cc: Rob Herring
Cc: Pawel Moll
Cc: Mark Rutland
Cc: Stephen Warren
Cc: Ian Campbell
Cc: devicet...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: driverdev-devel@linuxdriverproject.org
Cc: David Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: Mauro Carvalho Chehab
Cc: Laurent Pinchart
Cc: linux-me...@vger.kernel.org
Cc: Sascha Hauer
Cc: Shawn Guo
Cc: linux-arm-ker...@lists.infradead.org
Cc: Eric Bénard
Signed-off-by: Denis Carikli
---
ChangeLog v5:
- New patch.
---
arch/arm/boot/dts/imx51-apf51dev.dts|2 +-
arch/arm/boot/dts/imx53-m53evk.dts |2 +-
drivers/staging/imx-drm/ipu-v3/ipu-dc.c |4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/imx51-apf51dev.dts
b/arch/arm/boot/dts/imx51-apf51dev.dts
index f36a3aa..3b6de6a 100644
--- a/arch/arm/boot/dts/imx51-apf51dev.dts
+++ b/arch/arm/boot/dts/imx51-apf51dev.dts
@@ -19,7 +19,7 @@
display@di1 {
compatible = "fsl,imx-parallel-display";
crtcs = <&ipu 0>;
- interface-pix-fmt = "bgr666";
+ interface-pix-fmt = "rgb666";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ipu_disp1>;
diff --git a/arch/arm/boot/dts/imx53-m53evk.dts
b/arch/arm/boot/dts/imx53-m53evk.dts
index c623774..b98c897 100644
--- a/arch/arm/boot/dts/imx53-m53evk.dts
+++ b/arch/arm/boot/dts/imx53-m53evk.dts
@@ -24,7 +24,7 @@
display@di1 {
compatible = "fsl,imx-parallel-display";
crtcs = <&ipu 1>;
- interface-pix-fmt = "bgr666";
+ interface-pix-fmt = "rgb666";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ipu_disp1>;
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
index 617e65b..b11a2aa 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
@@ -397,9 +397,9 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
/* bgr666 */
ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
- ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 5, 0xfc); /* blue */
+ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 17, 0xfc); /* blue */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /* green */
- ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 17, 0xfc); /* red */
+ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 5, 0xfc); /* red */
/* bgr24 */
ipu_dc_map_clear(priv, IPU_DC_MAP_BGR24);
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv5][ 1/8] [media] v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.
That new macro is needed by the imx_drm staging driver
for supporting the QVGA display of the eukrea-cpuimx51 board.
Cc: Rob Herring
Cc: Pawel Moll
Cc: Mark Rutland
Cc: Stephen Warren
Cc: Ian Campbell
Cc: devicet...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: driverdev-devel@linuxdriverproject.org
Cc: David Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: Mauro Carvalho Chehab
Cc: Laurent Pinchart
Cc: linux-me...@vger.kernel.org
Cc: Sascha Hauer
Cc: Shawn Guo
Cc: linux-arm-ker...@lists.infradead.org
Cc: Eric Bénard
Signed-off-by: Denis Carikli
Acked-by: Mauro Carvalho Chehab
Acked-by: Laurent Pinchart
---
ChangeLog v3->v4:
- Added Laurent Pinchart's Ack.
ChangeLog v2->v3:
- Added some interested people in the Cc list.
- Added Mauro Carvalho Chehab's Ack.
- Added documentation.
---
.../DocBook/media/v4l/pixfmt-packed-rgb.xml| 78
include/uapi/linux/videodev2.h |1 +
2 files changed, 79 insertions(+)
diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
index 166c8d6..f6a3e84 100644
--- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
@@ -279,6 +279,45 @@ colorspace
V4L2_COLORSPACE_SRGB.
+
+ V4L2_PIX_FMT_RGB666
+ 'RGBH'
+
+ r5
+ r4
+ r3
+ r2
+ r1
+ r0
+ g5
+ g4
+
+ g3
+ g2
+ g1
+ g0
+ b5
+ b4
+ b3
+ b2
+
+ b1
+ b0
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
V4L2_PIX_FMT_BGR24
'BGR3'
@@ -781,6 +820,45 @@ defined in error. Drivers may interpret them as in
+
+ V4L2_PIX_FMT_RGB666
+ 'RGBH'
+
+ r5
+ r4
+ r3
+ r2
+ r1
+ r0
+ g5
+ g4
+
+ g3
+ g2
+ g1
+ g0
+ b5
+ b4
+ b3
+ b2
+
+ b1
+ b0
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
V4L2_PIX_FMT_BGR24
'BGR3'
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 437f1b0..e8ff410 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -294,6 +294,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_RGB555X v4l2_fourcc('R', 'G', 'B', 'Q') /* 16 RGB-5-5-5
BE */
#define V4L2_PIX_FMT_RGB565X v4l2_fourcc('R', 'G', 'B', 'R') /* 16 RGB-5-6-5
BE */
#define V4L2_PIX_FMT_BGR666 v4l2_fourcc('B', 'G', 'R', 'H') /* 18 BGR-6-6-6
*/
+#define V4L2_PIX_FMT_RGB666 v4l2_fourcc('R', 'G', 'B', 'H') /* 18 RGB-6-6-6
*/
#define V4L2_PIX_FMT_BGR24 v4l2_fourcc('B', 'G', 'R', '3') /* 24 BGR-8-8-8
*/
#define V4L2_PIX_FMT_RGB24 v4l2_fourcc('R', 'G', 'B', '3') /* 24 RGB-8-8-8
*/
#define V4L2_PIX_FMT_BGR32 v4l2_fourcc('B', 'G', 'R', '4') /* 32
BGR-8-8-8-8 */
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv5][ 4/8] staging: imx-drm: Use de-active and pixelclk-active display-timings.
If de-active and/or pixelclk-active properties were set in the
display-timings DT node, they were not used.
Instead the data-enable and the pixel data clock polarity
were hardcoded.
This change is needed for making the eukrea-cpuimx51
QVGA display work.
Greg Kroah-Hartman
Cc: driverdev-devel@linuxdriverproject.org
Cc: Philipp Zabel
Cc: Sascha Hauer
Cc: Shawn Guo
Cc: linux-arm-ker...@lists.infradead.org
Cc: David Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: Eric Bénard
Signed-off-by: Denis Carikli
---
ChangeLog v3->v4:
- The old patch was named "staging: imx-drm: ipuv3-crtc: don't harcode some
mode".
- Reworked the patch entierly: we now takes the mode flags from the device tree.
ChangeLog v2->v3:
- Added some interested people in the Cc list.
- Ajusted the flags to match the changes in "drm: Add the lacking
DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*"
---
drivers/staging/imx-drm/imx-drm.h |3 +++
drivers/staging/imx-drm/ipuv3-crtc.c | 11 +--
drivers/staging/imx-drm/parallel-display.c | 28
3 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/imx-drm/imx-drm.h
b/drivers/staging/imx-drm/imx-drm.h
index ae90c9c..dfdc180 100644
--- a/drivers/staging/imx-drm/imx-drm.h
+++ b/drivers/staging/imx-drm/imx-drm.h
@@ -5,6 +5,9 @@
#define IPU_PIX_FMT_GBR24 v4l2_fourcc('G', 'B', 'R', '3')
+#define IMXDRM_MODE_FLAG_DE_HIGH (1<<0)
+#define IMXDRM_MODE_FLAG_PIXDATA_POSEDGE (1<<1)
+
struct drm_crtc;
struct drm_connector;
struct drm_device;
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c
b/drivers/staging/imx-drm/ipuv3-crtc.c
index ce6ba98..f3d2cae 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -156,8 +156,15 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
if (mode->flags & DRM_MODE_FLAG_PVSYNC)
sig_cfg.Vsync_pol = 1;
- sig_cfg.enable_pol = 1;
- sig_cfg.clk_pol = 1;
+ /* Such flags are not availables in the DRM modes header,
+* and we don't want to export them to userspace.
+*/
+ if (mode->private_flags & IMXDRM_MODE_FLAG_DE_HIGH)
+ sig_cfg.enable_pol = 1;
+
+ if (mode->private_flags & IMXDRM_MODE_FLAG_PIXDATA_POSEDGE)
+ sig_cfg.clk_pol = 1;
+
sig_cfg.width = mode->hdisplay;
sig_cfg.height = mode->vdisplay;
sig_cfg.pixel_fmt = out_pixel_fmt;
diff --git a/drivers/staging/imx-drm/parallel-display.c
b/drivers/staging/imx-drm/parallel-display.c
index bb71d6d..65d0c18 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -74,7 +74,35 @@ static int imx_pd_connector_get_modes(struct drm_connector
*connector)
if (np) {
struct drm_display_mode *mode = drm_mode_create(connector->dev);
+ struct device_node *timings_np;
+ struct device_node *mode_np;
+ u32 val;
+
of_get_drm_display_mode(np, &imxpd->mode, 0);
+
+ /* Such flags are not availables in the DRM modes header,
+* and we don't want to export them to userspace.
+*/
+ timings_np = of_get_child_by_name(np, "display-timings");
+ if (timings_np) {
+ /* get the display mode node */
+ mode_np = of_parse_phandle(timings_np,
+ "native-mode", 0);
+ if (!mode_np)
+ mode_np = of_get_next_child(timings_np, NULL);
+
+ /* set de-active to 1 if not set */
+ of_property_read_u32(mode_np, "de-active", &val);
+ if (!!val)
+ imxpd->mode.private_flags |=
+ IMXDRM_MODE_FLAG_DE_HIGH;
+
+ /* set pixelclk-active to 1 if not set */
+ of_property_read_u32(mode_np, "pixelclk-active", &val);
+ if (!!val)
+ imxpd->mode.private_flags |=
+ IMXDRM_MODE_FLAG_PIXDATA_POSEDGE;
+ }
drm_mode_copy(mode, &imxpd->mode);
mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
drm_mode_probed_add(connector, mode);
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv5][ 6/8] ARM: dts: mbimx51sd: Add display support.
The CMO-QVGA, DVI-SVGA and DVI-VGA are added.
Cc: Shawn Guo
Cc: Sascha Hauer
Cc: linux-arm-ker...@lists.infradead.org
Cc: Eric Bénard
Signed-off-by: Denis Carikli
---
ChangeLog v3->v5:
- Updated to new GPIO defines.
- Updated to new licenses checkpatch requirements.
- one whitespace cleanup.
ChangeLog v2->v3:
- Splitted out from the patch that added support for the cpuimx51/mbimxsd51
boards.
- This patch now only adds display support.
- Added some interested people in the Cc list, and removed some people that
might be annoyed by the receiving of that patch which is unrelated to their
subsystem.
- rebased and reworked the dts displays addition.
- Also rebased and reworked the fsl,pins usage.
---
.../imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts | 55
.../imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts | 42 +++
.../imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts | 42 +++
.../boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts | 13 +
4 files changed, 152 insertions(+)
create mode 100644
arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
create mode 100644
arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts
create mode 100644
arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts
diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
new file mode 100644
index 000..f37d65b
--- /dev/null
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2013 Eukréa Electromatique
+ *
+ * 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.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include "imx51-eukrea-mbimxsd51-baseboard.dts"
+
+/ {
+ model = "Eukrea MBIMXSD51 with the CMO-QVGA Display";
+ compatible = "eukrea,mbimxsd51-baseboard-cmo-qvga",
"eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51";
+
+ reg_lcd_3v3: lcd-en {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_reg_lcd_3v3>;
+ regulator-name = "lcd-3v3";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ gpio = <&gpio3 13 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+};
+
+&display {
+ display-supply = <®_lcd_3v3>;
+ status = "okay";
+ display-timings {
+ model = "CMO-QVGA";
+ bits-per-pixel = <16>;
+ cmoqvga {
+ native-mode;
+ clock-frequency = <650>;
+ hactive = <320>;
+ vactive = <240>;
+ hfront-porch = <20>;
+ hback-porch = <38>;
+ vfront-porch = <4>;
+ vback-porch = <15>;
+ hsync-len = <30>;
+ vsync-len = <3>;
+ hsync-active = <0>;
+ vsync-active = <0>;
+ de-active = <0>;
+ pixelclk-active = <1>;
+ };
+ };
+};
diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts
b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts
new file mode 100644
index 000..07e80e8
--- /dev/null
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2013 Eukréa Electromatique
+ *
+ * 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.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include "imx51-eukrea-mbimxsd51-baseboard.dts"
+
+/ {
+ model = "Eukrea MBIMXSD51 with the DVI-SVGA Display";
+ compatible = "eukrea,mbimxsd51-baseboard-dvi-svga",
"eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51";
+};
+
+&display {
+ status = "okay";
+ display-timings {
+ model = "DVI-SVGA";
+ bits-per-pixel = <16>;
+ svga {
+ clock-frequency = <38251000>;
+
[PATCHv5][ 5/8] staging: imx-drm: parallel display: add regulator support.
Cc: Dan Carpenter
Cc: Rob Herring
Cc: Pawel Moll
Cc: Mark Rutland
Cc: Stephen Warren
Cc: Ian Campbell
Cc: devicet...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: driverdev-devel@linuxdriverproject.org
Cc: David Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: Sascha Hauer
Cc: Shawn Guo
Cc: linux-arm-ker...@lists.infradead.org
Cc: Eric Bénard
Signed-off-by: Denis Carikli
---
ChangeLog v3->v5:
- Code clenaup.
ChangeLog v2->v3:
- Added some interested people in the Cc list.
- the lcd-supply is now called display-supply (not all display are LCD).
- The code and documentation was updated accordingly.
- regulator_is_enabled now guard the regulator enables/disables because
regulator_disable does not check the regulator state.
---
.../bindings/staging/imx-drm/fsl-imx-drm.txt |1 +
drivers/staging/imx-drm/parallel-display.c | 22
2 files changed, 23 insertions(+)
diff --git a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
index 2d24425..4dd7ce5 100644
--- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
+++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
@@ -28,6 +28,7 @@ Required properties:
- compatible: Should be "fsl,imx-parallel-display"
- crtc: the crtc this display is connected to, see below
Optional properties:
+- display-supply : phandle to the regulator device tree node if needed.
- interface_pix_fmt: How this display is connected to the
crtc. Currently supported types: "rgb24", "rgb565", "bgr666", "rgb666"
- edid: verbatim EDID data block describing attached display.
diff --git a/drivers/staging/imx-drm/parallel-display.c
b/drivers/staging/imx-drm/parallel-display.c
index 65d0c18..61c0aeb 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include "imx-drm.h"
@@ -35,6 +36,7 @@ struct imx_parallel_display {
struct drm_encoder encoder;
struct imx_drm_encoder *imx_drm_encoder;
struct device *dev;
+ struct regulator *disp_reg;
void *edid;
int edid_len;
u32 interface_pix_fmt;
@@ -141,6 +143,13 @@ static void imx_pd_encoder_prepare(struct drm_encoder
*encoder)
{
struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
+ if (!IS_ERR(imxpd->disp_reg) &&
+ !regulator_is_enabled(imxpd->disp_reg)) {
+ if (regulator_enable(imxpd->disp_reg))
+ dev_err(imxpd->dev,
+ "Failed to enable regulator.\n");
+ }
+
imx_drm_crtc_panel_format(encoder->crtc, DRM_MODE_ENCODER_NONE,
imxpd->interface_pix_fmt);
}
@@ -157,6 +166,12 @@ static void imx_pd_encoder_mode_set(struct drm_encoder
*encoder,
static void imx_pd_encoder_disable(struct drm_encoder *encoder)
{
+ struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
+
+ if (!IS_ERR(imxpd->disp_reg) && regulator_is_enabled(imxpd->disp_reg)) {
+ if (regulator_disable(imxpd->disp_reg))
+ dev_err(imxpd->dev, "Failed to disable regulator.\n");
+ }
}
static void imx_pd_encoder_destroy(struct drm_encoder *encoder)
@@ -260,6 +275,13 @@ static int imx_pd_probe(struct platform_device *pdev)
if (ret)
return ret;
+ imxpd->disp_reg = devm_regulator_get(&pdev->dev, "display");
+ if (PTR_ERR(imxpd->disp_reg) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ if (IS_ERR(imxpd->disp_reg))
+ dev_dbg(&pdev->dev, "Operating without display regulator.\n");
+
ret = imx_drm_encoder_add_possible_crtcs(imxpd->imx_drm_encoder, np);
platform_set_drvdata(pdev, imxpd);
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv5][ 8/8] ARM: imx_v6_v7_defconfig: Enable backlight gpio support.
The eukrea mbimxsd51 has a gpio backlight for its LCD display, so we turn that driver on. Cc: Sascha Hauer Cc: linux-arm-ker...@lists.infradead.org Cc: Fabio Estevam Cc: Shawn Guo Cc: Eric Bénard Signed-off-by: Denis Carikli --- ChangeLog v5: - New patch in this serie. --- arch/arm/configs/imx_v6_v7_defconfig |1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig index d479049..59f3cf0 100644 --- a/arch/arm/configs/imx_v6_v7_defconfig +++ b/arch/arm/configs/imx_v6_v7_defconfig @@ -183,6 +183,7 @@ CONFIG_LCD_L4F00242T03=y CONFIG_LCD_PLATFORM=y CONFIG_BACKLIGHT_CLASS_DEVICE=y CONFIG_BACKLIGHT_PWM=y +CONFIG_BACKLIGHT_GPIO=y CONFIG_FRAMEBUFFER_CONSOLE=y CONFIG_LOGO=y CONFIG_SOUND=y -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv5][ 7/8] ARM: dts: mbimx51sd: Add CMO-QVGA backlight support.
Cc: Shawn Guo
Cc: Sascha Hauer
Cc: linux-arm-ker...@lists.infradead.org
Cc: Eric Bénard
Signed-off-by: Denis Carikli
---
ChangeLog v3->v5:
- Updated to the new GPIO defines.
ChangeLog v2->v3:
- Splitted out from the patch that added support for the cpuimx51/mbimxsd51
boards.
- This patch now only adds backlight support.
- Added some interested people in the Cc list, and removed some people that
might be annoyed by the receiving of that patch which is unrelated to their
subsystem.
---
.../imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts |8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
index f37d65b..f8f5abe 100644
--- a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
@@ -17,6 +17,14 @@
model = "Eukrea MBIMXSD51 with the CMO-QVGA Display";
compatible = "eukrea,mbimxsd51-baseboard-cmo-qvga",
"eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51";
+ backlight {
+ compatible = "gpio-backlight";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_backlight_1>;
+ gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>;
+ default-brightness-level = <1>;
+ };
+
reg_lcd_3v3: lcd-en {
compatible = "regulator-fixed";
pinctrl-names = "default";
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 9/9] staging: android: binder: Add binder compat layer
Hi all,
Thanks for your feedback! Sadly enough, being in a different time-zone,
is not useful.
Sorry for the confusion related to why is this patch needed or not. I
should highlight a bit more what is the patch enabling and what would be
the different alternatives, at least from my perspective.
*64bit kernel/ 32bit userspace*
This patch series adds support for 32bit userspace running on 64bit
kernels. Thus by applying this patch to your kernel you will be able to
use any existing 32bit Android userspace on your 64bit platform, running
a 64bit kernel. That is pure 32bit userspace with no 64bit support!
This means *no modifications to the 32bit userspace*. Therefore any
applications or userspace side drivers, that uses the binder interface
at a native level will not have to be modified. These kind of
applications are not "good citizens" - the native binder API is not
exposed in the Android NDK. However I do not know how many applications
do this and if breaking the compatibility is a concernt for 32bit
userspace running on 64bit kernels.
Below you will find more information on one of the alternative
solutions, the userspace compat.
*32bit kernel/ 32bit userspace*
*64bit kernel/ 64bit userspace*
My last series of binder patches, accepted into 3.12 revision of the
Linux kernel, audits the binder interface for 64bit. A kernel with these
changes applied can be used with a pure 64bit userspace (this does not
include support for 32bit applications coexisting with 64bit ones). The
userspace side needs trivial changes to libbinder.so and servicemanager,
that I will upstream ASAP to AOSP, and that work for 32/32 systems and
64/64 systems without modifying the existing 32bit interface ABI (most
of the changes are related to uint32_t != void *).
Author: Serban Constantinescu
Date: Thu Jul 4 10:54:48 2013 +0100
staging: android: binder: fix binder interface for 64bit compat layer
*64bit kernel/ 64bit coexisting with 32bit userspace
Please note that *none of the above solutions fix this yet*. To
understand why this is a special case please take a look at
how the binder driver works, seen from a high level perspective:
ServiceManager
App1 <-> App2
Thus, for two apps to exchange information between each other they will
have to communicate with the userspace governor, ServiceManager. All the
interaction between App1, App2 and ServiceManager is done through a
combination of libbinder.so (Userspace HAL) and the binder driver. Note
that there can only been one ServiceManager process, that is set during
the userspace boot process.
Now lets consider the case that Arve described earlier 32bit
Applications coexisting with 64bit ones. In this case all the commands
and interface used will have to be the same, the ones used by the 64bit
ServiceManager. For this the kernel entry point for 32bit compat will
have to be changed to:
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -3893,9 +3893,13 @@ static const struct file_operations binder_fops = {
.owner = THIS_MODULE,
.poll = binder_poll,
.unlocked_ioctl = binder_ioctl,
-#ifdef CONFIG_COMPAT
+#if defined(CONFIG_COMPAT) && !defined(COEXIST_32BIT_64BIT)
.compat_ioctl = compat_binder_ioctl,
#endif
+
+#if defined(COEXIST_32BIT_64BIT)
+ .compat_ioctl = binder_ioctl,
+#endif
.mmap = binder_mmap,
.open = binder_open,
.flush = binder_flush,
thus from the perspective of a kernel that works with a 64bit userspace
where 64bit applications coexist with 32bit applications the handling
will be the same for both 32bit and 64bit processes. This will also
involve modifying libbinder.so to use 64bit structures like:
--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
+#if defined(COEXIST_32BIT_64BIT) && !defined(__LP64__)
+/*
+ * For running 32-bit processes on a 64-bit system, make the interaction with
the
+ * binder driver the same from this, when built for 32-bit, as for a 64-bit
process.
+ */
+struct coexist_binder_write_read {
+uint64_twrite_size; /* __LP64__ size_t: bytes to write */
+uint64_twrite_consumed; /* __LP64__ size_t: bytes consumed by driver */
+uint64_twrite_buffer; /* __LP64__ unsigned long */
+uint64_tread_size; /* __LP64__ size_t: bytes to read */
+uint64_tread_consumed; /* __LP64__ size_t: bytes consumed by driver */
+uint64_tread_buffer;/* __LP64__ unsigned long */
+};
"Good citizen" Android applications will go through these changes (since
they should only use the binder Java API), and so shouldn't encounter
any problem. Any 32bit applications that use the binder interface at a
native level will not work with this model (they should not do so!)·
This is exactly what the kernel compat "guarantees" *only* for 64/32bit
systems.
The cleaner solution from the binder kernel perspective is to hoo
Re: [PATCH v1 1/9] staging: android: binder: Move some of the logic into subfunction
On 05/12/13 08:00, Dan Carpenter wrote:
On Wed, Dec 04, 2013 at 06:09:33PM +, Serban Constantinescu wrote:
+static void bc_dead_binder_done(struct binder_proc *proc,
+ struct binder_thread *thread, void __user *cookie)
+{
+ struct binder_work *w;
+ struct binder_ref_death *death = NULL;
+
+ list_for_each_entry(w, &proc->delivered_death, entry) {
+ struct binder_ref_death *tmp_death = container_of(w, struct
binder_ref_death, work);
Put a blank line after the variable declaration block. Also break it up
into two lines instead of having the lines be a million characters long.
I will do that, I have followed the same conventions used by the code
allready there.
list_for_each_entry(w, &proc->delivered_death, entry) {
struct binder_ref_death *tmp_death;
tmp_death = container_of(w, struct binder_ref_death, work);
+ if (tmp_death->cookie == cookie) {
+ death = tmp_death;
+ return;
Should be a break here. This function wasn't tested.
There should be a break there! I have tested the driver with 32bit
Android tests, none of which seem to hit this.
Thanks for your review,
Serban C
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 1/9] staging: android: binder: Move some of the logic into subfunction
On 05/12/13 08:18, Dan Carpenter wrote:
On Wed, Dec 04, 2013 at 06:09:33PM +, Serban Constantinescu wrote:
+static void bc_increfs_done(struct binder_proc *proc,
+ struct binder_thread *thread, uint32_t cmd,
+ void __user *node_ptr, void __user *cookie)
+{
+ struct binder_node *node;
+
+ node = binder_get_node(proc, node_ptr);
+ if (node == NULL) {
+ binder_user_error("%d:%d %s u%p no match\n",
+ proc->pid, thread->pid,
+ cmd == BC_INCREFS_DONE ?
+ "BC_INCREFS_DONE" :
+ "BC_ACQUIRE_DONE",
+ node_ptr);
+ return;
+ }
+ if (cookie != node->cookie) {
+ binder_user_error("%d:%d %s u%p node %d cookie mismatch %p !=
%p\n",
+ proc->pid, thread->pid,
+ cmd == BC_INCREFS_DONE ?
+ "BC_INCREFS_DONE" : "BC_ACQUIRE_DONE",
+ node_ptr, node->debug_id,
+ cookie, node->cookie);
+ return;
+ }
+ if (cmd == BC_ACQUIRE_DONE) {
+ if (node->pending_strong_ref == 0) {
+ binder_user_error("%d:%d BC_ACQUIRE_DONE node %d has no
pending acquire request\n",
+ proc->pid, thread->pid,
+ node->debug_id);
+ return;
+ }
+ node->pending_strong_ref = 0;
+ } else {
+ if (node->pending_weak_ref == 0) {
+ binder_user_error("%d:%d BC_INCREFS_DONE node %d has no
pending increfs request\n",
+ proc->pid, thread->pid,
+ node->debug_id);
+ return;
+ }
+ node->pending_weak_ref = 0;
+ }
+ binder_dec_node(node, cmd == BC_ACQUIRE_DONE, 0);
+ binder_debug(BINDER_DEBUG_USER_REFS,
+"%d:%d %s node %d ls %d lw %d\n",
+proc->pid, thread->pid,
+cmd == BC_INCREFS_DONE ?
+"BC_INCREFS_DONE" :
+"BC_ACQUIRE_DONE",
+node->debug_id, node->local_strong_refs,
+node->local_weak_refs);
+ return;
+}
This function is 52 lines long but at least 32 of those lines are
debug code.
Just extra of lines of code means you have increased the number of bugs
150%. But what I know is that from a static analysis perspective, debug
code has more defects per line then regular code it's worse than that.
Plus all the extra noise obscures the actual logic and makes the real
code annoying to look at so it's worse still.
If you want to move this stuff out of staging, then remove all the extra
crap so it doesn't look like barf.
This patch moves code around so that some of it can be shared with the
compat layer. I agree that due to the abundance of debug code it is, at
times, hard to have a clear idea of what is actually happening and adds
extra obscurities to the logic.
Thanks,
Serban C
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 2/9] staging: android: binder: Add binder_copy_to_user()
On 04/12/13 23:17, Greg KH wrote:
On Wed, Dec 04, 2013 at 06:09:34PM +, Serban Constantinescu wrote:
This patch adds binder_copy_to_user() to be used for copying binder
commands to user address space. This way we can abstract away the
copy_to_user() calls and add separate handling for the compat layer.
Signed-off-by: Serban Constantinescu
---
drivers/staging/android/binder.c | 39 --
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index 233889c..6fbb340 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -2117,6 +2117,18 @@ static int binder_has_thread_work(struct binder_thread
*thread)
(thread->looper & BINDER_LOOPER_STATE_NEED_RETURN);
}
+static int binder_copy_to_user(uint32_t cmd, void *parcel,
+ void __user **ptr, size_t size)
+{
+ if (put_user(cmd, (uint32_t __user *)*ptr))
+ return -EFAULT;
+ *ptr += sizeof(uint32_t);
+ if (copy_to_user(*ptr, parcel, size))
+ return -EFAULT;
+ *ptr += size;
+ return 0;
+}
I know what you are trying to do here, but ick, why not just use the
structure involved in the copying out here? Or just copy the thing out
in one "chunk", not two different calls, which should make this go
faster, right?
Ick... agree. I do this split here for the compat handling added in the
next patches, where the cmd will have to be converted to a 32bit compat
cmd and the size, parcel ,passed here will change to a 32bit compat
size, parcel.
This patch makes more sense when looking at the following snippet:
**
+static int binder_copy_to_user(uint32_t cmd, void *parcel,
+ void __user **ptr, size_t size)
+{
+ if (!is_compat_task()) {
+ if (put_user(cmd, (uint32_t __user *)*ptr))
+ return -EFAULT;
+ *ptr += sizeof(uint32_t);
+ if (copy_to_user(*ptr, parcel, size))
+ return -EFAULT;
+ *ptr += size;
+ return 0;
+ }
+ switch (cmd) {
+ case BR_INCREFS:
+ case BR_ACQUIRE:
+ case BR_RELEASE:
+ case BR_DECREFS: {
+ struct binder_ptr_cookie *fp;
+ struct compat_binder_ptr_cookie tmp;
+
+ cmd = compat_change_size(cmd, sizeof(tmp));
Passing the cmd, and the structure to be copied to binder_copy_to_user()
allows me to add this extra handling here. Where, first, cmd is changed
to a compat cmd, and then copied to userspace - i.e:
I change
cmd = BR_INCREFS = _IOR('r', 7, struct binder_ptr_cookie)
to
cmd = COMPAT_BR_INCREFS = _IOR('r', 7, struct compat_binder_ptr_cookie)
where
struct binder_ptr_cookie {
void* ptr;
void* cookie;
};
and
struct compat_binder_ptr_cookie {
compat_uptr_t ptr;
compat_uptr_t cookie;
};
Thus BR_INCREFS will be different between 32bit userspace and 64bit kernel.
+ BUG_ON((cmd != COMPAT_BR_INCREFS) &&
+ (cmd != COMPAT_BR_ACQUIRE) &&
+ (cmd != COMPAT_BR_RELEASE) &&
+ (cmd != COMPAT_BR_DECREFS));
+
+ fp = (struct binder_ptr_cookie *) parcel;
+ tmp.ptr = ptr_to_compat(fp->ptr);
+ tmp.cookie = ptr_to_compat(fp->cookie);
+ if (put_user(cmd, (uint32_t __user *)*ptr))
+ return -EFAULT;
+ *ptr += sizeof(uint32_t);
+ if (copy_to_user(*ptr, &tmp, sizeof(tmp)))
+ return -EFAULT;
+ *ptr += sizeof(tmp);
Also, since the size of the parcel will differ when copied to a compat
task (32bit userspace) I increment the buffer ptr here, rather then
doing this in binder_thread_read().
This way we can safely move to the next cmd, by incrementing the buffer
ptr accordingly whether 32 or 64bit structures are needed.
**
+
static int binder_thread_read(struct binder_proc *proc,
struct binder_thread *thread,
void __user *buffer, size_t size,
@@ -2263,15 +2275,12 @@ retry:
node->has_weak_ref = 0;
}
if (cmd != BR_NOOP) {
- if (put_user(cmd, (uint32_t __user *)ptr))
- return -EFAULT;
- ptr += sizeof(uint32_t);
- if (put_user(node->ptr, (void * __user *)ptr))
- return -EFAULT;
- ptr += sizeof(void *);
- if (put_user(node->cookie, (void * __user
*)ptr))
+ struct binder_ptr_cookie tmp;
+
+ tmp.ptr = node->ptr;
+
Re: [PATCH v1 3/9] staging: android: binder: Add cmd == CMD_NAME handling
On 05/12/13 08:40, Dan Carpenter wrote:
On Wed, Dec 04, 2013 at 06:09:35PM +, Serban Constantinescu wrote:
This patch modifies the functions that need to be passed the explicit
command to use a boolean flag. This way we can reuse the code for 64bit
compat commands.
I don't understand this at all. cmd seems like it should be 32 bits
on both arches.
Command is 32bit for both arches but does not have the same value. Here
is what happens, this patch make more sense when looking at the compat
layer.
The binder commands differ between 32bit userspace and 64bit kernels. E.g:
BR_INCREFS = _IOR('r', 7, struct binder_ptr_cookie)
COMPAT_BR_INCREFS = _IOR('r', 7, struct compat_binder_ptr_cookie)
struct compat_binder_ptr_cookie {
compat_uptr_t ptr;
compat_uptr_t cookie;
};
struct binder_ptr_cookie {
void *ptr;
void *cookie;
};
(the IOR() macro encodes the size of the transaction - struct
binder_ptr_cookie).
This enables me to use the same handler for:
* native 64bit kernel/ 64bit userspace
case BC_INCREFS_DONE:
case BC_ACQUIRE_DONE: {
void __user *node_ptr;
void *cookie;
if (get_user(node_ptr, (void * __user *)ptr))
return -EFAULT;
ptr += sizeof(void *);
if (get_user(cookie, (void * __user *)ptr))
return -EFAULT;
ptr += sizeof(void *);
bc_increfs_done(proc, thread, cmd == BC_ACQUIRE_DONE,
node_ptr, cookie);
break;
}
* compat 64bit kernel/ 32bit userspace.
+ case COMPAT_BC_INCREFS_DONE:
+ case COMPAT_BC_ACQUIRE_DONE: {
+ compat_uptr_t node_ptr;
+ compat_uptr_t cookie;
+
+ if (get_user(node_ptr, (compat_uptr_t __user *)*ptr))
+ return -EFAULT;
+ *ptr += sizeof(compat_uptr_t);
+ if (get_user(cookie, (compat_uptr_t __user *)*ptr))
+ return -EFAULT;
+ *ptr += sizeof(compat_uptr_t);
+ bc_increfs_done(proc, thread, cmd == COMPAT_BC_ACQUIRE_DONE,
+ compat_ptr(node_ptr), compat_ptr(cookie));
+ break;
where the prototype for bc_increfs_done() has changed to:
static void bc_increfs_done(struct binder_proc *proc,
-struct binder_thread *thread, uint32_t cmd,
+struct binder_thread *thread, bool acquire,
Obsucre on its own, I should add more details in the commit message.
Thanks,
Serban C
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1 9/9] staging: android: binder: Add binder compat layer
On Thu, Dec 05, 2013 at 06:31:25PM +, Serban Constantinescu wrote: > Hi all, > > Thanks for your feedback! Sadly enough, being in a different > time-zone, is not useful. > > Sorry for the confusion related to why is this patch needed or not. I > should highlight a bit more what is the patch enabling and what > would be the different alternatives, at least from my perspective. > > *64bit kernel/ 32bit userspace* > > This patch series adds support for 32bit userspace running on 64bit > kernels. Thus by applying this patch to your kernel you will be able > to use any existing 32bit Android userspace on your 64bit platform, > running a 64bit kernel. That is pure 32bit userspace with no 64bit > support! > > This means *no modifications to the 32bit userspace*. Therefore any > applications or userspace side drivers, that uses the binder > interface at a native level will not have to be modified. These kind > of applications are not "good citizens" - the native binder API is > not exposed in the Android NDK. However I do not know how many > applications do this and if breaking the compatibility is a concernt > for 32bit userspace running on 64bit kernels. Um, I thought we were assured that the _only_ user of the kernel binder interface was libbinder. If other programs are touching this interface "directly", you have bigger problems then just a 32/64 bit issue, and that needs to be fixed. In other words, you should be totally safe in modifying libbinder as well as the kernel interface at the same time. Now if you really want to do that or not is another issue, but it should be possible (and in my opinion, the better option...) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 13/31] staging: comedi: amplc_pci230: tidy up irq request
Clean up the irq request in the attach of this driver and remove the
dev_{level} noise.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/amplc_pci230.c | 27 +++
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c
b/drivers/staging/comedi/drivers/amplc_pci230.c
index 58e7a43..48a0f52 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -2633,7 +2633,7 @@ static int pci230_attach_common(struct comedi_device *dev,
struct comedi_subdevice *s;
unsigned long iobase1, iobase2;
/* PCI230's I/O spaces 1 and 2 respectively. */
- int irq_hdl, rc;
+ int rc;
comedi_set_hw_dev(dev, &pci_dev->dev);
@@ -2705,16 +2705,12 @@ static int pci230_attach_common(struct comedi_device
*dev,
outw(devpriv->adcg, dev->iobase + PCI230_ADCG);
outw(devpriv->adccon | PCI230_ADC_FIFO_RESET,
dev->iobase + PCI230_ADCCON);
- /* Register the interrupt handler. */
- irq_hdl = request_irq(pci_dev->irq, pci230_interrupt,
- IRQF_SHARED, "amplc_pci230", dev);
- if (irq_hdl < 0) {
- dev_warn(dev->class_dev,
-"unable to register irq %u, commands will not be
available\n",
-pci_dev->irq);
- } else {
- dev->irq = pci_dev->irq;
- dev_dbg(dev->class_dev, "registered irq %u\n", pci_dev->irq);
+
+ if (pci_dev->irq) {
+ rc = request_irq(pci_dev->irq, pci230_interrupt, IRQF_SHARED,
+dev->board_name, dev);
+ if (rc == 0)
+ dev->irq = pci_dev->irq;
}
rc = comedi_alloc_subdevices(dev, 3);
@@ -2730,14 +2726,14 @@ static int pci230_attach_common(struct comedi_device
*dev,
s->range_table = &pci230_ai_range;
s->insn_read = &pci230_ai_rinsn;
s->len_chanlist = 256; /* but there are restrictions. */
- /* Only register commands if the interrupt handler is installed. */
- if (irq_hdl == 0) {
+ if (dev->irq) {
dev->read_subdev = s;
s->subdev_flags |= SDF_CMD_READ;
s->do_cmd = &pci230_ai_cmd;
s->do_cmdtest = &pci230_ai_cmdtest;
s->cancel = pci230_ai_cancel;
}
+
s = &dev->subdevices[1];
/* analog output subdevice */
if (thisboard->ao_chans > 0) {
@@ -2749,9 +2745,7 @@ static int pci230_attach_common(struct comedi_device *dev,
s->insn_write = &pci230_ao_winsn;
s->insn_read = &pci230_ao_rinsn;
s->len_chanlist = thisboard->ao_chans;
- /* Only register commands if the interrupt handler is
-* installed. */
- if (irq_hdl == 0) {
+ if (dev->irq) {
dev->write_subdev = s;
s->subdev_flags |= SDF_CMD_WRITE;
s->do_cmd = &pci230_ao_cmd;
@@ -2761,6 +2755,7 @@ static int pci230_attach_common(struct comedi_device *dev,
} else {
s->type = COMEDI_SUBD_UNUSED;
}
+
s = &dev->subdevices[2];
/* digital i/o subdevice */
if (thisboard->have_dio) {
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 02/31] staging: comedi: me4000: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/me4000.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/me4000.c
b/drivers/staging/comedi/drivers/me4000.c
index 081a310..e4afca4 100644
--- a/drivers/staging/comedi/drivers/me4000.c
+++ b/drivers/staging/comedi/drivers/me4000.c
@@ -1105,7 +1105,7 @@ static irqreturn_t me4000_ai_isr(int irq, void *dev_id)
{
unsigned int tmp;
struct comedi_device *dev = dev_id;
- struct comedi_subdevice *s = &dev->subdevices[0];
+ struct comedi_subdevice *s = dev->read_subdev;
int i;
int c = 0;
unsigned int lval;
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 08/31] staging: comedi: adl_pci9111: fix incorrect irq passed to request_irq()
The dev->irq passed to request_irq() will always be 0 when the auto_attach
function is called. The pcidev->irq should be used instead to get the correct
irq number.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adl_pci9111.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9111.c
b/drivers/staging/comedi/drivers/adl_pci9111.c
index 03b38ea..8dd3973 100644
--- a/drivers/staging/comedi/drivers/adl_pci9111.c
+++ b/drivers/staging/comedi/drivers/adl_pci9111.c
@@ -859,7 +859,7 @@ static int pci9111_auto_attach(struct comedi_device *dev,
pci9111_reset(dev);
if (pcidev->irq > 0) {
- ret = request_irq(dev->irq, pci9111_interrupt,
+ ret = request_irq(pcidev->irq, pci9111_interrupt,
IRQF_SHARED, dev->board_name, dev);
if (ret)
return ret;
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 01/31] staging: comedi: ni_at_2150: tidy up irq/dma request
This driver needs both an irq and dma in order to support async
commands. If the irq and dma are not available the driver will
still function for single analog input reads.
Tidy up the code that does the irq and dma requests so that the
driver will still attach if they are not avaliable. The attach
will still fail, with -ENOMEM, if the dma buffer cannot be allocated.
Remove the noise about the irq and dma during the attach.
Only hook up the async commands support if the irq and dma are
available. Remove the then unnecessary sanity check in a2150_ai_cmd().
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/ni_at_a2150.c | 79
1 file changed, 33 insertions(+), 46 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_at_a2150.c
b/drivers/staging/comedi/drivers/ni_at_a2150.c
index cc69dde..0876bc5 100644
--- a/drivers/staging/comedi/drivers/ni_at_a2150.c
+++ b/drivers/staging/comedi/drivers/ni_at_a2150.c
@@ -395,11 +395,6 @@ static int a2150_ai_cmd(struct comedi_device *dev, struct
comedi_subdevice *s)
unsigned int old_config_bits = devpriv->config_bits;
unsigned int trigger_bits;
- if (!dev->irq || !devpriv->dma) {
- comedi_error(dev,
-" irq and dma required, cannot do hardware
conversions");
- return -1;
- }
if (cmd->flags & TRIG_RT) {
comedi_error(dev,
" dma incompatible with hard real-time interrupt
(TRIG_RT), aborting");
@@ -703,46 +698,35 @@ static int a2150_attach(struct comedi_device *dev, struct
comedi_devconfig *it)
if (ret)
return ret;
- /* grab our IRQ */
- if (irq) {
- /* check that irq is supported */
- if (irq < 3 || irq == 8 || irq == 13 || irq > 15) {
- printk(" invalid irq line %u\n", irq);
- return -EINVAL;
- }
- if (request_irq(irq, a2150_interrupt, 0,
- dev->driver->driver_name, dev)) {
- printk("unable to allocate irq %u\n", irq);
- return -EINVAL;
+ dev->board_ptr = a2150_boards + a2150_probe(dev);
+ thisboard = comedi_board(dev);
+ dev->board_name = thisboard->name;
+
+ if ((irq >= 3 && irq <= 7) || (irq >= 9 && irq <= 12) ||
+ irq == 14 || irq == 15) {
+ ret = request_irq(irq, a2150_interrupt, 0,
+ dev->board_name, dev);
+ if (ret == 0) {
+ devpriv->irq_dma_bits |= IRQ_LVL_BITS(irq);
+ dev->irq = irq;
}
- devpriv->irq_dma_bits |= IRQ_LVL_BITS(irq);
- dev->irq = irq;
}
- /* initialize dma */
- if (dma) {
- if (dma == 4 || dma > 7) {
- printk(" invalid dma channel %u\n", dma);
- return -EINVAL;
- }
- if (request_dma(dma, dev->driver->driver_name)) {
- printk(" failed to allocate dma channel %u\n", dma);
- return -EINVAL;
- }
- devpriv->dma = dma;
- devpriv->dma_buffer =
- kmalloc(A2150_DMA_BUFFER_SIZE, GFP_KERNEL | GFP_DMA);
- if (devpriv->dma_buffer == NULL)
- return -ENOMEM;
- disable_dma(dma);
- set_dma_mode(dma, DMA_MODE_READ);
+ if (dev->irq && ((dma >= 0 && dma <= 4) || (dma >= 5 && dma <= 7))) {
+ ret = request_dma(dma, dev->board_name);
+ if (ret == 0) {
+ devpriv->dma = dma;
+ devpriv->dma_buffer = kmalloc(A2150_DMA_BUFFER_SIZE,
+ GFP_KERNEL | GFP_DMA);
+ if (!devpriv->dma_buffer)
+ return -ENOMEM;
- devpriv->irq_dma_bits |= DMA_CHAN_BITS(dma);
- }
+ disable_dma(dma);
+ set_dma_mode(dma, DMA_MODE_READ);
- dev->board_ptr = a2150_boards + a2150_probe(dev);
- thisboard = comedi_board(dev);
- dev->board_name = thisboard->name;
+ devpriv->irq_dma_bits |= DMA_CHAN_BITS(dma);
+ }
+ }
ret = comedi_alloc_subdevices(dev, 1);
if (ret)
@@ -750,17 +734,20 @@ static int a2150_attach(struct comedi_device *dev, struct
comedi_devconfig *it)
/* analog input subdevice */
s = &dev->subdevices[0];
- dev->read_subdev = s;
s->type = COMEDI_SUBD_AI;
- s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_OTHER | SDF_CMD_READ;
+ s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_OTHER;
s->n_chan = 4;
- s->len_chanlist = 4;
[PATCH v2 22/31] staging: comedi: adl_pci9118: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead of accessing the dev->subdevices array directly. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/adl_pci9118.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c b/drivers/staging/comedi/drivers/adl_pci9118.c index 7e66978..a1de963 100644 --- a/drivers/staging/comedi/drivers/adl_pci9118.c +++ b/drivers/staging/comedi/drivers/adl_pci9118.c @@ -1126,7 +1126,7 @@ static irqreturn_t interrupt_pci9118(int irq, void *d) } } - (devpriv->int_ai_func) (dev, &dev->subdevices[0], int_adstat, + (devpriv->int_ai_func) (dev, dev->read_subdev, int_adstat, int_amcc, int_daq); } -- 1.8.4.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 24/31] staging: comedi: amplc_pci224: use dev->write_subdev
Use the dev->write_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/amplc_pci224.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c
b/drivers/staging/comedi/drivers/amplc_pci224.c
index 230a010..f16b472 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1140,7 +1140,7 @@ static irqreturn_t pci224_interrupt(int irq, void *d)
{
struct comedi_device *dev = d;
struct pci224_private *devpriv = dev->private;
- struct comedi_subdevice *s = &dev->subdevices[0];
+ struct comedi_subdevice *s = dev->write_subdev;
struct comedi_cmd *cmd;
unsigned char intstat, valid_intstat;
unsigned char curenab;
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 25/31] staging: comedi: ni_65xx: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/ni_65xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/ni_65xx.c
b/drivers/staging/comedi/drivers/ni_65xx.c
index effa124..6e42001 100644
--- a/drivers/staging/comedi/drivers/ni_65xx.c
+++ b/drivers/staging/comedi/drivers/ni_65xx.c
@@ -427,7 +427,7 @@ static irqreturn_t ni_65xx_interrupt(int irq, void *d)
{
struct comedi_device *dev = d;
struct ni_65xx_private *devpriv = dev->private;
- struct comedi_subdevice *s = &dev->subdevices[2];
+ struct comedi_subdevice *s = dev->read_subdev;
unsigned int status;
status = readb(devpriv->mite->daq_io_addr + Change_Status);
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 09/31] staging: comedi: adl_pci9111: the irq is only needed for async command support
An irq is only needed for async command support, modify the attach of
the subdevices so that the command support is only hooked up if the irq
request was successful. Remove the then unnecessary sanity check in
pci9111_ai_do_cmd().
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adl_pci9111.c | 29 +---
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9111.c
b/drivers/staging/comedi/drivers/adl_pci9111.c
index 8dd3973..de763ec 100644
--- a/drivers/staging/comedi/drivers/adl_pci9111.c
+++ b/drivers/staging/comedi/drivers/adl_pci9111.c
@@ -470,11 +470,6 @@ static int pci9111_ai_do_cmd(struct comedi_device *dev,
struct pci9111_private_data *dev_private = dev->private;
struct comedi_cmd *async_cmd = &s->async->cmd;
- if (!dev->irq) {
- comedi_error(dev,
-"no irq assigned for PCI9111, cannot do hardware
conversion");
- return -1;
- }
/* Set channel scan limit */
/* PCI9111 allows only scanning from channel 0 to channel n */
/* TODO: handle the case of an external multiplexer */
@@ -858,12 +853,11 @@ static int pci9111_auto_attach(struct comedi_device *dev,
pci9111_reset(dev);
- if (pcidev->irq > 0) {
+ if (pcidev->irq) {
ret = request_irq(pcidev->irq, pci9111_interrupt,
IRQF_SHARED, dev->board_name, dev);
- if (ret)
- return ret;
- dev->irq = pcidev->irq;
+ if (ret == 0)
+ dev->irq = pcidev->irq;
}
ret = comedi_alloc_subdevices(dev, 4);
@@ -871,18 +865,21 @@ static int pci9111_auto_attach(struct comedi_device *dev,
return ret;
s = &dev->subdevices[0];
- dev->read_subdev = s;
s->type = COMEDI_SUBD_AI;
- s->subdev_flags = SDF_READABLE | SDF_COMMON | SDF_CMD_READ;
+ s->subdev_flags = SDF_READABLE | SDF_COMMON;
s->n_chan = 16;
s->maxdata = 0x;
- s->len_chanlist = 16;
s->range_table = &pci9111_ai_range;
- s->cancel = pci9111_ai_cancel;
s->insn_read= pci9111_ai_insn_read;
- s->do_cmdtest = pci9111_ai_do_cmd_test;
- s->do_cmd = pci9111_ai_do_cmd;
- s->munge= pci9111_ai_munge;
+ if (dev->irq) {
+ dev->read_subdev = s;
+ s->subdev_flags |= SDF_CMD_READ;
+ s->len_chanlist = s->n_chan;
+ s->do_cmdtest = pci9111_ai_do_cmd_test;
+ s->do_cmd = pci9111_ai_do_cmd;
+ s->cancel = pci9111_ai_cancel;
+ s->munge= pci9111_ai_munge;
+ }
s = &dev->subdevices[1];
s->type = COMEDI_SUBD_AO;
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 20/31] staging: comedi: hwrdv_apci3120: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c
index 3c9eec8..bd05857 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c
@@ -1414,7 +1414,7 @@ static void v_APCI3120_InterruptDma(int irq, void *d)
{
struct comedi_device *dev = d;
struct addi_private *devpriv = dev->private;
- struct comedi_subdevice *s = &dev->subdevices[0];
+ struct comedi_subdevice *s = dev->read_subdev;
unsigned int next_dma_buf, samplesinbuf;
unsigned long low_word, high_word, var;
unsigned int ui_Tmp;
@@ -1568,8 +1568,8 @@ static void v_APCI3120_InterruptDma(int irq, void *d)
static int i_APCI3120_InterruptHandleEos(struct comedi_device *dev)
{
struct addi_private *devpriv = dev->private;
+ struct comedi_subdevice *s = dev->read_subdev;
int n_chan, i;
- struct comedi_subdevice *s = &dev->subdevices[0];
int err = 1;
n_chan = devpriv->ui_AiNbrofChannels;
@@ -1593,11 +1593,11 @@ static void v_APCI3120_Interrupt(int irq, void *d)
{
struct comedi_device *dev = d;
struct addi_private *devpriv = dev->private;
+ struct comedi_subdevice *s = dev->read_subdev;
unsigned short int_daq;
unsigned int int_amcc, ui_Check, i;
unsigned short us_TmpValue;
unsigned char b_DummyRead;
- struct comedi_subdevice *s = &dev->subdevices[0];
ui_Check = 1;
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 30/31] staging: comedi: ni_pcidio: request_irq() before seting up subdevices
Do the request_irq() before setting up the subdevices. Only hook up
the command support of the irq was sucessfully requested.
Note that, because of the IRQF_SHARED flag, nidio_interrupt() _may_
be called before the device is ready and the subdevices are setup.
This condition is handled by the (!dev->attached) sanity check. The
initialzation of the local variable pointers before this test is
still safe since we are just getting addresses and not dereferencing
them.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/ni_pcidio.c | 38 +++---
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_pcidio.c
b/drivers/staging/comedi/drivers/ni_pcidio.c
index 30c46a3..8ab8a87 100644
--- a/drivers/staging/comedi/drivers/ni_pcidio.c
+++ b/drivers/staging/comedi/drivers/ni_pcidio.c
@@ -1011,6 +1011,14 @@ static int nidio_auto_attach(struct comedi_device *dev,
nidio_reset_board(dev);
+ irq = mite_irq(devpriv->mite);
+ if (irq) {
+ ret = request_irq(irq, nidio_interrupt, IRQF_SHARED,
+ dev->board_name, dev);
+ if (ret == 0)
+ dev->irq = irq;
+ }
+
ret = comedi_alloc_subdevices(dev, 1);
if (ret)
return ret;
@@ -1019,31 +1027,23 @@ static int nidio_auto_attach(struct comedi_device *dev,
readb(devpriv->mite->daq_io_addr + Chip_Version));
s = &dev->subdevices[0];
-
- dev->read_subdev = s;
s->type = COMEDI_SUBD_DIO;
- s->subdev_flags =
- SDF_READABLE | SDF_WRITABLE | SDF_LSAMPL | SDF_PACKED |
- SDF_CMD_READ;
+ s->subdev_flags = SDF_READABLE | SDF_WRITABLE | SDF_LSAMPL | SDF_PACKED;
s->n_chan = 32;
s->range_table = &range_digital;
s->maxdata = 1;
s->insn_config = &ni_pcidio_insn_config;
s->insn_bits = &ni_pcidio_insn_bits;
- s->do_cmd = &ni_pcidio_cmd;
- s->do_cmdtest = &ni_pcidio_cmdtest;
- s->cancel = &ni_pcidio_cancel;
- s->len_chanlist = 32; /* XXX */
- s->buf_change = &ni_pcidio_change;
- s->async_dma_dir = DMA_BIDIRECTIONAL;
- s->poll = &ni_pcidio_poll;
-
- irq = mite_irq(devpriv->mite);
- if (irq) {
- ret = request_irq(irq, nidio_interrupt, IRQF_SHARED,
- dev->board_name, dev);
- if (ret == 0)
- dev->irq = irq;
+ if (dev->irq) {
+ dev->read_subdev = s;
+ s->subdev_flags |= SDF_CMD_READ;
+ s->async_dma_dir = DMA_BIDIRECTIONAL;
+ s->len_chanlist = s->n_chan;
+ s->do_cmd = ni_pcidio_cmd;
+ s->do_cmdtest = ni_pcidio_cmdtest;
+ s->cancel = ni_pcidio_cancel;
+ s->poll = ni_pcidio_poll;
+ s->buf_change = ni_pcidio_change;
}
return 0;
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 15/31] staging: comedi: adv_pci1710: only init async command members when needed
The 'len_chanlist' and 'cancel' members of the comedi_subdevice are
only used with async command support. Only initialize them if the irq
was sucessfully requested.
Also, only init the dev->read_subdev if we have the irq.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index a1519c9..69ab2a6 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -1264,21 +1264,21 @@ static int pci1710_auto_attach(struct comedi_device
*dev,
if (this_board->n_aichan) {
s = &dev->subdevices[subdev];
- dev->read_subdev = s;
s->type = COMEDI_SUBD_AI;
s->subdev_flags = SDF_READABLE | SDF_COMMON | SDF_GROUND;
if (this_board->n_aichand)
s->subdev_flags |= SDF_DIFF;
s->n_chan = this_board->n_aichan;
s->maxdata = this_board->ai_maxdata;
- s->len_chanlist = this_board->n_aichan;
s->range_table = this_board->rangelist_ai;
- s->cancel = pci171x_ai_cancel;
s->insn_read = pci171x_insn_read_ai;
if (dev->irq) {
+ dev->read_subdev = s;
s->subdev_flags |= SDF_CMD_READ;
+ s->len_chanlist = s->n_chan;
s->do_cmdtest = pci171x_ai_cmdtest;
s->do_cmd = pci171x_ai_cmd;
+ s->cancel = pci171x_ai_cancel;
}
devpriv->i8254_osc_base = I8254_OSC_BASE_10MHZ;
subdev++;
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 21/31] staging: comedi: hwrdv_apci3200: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/addi-data/hwdrv_apci3200.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3200.c
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3200.c
index dc73d4d..8c85a09 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3200.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3200.c
@@ -2590,8 +2590,8 @@ static int i_APCI3200_CommandAnalogInput(struct
comedi_device *dev,
static int i_APCI3200_InterruptHandleEos(struct comedi_device *dev)
{
struct addi_private *devpriv = dev->private;
+ struct comedi_subdevice *s = dev->read_subdev;
unsigned int ui_StatusRegister = 0;
- struct comedi_subdevice *s = &dev->subdevices[0];
/* BEGIN JK 18.10.2004: APCI-3200 Driver update 0.7.57 -> 0.7.68 */
/* comedi_async *async = s->async; */
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 26/31] staging: comedi: ni_atmio16d: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/ni_atmio16d.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c
b/drivers/staging/comedi/drivers/ni_atmio16d.c
index 834c9e1..3a92fe5 100644
--- a/drivers/staging/comedi/drivers/ni_atmio16d.c
+++ b/drivers/staging/comedi/drivers/ni_atmio16d.c
@@ -229,7 +229,7 @@ static void reset_atmio16d(struct comedi_device *dev)
static irqreturn_t atmio16d_interrupt(int irq, void *d)
{
struct comedi_device *dev = d;
- struct comedi_subdevice *s = &dev->subdevices[0];
+ struct comedi_subdevice *s = dev->read_subdev;
comedi_buf_put(s->async, inw(dev->iobase + AD_FIFO_REG));
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 23/31] staging: comedi: amplc_pc236: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/amplc_pc236.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c
b/drivers/staging/comedi/drivers/amplc_pc236.c
index eaf21ce..31734e1 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -356,7 +356,7 @@ static int pc236_intr_cancel(struct comedi_device *dev,
static irqreturn_t pc236_interrupt(int irq, void *d)
{
struct comedi_device *dev = d;
- struct comedi_subdevice *s = &dev->subdevices[1];
+ struct comedi_subdevice *s = dev->read_subdev;
int handled;
handled = pc236_intr_check(dev);
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 10/31] staging: comedi: dt2814: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/dt2814.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt2814.c
b/drivers/staging/comedi/drivers/dt2814.c
index e52d22e..abad6e4 100644
--- a/drivers/staging/comedi/drivers/dt2814.c
+++ b/drivers/staging/comedi/drivers/dt2814.c
@@ -197,7 +197,7 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
int lo, hi;
struct comedi_device *dev = d;
struct dt2814_private *devpriv = dev->private;
- struct comedi_subdevice *s;
+ struct comedi_subdevice *s = dev->read_subdev;
int data;
if (!dev->attached) {
@@ -205,8 +205,6 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
return IRQ_HANDLED;
}
- s = &dev->subdevices[0];
-
hi = inb(dev->iobase + DT2814_DATA);
lo = inb(dev->iobase + DT2814_DATA);
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 07/31] staging: comedi: das16m1: remove unnecessary 'dev->irq' test
This function can only be called if the irq was sucessfully requested.
The dev->irq will always be valid.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/das16m1.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/staging/comedi/drivers/das16m1.c
b/drivers/staging/comedi/drivers/das16m1.c
index 0081ee4..36c2ba5 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -269,11 +269,6 @@ static int das16m1_cmd_exec(struct comedi_device *dev,
struct comedi_cmd *cmd = &async->cmd;
unsigned int byte, i;
- if (dev->irq == 0) {
- comedi_error(dev, "irq required to execute comedi_cmd");
- return -1;
- }
-
/* disable interrupts and internal pacer */
devpriv->control_state &= ~INTE & ~PACER_MASK;
outb(devpriv->control_state, dev->iobase + DAS16M1_INTR_CONTROL);
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 17/31] staging: comedi: dt3000: don't fail attach if irq is not available
The irq is only needed to support async commands. Don't fail the attach
if it is not available.
Only hook up the command support if the request_irq() was successful.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/dt3000.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt3000.c
b/drivers/staging/comedi/drivers/dt3000.c
index 1410943..50aaa01 100644
--- a/drivers/staging/comedi/drivers/dt3000.c
+++ b/drivers/staging/comedi/drivers/dt3000.c
@@ -702,29 +702,33 @@ static int dt3000_auto_attach(struct comedi_device *dev,
if (!devpriv->io_addr)
return -ENOMEM;
- ret = request_irq(pcidev->irq, dt3k_interrupt, IRQF_SHARED,
- dev->board_name, dev);
- if (ret)
- return ret;
- dev->irq = pcidev->irq;
+ if (pcidev->irq) {
+ ret = request_irq(pcidev->irq, dt3k_interrupt, IRQF_SHARED,
+ dev->board_name, dev);
+ if (ret == 0)
+ dev->irq = pcidev->irq;
+ }
ret = comedi_alloc_subdevices(dev, 4);
if (ret)
return ret;
s = &dev->subdevices[0];
- dev->read_subdev = s;
/* ai subdevice */
s->type = COMEDI_SUBD_AI;
- s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_DIFF | SDF_CMD_READ;
+ s->subdev_flags = SDF_READABLE | SDF_GROUND | SDF_DIFF;
s->n_chan = this_board->adchan;
s->insn_read= dt3k_ai_insn;
s->maxdata = (1 << this_board->adbits) - 1;
- s->len_chanlist = 512;
s->range_table = &range_dt3000_ai; /* XXX */
- s->do_cmd = dt3k_ai_cmd;
- s->do_cmdtest = dt3k_ai_cmdtest;
- s->cancel = dt3k_ai_cancel;
+ if (dev->irq) {
+ dev->read_subdev = s;
+ s->subdev_flags |= SDF_CMD_READ;
+ s->len_chanlist = 512;
+ s->do_cmd = dt3k_ai_cmd;
+ s->do_cmdtest = dt3k_ai_cmdtest;
+ s->cancel = dt3k_ai_cancel;
+ }
s = &dev->subdevices[1];
/* ao subsystem */
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 05/31] staging: comedi: das1800: tidy up irq request
This driver only needs an irq in order to support async commands.
If the irq is not available the driver will still function for
single analog input reads.
Tidy up the code that does the irq requests so that the driver
will still attach if it is not avaliable.
Remove the noise about the irq during the attach.
Only hook up the async commands support if the irq is available.
Remove the then unnecessary sanity check in das1800_ai_do_cmd().
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/das1800.c | 86 ++--
1 file changed, 37 insertions(+), 49 deletions(-)
diff --git a/drivers/staging/comedi/drivers/das1800.c
b/drivers/staging/comedi/drivers/das1800.c
index 1880038..92df02d 100644
--- a/drivers/staging/comedi/drivers/das1800.c
+++ b/drivers/staging/comedi/drivers/das1800.c
@@ -1150,12 +1150,6 @@ static int das1800_ai_do_cmd(struct comedi_device *dev,
struct comedi_async *async = s->async;
const struct comedi_cmd *cmd = &async->cmd;
- if (!dev->irq) {
- comedi_error(dev,
-"no irq assigned for das-1800, cannot do hardware
conversions");
- return -1;
- }
-
/* disable dma on TRIG_WAKE_EOS, or TRIG_RT
* (because dma in handler is unsafe at hard real-time priority) */
if (cmd->flags & (TRIG_WAKE_EOS | TRIG_RT))
@@ -1522,43 +1516,34 @@ static int das1800_attach(struct comedi_device *dev,
devpriv->iobase2 = iobase2;
}
- /* grab our IRQ */
- if (irq) {
- if (request_irq(irq, das1800_interrupt, 0,
- dev->driver->driver_name, dev)) {
- dev_dbg(dev->class_dev, "unable to allocate irq %u\n",
- irq);
- return -EINVAL;
- }
- }
- dev->irq = irq;
+ if (irq == 3 || irq == 5 || irq == 7 || irq == 10 || irq == 11 ||
+ irq == 15) {
+ ret = request_irq(irq, das1800_interrupt, 0,
+ dev->board_name, dev);
+ if (ret == 0) {
+ dev->irq = irq;
- /* set bits that tell card which irq to use */
- switch (irq) {
- case 0:
- break;
- case 3:
- devpriv->irq_dma_bits |= 0x8;
- break;
- case 5:
- devpriv->irq_dma_bits |= 0x10;
- break;
- case 7:
- devpriv->irq_dma_bits |= 0x18;
- break;
- case 10:
- devpriv->irq_dma_bits |= 0x28;
- break;
- case 11:
- devpriv->irq_dma_bits |= 0x30;
- break;
- case 15:
- devpriv->irq_dma_bits |= 0x38;
- break;
- default:
- dev_err(dev->class_dev, "irq out of range\n");
- return -EINVAL;
- break;
+ switch (irq) {
+ case 3:
+ devpriv->irq_dma_bits |= 0x8;
+ break;
+ case 5:
+ devpriv->irq_dma_bits |= 0x10;
+ break;
+ case 7:
+ devpriv->irq_dma_bits |= 0x18;
+ break;
+ case 10:
+ devpriv->irq_dma_bits |= 0x28;
+ break;
+ case 11:
+ devpriv->irq_dma_bits |= 0x30;
+ break;
+ case 15:
+ devpriv->irq_dma_bits |= 0x38;
+ break;
+ }
+ }
}
ret = das1800_init_dma(dev, dma0, dma1);
@@ -1578,20 +1563,23 @@ static int das1800_attach(struct comedi_device *dev,
/* analog input subdevice */
s = &dev->subdevices[0];
- dev->read_subdev = s;
s->type = COMEDI_SUBD_AI;
- s->subdev_flags = SDF_READABLE | SDF_DIFF | SDF_GROUND | SDF_CMD_READ;
+ s->subdev_flags = SDF_READABLE | SDF_DIFF | SDF_GROUND;
if (thisboard->common)
s->subdev_flags |= SDF_COMMON;
s->n_chan = thisboard->qram_len;
- s->len_chanlist = thisboard->qram_len;
s->maxdata = (1 << thisboard->resolution) - 1;
s->range_table = thisboard->range_ai;
- s->do_cmd = das1800_ai_do_cmd;
- s->do_cmdtest = das1800_ai_do_cmdtest;
s->insn_read = das1800_ai_rinsn;
- s->poll = das1800_ai_poll;
- s->cancel = das1800_cancel;
+ if (dev->irq) {
+ dev->read_subdev = s;
+ s->subdev_flags |= SDF_CMD_READ;
+ s->len_chanlist = s->n_chan;
+ s->do_cmd = das1800_ai_do_cmd;
+ s->d
[PATCH v2 18/31] staging: comedi: dt3000: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/dt3000.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt3000.c
b/drivers/staging/comedi/drivers/dt3000.c
index 50aaa01..f52a447 100644
--- a/drivers/staging/comedi/drivers/dt3000.c
+++ b/drivers/staging/comedi/drivers/dt3000.c
@@ -352,13 +352,12 @@ static irqreturn_t dt3k_interrupt(int irq, void *d)
{
struct comedi_device *dev = d;
struct dt3k_private *devpriv = dev->private;
- struct comedi_subdevice *s;
+ struct comedi_subdevice *s = dev->read_subdev;
unsigned int status;
if (!dev->attached)
return IRQ_NONE;
- s = &dev->subdevices[0];
status = readw(devpriv->io_addr + DPR_Intr_Flag);
if (status & DT3000_ADFULL) {
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 27/31] staging: comedi: rtd520: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/rtd520.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/rtd520.c
b/drivers/staging/comedi/drivers/rtd520.c
index c6a664f..0f026af 100644
--- a/drivers/staging/comedi/drivers/rtd520.c
+++ b/drivers/staging/comedi/drivers/rtd520.c
@@ -689,7 +689,7 @@ static int ai_read_dregs(struct comedi_device *dev, struct
comedi_subdevice *s)
static irqreturn_t rtd_interrupt(int irq, void *d)
{
struct comedi_device *dev = d;
- struct comedi_subdevice *s = &dev->subdevices[0];
+ struct comedi_subdevice *s = dev->read_subdev;
struct rtd_private *devpriv = dev->private;
u32 overrun;
u16 status;
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 12/31] staging: comedi: dt282x: use dev->write_subdev
Use the dev->write_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/dt282x.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt282x.c
b/drivers/staging/comedi/drivers/dt282x.c
index 5815fc2..ae714a5 100644
--- a/drivers/staging/comedi/drivers/dt282x.c
+++ b/drivers/staging/comedi/drivers/dt282x.c
@@ -306,10 +306,10 @@ static void dt282x_munge(struct comedi_device *dev,
unsigned short *buf,
static void dt282x_ao_dma_interrupt(struct comedi_device *dev)
{
struct dt282x_private *devpriv = dev->private;
+ struct comedi_subdevice *s = dev->write_subdev;
void *ptr;
int size;
int i;
- struct comedi_subdevice *s = &dev->subdevices[1];
outw(devpriv->supcsr | DT2821_CLRDMADNE, dev->iobase + DT2821_SUPCSR);
@@ -449,7 +449,7 @@ static irqreturn_t dt282x_interrupt(int irq, void *d)
struct comedi_device *dev = d;
struct dt282x_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
- struct comedi_subdevice *s_ao;
+ struct comedi_subdevice *s_ao = dev->write_subdev;
unsigned int supcsr, adcsr, dacsr;
int handled = 0;
@@ -458,7 +458,6 @@ static irqreturn_t dt282x_interrupt(int irq, void *d)
return IRQ_HANDLED;
}
- s_ao = &dev->subdevices[1];
adcsr = inw(dev->iobase + DT2821_ADCSR);
dacsr = inw(dev->iobase + DT2821_DACSR);
supcsr = inw(dev->iobase + DT2821_SUPCSR);
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 04/31] staging: comedi: me4000: remove unnecessary check in the irq handler
The sanity check of the irq is not necessary. If it _is_ wrong we have
bigger problems in the kernel...
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/me4000.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/staging/comedi/drivers/me4000.c
b/drivers/staging/comedi/drivers/me4000.c
index cb1032f..7276002 100644
--- a/drivers/staging/comedi/drivers/me4000.c
+++ b/drivers/staging/comedi/drivers/me4000.c
@@ -1116,12 +1116,6 @@ static irqreturn_t me4000_ai_isr(int irq, void *dev_id)
/* Reset all events */
s->async->events = 0;
- /* Check if irq number is right */
- if (irq != dev->irq) {
- dev_err(dev->class_dev, "Incorrect interrupt num: %d\n", irq);
- return IRQ_HANDLED;
- }
-
if (inl(dev->iobase + ME4000_IRQ_STATUS_REG) &
ME4000_IRQ_STATUS_BIT_AI_HF) {
/* Read status register to find out what happened */
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 11/31] staging: comedi: dt282x: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/dt282x.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt282x.c
b/drivers/staging/comedi/drivers/dt282x.c
index df37d18..5815fc2 100644
--- a/drivers/staging/comedi/drivers/dt282x.c
+++ b/drivers/staging/comedi/drivers/dt282x.c
@@ -339,11 +339,11 @@ static void dt282x_ao_dma_interrupt(struct comedi_device
*dev)
static void dt282x_ai_dma_interrupt(struct comedi_device *dev)
{
struct dt282x_private *devpriv = dev->private;
+ struct comedi_subdevice *s = dev->read_subdev;
void *ptr;
int size;
int i;
int ret;
- struct comedi_subdevice *s = &dev->subdevices[0];
outw(devpriv->supcsr | DT2821_CLRDMADNE, dev->iobase + DT2821_SUPCSR);
@@ -448,7 +448,7 @@ static irqreturn_t dt282x_interrupt(int irq, void *d)
{
struct comedi_device *dev = d;
struct dt282x_private *devpriv = dev->private;
- struct comedi_subdevice *s;
+ struct comedi_subdevice *s = dev->read_subdev;
struct comedi_subdevice *s_ao;
unsigned int supcsr, adcsr, dacsr;
int handled = 0;
@@ -458,7 +458,6 @@ static irqreturn_t dt282x_interrupt(int irq, void *d)
return IRQ_HANDLED;
}
- s = &dev->subdevices[0];
s_ao = &dev->subdevices[1];
adcsr = inw(dev->iobase + DT2821_ADCSR);
dacsr = inw(dev->iobase + DT2821_DACSR);
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 19/31] staging: comedi: s626: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/s626.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/s626.c
b/drivers/staging/comedi/drivers/s626.c
index 5835838..ae13b0e 100644
--- a/drivers/staging/comedi/drivers/s626.c
+++ b/drivers/staging/comedi/drivers/s626.c
@@ -1614,12 +1614,13 @@ static irqreturn_t s626_irq_handler(int irq, void *d)
static void s626_reset_adc(struct comedi_device *dev, uint8_t *ppl)
{
struct s626_private *devpriv = dev->private;
+ struct comedi_subdevice *s = dev->read_subdev;
+ struct comedi_cmd *cmd = &s->async->cmd;
uint32_t *rps;
uint32_t jmp_adrs;
uint16_t i;
uint16_t n;
uint32_t local_ppl;
- struct comedi_cmd *cmd = &dev->subdevices->async->cmd;
/* Stop RPS program in case it is currently running */
s626_mc_disable(dev, S626_MC1_ERPS1, S626_P_MC1);
@@ -2639,6 +2640,7 @@ static void s626_initialize(struct comedi_device *dev)
* a defined state after a PCI reset.
*/
{
+ struct comedi_subdevice *s = dev->read_subdev;
uint8_t poll_list;
uint16_t adc_data;
uint16_t start_val;
@@ -2650,7 +2652,7 @@ static void s626_initialize(struct comedi_device *dev)
s626_reset_adc(dev, &poll_list);
/* Get initial ADC value */
- s626_ai_rinsn(dev, dev->subdevices, NULL, data);
+ s626_ai_rinsn(dev, s, NULL, data);
start_val = data[0];
/*
@@ -2664,7 +2666,7 @@ static void s626_initialize(struct comedi_device *dev)
* being unusually quiet or at the rail.
*/
for (index = 0; index < 500; index++) {
- s626_ai_rinsn(dev, dev->subdevices, NULL, data);
+ s626_ai_rinsn(dev, s, NULL, data);
adc_data = data[0];
if (adc_data != start_val)
break;
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv5][ 3/8] staging: imx-drm: Correct BGR666 and the board's dts that use them.
On Thursday, December 05, 2013 at 07:28:07 PM, Denis Carikli wrote: [...] Can you please explain the correction here ? Why is it needed ? What was the problem ? Thanks! Best regards, Marek Vasut ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv5][ 4/8] staging: imx-drm: Use de-active and pixelclk-active display-timings.
On Thursday, December 05, 2013 at 07:28:08 PM, Denis Carikli wrote:
> If de-active and/or pixelclk-active properties were set in the
> display-timings DT node, they were not used.
>
> Instead the data-enable and the pixel data clock polarity
> were hardcoded.
>
> This change is needed for making the eukrea-cpuimx51
> QVGA display work.
>
> Greg Kroah-Hartman
> Cc: driverdev-devel@linuxdriverproject.org
> Cc: Philipp Zabel
> Cc: Sascha Hauer
> Cc: Shawn Guo
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: David Airlie
> Cc: dri-de...@lists.freedesktop.org
> Cc: Eric Bénard
> Signed-off-by: Denis Carikli
> ---
> ChangeLog v3->v4:
> - The old patch was named "staging: imx-drm: ipuv3-crtc: don't harcode some
> mode". - Reworked the patch entierly: we now takes the mode flags from the
> device tree.
>
> ChangeLog v2->v3:
> - Added some interested people in the Cc list.
> - Ajusted the flags to match the changes in "drm: Add the lacking
> DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*"
> ---
> drivers/staging/imx-drm/imx-drm.h |3 +++
> drivers/staging/imx-drm/ipuv3-crtc.c | 11 +--
> drivers/staging/imx-drm/parallel-display.c | 28
> 3 files changed, 40 insertions(+), 2
> deletions(-)
>
> diff --git a/drivers/staging/imx-drm/imx-drm.h
> b/drivers/staging/imx-drm/imx-drm.h index ae90c9c..dfdc180 100644
> --- a/drivers/staging/imx-drm/imx-drm.h
> +++ b/drivers/staging/imx-drm/imx-drm.h
> @@ -5,6 +5,9 @@
>
> #define IPU_PIX_FMT_GBR24v4l2_fourcc('G', 'B', 'R', '3')
>
> +#define IMXDRM_MODE_FLAG_DE_HIGH (1<<0)
> +#define IMXDRM_MODE_FLAG_PIXDATA_POSEDGE (1<<1)
> +
> struct drm_crtc;
> struct drm_connector;
> struct drm_device;
> diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c
> b/drivers/staging/imx-drm/ipuv3-crtc.c index ce6ba98..f3d2cae 100644
> --- a/drivers/staging/imx-drm/ipuv3-crtc.c
> +++ b/drivers/staging/imx-drm/ipuv3-crtc.c
> @@ -156,8 +156,15 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
> if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> sig_cfg.Vsync_pol = 1;
>
> - sig_cfg.enable_pol = 1;
> - sig_cfg.clk_pol = 1;
> + /* Such flags are not availables in the DRM modes header,
> + * and we don't want to export them to userspace.
> + */
The multiline comment here is not correct.
> + if (mode->private_flags & IMXDRM_MODE_FLAG_DE_HIGH)
> + sig_cfg.enable_pol = 1;
> +
> + if (mode->private_flags & IMXDRM_MODE_FLAG_PIXDATA_POSEDGE)
> + sig_cfg.clk_pol = 1;
> +
> sig_cfg.width = mode->hdisplay;
> sig_cfg.height = mode->vdisplay;
> sig_cfg.pixel_fmt = out_pixel_fmt;
> diff --git a/drivers/staging/imx-drm/parallel-display.c
> b/drivers/staging/imx-drm/parallel-display.c index bb71d6d..65d0c18 100644
> --- a/drivers/staging/imx-drm/parallel-display.c
> +++ b/drivers/staging/imx-drm/parallel-display.c
> @@ -74,7 +74,35 @@ static int imx_pd_connector_get_modes(struct
> drm_connector *connector)
>
> if (np) {
> struct drm_display_mode *mode = drm_mode_create(connector->dev);
> + struct device_node *timings_np;
> + struct device_node *mode_np;
> + u32 val;
> +
> of_get_drm_display_mode(np, &imxpd->mode, 0);
> +
> + /* Such flags are not availables in the DRM modes header,
> + * and we don't want to export them to userspace.
> + */
DTTO
> + timings_np = of_get_child_by_name(np, "display-timings");
> + if (timings_np) {
> + /* get the display mode node */
> + mode_np = of_parse_phandle(timings_np,
> +"native-mode", 0);
> + if (!mode_np)
> + mode_np = of_get_next_child(timings_np, NULL);
> +
> + /* set de-active to 1 if not set */
> + of_property_read_u32(mode_np, "de-active", &val);
> + if (!!val)
'if (val)' is enough.
> + imxpd->mode.private_flags |=
> + IMXDRM_MODE_FLAG_DE_HIGH;
> +
> + /* set pixelclk-active to 1 if not set */
> + of_property_read_u32(mode_np, "pixelclk-active", &val);
> + if (!!val)
DTTO, also please add {} around multiline expression.
> + imxpd->mode.private_flags |=
> + IMXDRM_MODE_FLAG_PIXDATA_POSEDGE;
> + }
> drm_mode_copy(mode, &imxpd->mode);
> mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> drm_mode_probed_add(connector, mode);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv5][ 5/8] staging: imx-drm: parallel display: add regulator support.
On Thursday, December 05, 2013 at 07:28:09 PM, Denis Carikli wrote:
> Cc: Dan Carpenter
> Cc: Rob Herring
> Cc: Pawel Moll
> Cc: Mark Rutland
> Cc: Stephen Warren
> Cc: Ian Campbell
> Cc: devicet...@vger.kernel.org
> Cc: Greg Kroah-Hartman
> Cc: driverdev-devel@linuxdriverproject.org
> Cc: David Airlie
> Cc: dri-de...@lists.freedesktop.org
> Cc: Sascha Hauer
> Cc: Shawn Guo
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: Eric Bénard
> Signed-off-by: Denis Carikli
> ---
> ChangeLog v3->v5:
> - Code clenaup.
>
> ChangeLog v2->v3:
> - Added some interested people in the Cc list.
> - the lcd-supply is now called display-supply (not all display are LCD).
> - The code and documentation was updated accordingly.
> - regulator_is_enabled now guard the regulator enables/disables because
> regulator_disable does not check the regulator state.
> ---
> .../bindings/staging/imx-drm/fsl-imx-drm.txt |1 +
> drivers/staging/imx-drm/parallel-display.c | 22
> 2 files changed, 23 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
> b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt index
> 2d24425..4dd7ce5 100644
> --- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
> +++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
> @@ -28,6 +28,7 @@ Required properties:
> - compatible: Should be "fsl,imx-parallel-display"
> - crtc: the crtc this display is connected to, see below
> Optional properties:
> +- display-supply : phandle to the regulator device tree node if needed.
> - interface_pix_fmt: How this display is connected to the
>crtc. Currently supported types: "rgb24", "rgb565", "bgr666", "rgb666"
> - edid: verbatim EDID data block describing attached display.
> diff --git a/drivers/staging/imx-drm/parallel-display.c
> b/drivers/staging/imx-drm/parallel-display.c index 65d0c18..61c0aeb 100644
> --- a/drivers/staging/imx-drm/parallel-display.c
> +++ b/drivers/staging/imx-drm/parallel-display.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include "imx-drm.h"
> @@ -35,6 +36,7 @@ struct imx_parallel_display {
> struct drm_encoder encoder;
> struct imx_drm_encoder *imx_drm_encoder;
> struct device *dev;
> + struct regulator *disp_reg;
> void *edid;
> int edid_len;
> u32 interface_pix_fmt;
> @@ -141,6 +143,13 @@ static void imx_pd_encoder_prepare(struct drm_encoder
> *encoder) {
> struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
>
> + if (!IS_ERR(imxpd->disp_reg) &&
> + !regulator_is_enabled(imxpd->disp_reg)) {
> + if (regulator_enable(imxpd->disp_reg))
> + dev_err(imxpd->dev,
> + "Failed to enable regulator.\n");
I wonder, is this linebreak needed for this function call ?
[...]
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv5][ 6/8] ARM: dts: mbimx51sd: Add display support.
On Thursday, December 05, 2013 at 07:28:10 PM, Denis Carikli wrote:
> The CMO-QVGA, DVI-SVGA and DVI-VGA are added.
>
> Cc: Shawn Guo
> Cc: Sascha Hauer
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: Eric Bénard
> Signed-off-by: Denis Carikli
> ---
> ChangeLog v3->v5:
> - Updated to new GPIO defines.
> - Updated to new licenses checkpatch requirements.
> - one whitespace cleanup.
>
> ChangeLog v2->v3:
> - Splitted out from the patch that added support for the cpuimx51/mbimxsd51
> boards. - This patch now only adds display support.
> - Added some interested people in the Cc list, and removed some people that
> might be annoyed by the receiving of that patch which is unrelated to
> their subsystem.
> - rebased and reworked the dts displays addition.
> - Also rebased and reworked the fsl,pins usage.
> ---
> .../imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts | 55
> .../imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts |
> 42 +++ .../imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts |
> 42 +++ .../boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts |
> 13 +
> 4 files changed, 152 insertions(+)
> create mode 100644
> arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts create
> mode 100644
> arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts create
> mode 100644 arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts
>
> diff --git
> a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
> b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts new file
> mode 100644
> index 000..f37d65b
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright 2013 Eukréa Electromatique
> + *
> + * 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.
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include "imx51-eukrea-mbimxsd51-baseboard.dts"
> +
> +/ {
> + model = "Eukrea MBIMXSD51 with the CMO-QVGA Display";
> + compatible = "eukrea,mbimxsd51-baseboard-cmo-qvga",
> "eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51"; +
> + reg_lcd_3v3: lcd-en {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_reg_lcd_3v3>;
> + regulator-name = "lcd-3v3";
> + regulator-min-microvolt = <330>;
> + regulator-max-microvolt = <330>;
> + gpio = <&gpio3 13 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +};
> +
> +&display {
> + display-supply = <®_lcd_3v3>;
> + status = "okay";
The "status = " here should probably be in the board DTS, not in the LCD DTS.
> + display-timings {
> + model = "CMO-QVGA";
> + bits-per-pixel = <16>;
> + cmoqvga {
> + native-mode;
> + clock-frequency = <650>;
> + hactive = <320>;
> + vactive = <240>;
> + hfront-porch = <20>;
> + hback-porch = <38>;
> + vfront-porch = <4>;
> + vback-porch = <15>;
> + hsync-len = <30>;
> + vsync-len = <3>;
> + hsync-active = <0>;
> + vsync-active = <0>;
> + de-active = <0>;
> + pixelclk-active = <1>;
> + };
> + };
> +};
[...]
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: silicom: fix 'return is not a function, parentheses are not required' in bpctl_mod.c
Fixes warnings regarding redundant parantheses thrown by the checkpatch tool in
bpctl_mod.c
Signed-off-by: Will Tange
---
drivers/staging/silicom/bpctl_mod.c | 122 ++--
1 file changed, 61 insertions(+), 61 deletions(-)
diff --git a/drivers/staging/silicom/bpctl_mod.c
b/drivers/staging/silicom/bpctl_mod.c
index 39dc92a..ffd5d48 100644
--- a/drivers/staging/silicom/bpctl_mod.c
+++ b/drivers/staging/silicom/bpctl_mod.c
@@ -3125,11 +3125,11 @@ static int tx_status(struct bpctl_dev *pbpctl_dev)
ctrl = BPCTL_READ_REG(pbpctl_dev, CTRL);
if (pbpctl_dev->bp_i80)
- return ((ctrl & BPCTLI_CTRL_SWDPIN1) != 0 ? 0 : 1);
+ return (ctrl & BPCTLI_CTRL_SWDPIN1) != 0 ? 0 : 1;
if (pbpctl_dev->bp_540) {
ctrl = BP10G_READ_REG(pbpctl_dev, ESDP);
- return ((ctrl & BP10G_SDP1_DATA) != 0 ? 0 : 1);
+ return (ctrl & BP10G_SDP1_DATA) != 0 ? 0 : 1;
}
}
@@ -3150,8 +3150,8 @@ static int tx_status(struct bpctl_dev *pbpctl_dev)
}
if (pbpctl_dev->bp_10g9) {
- return ((BP10G_READ_REG(pbpctl_dev, ESDP) &
-BP10G_SDP3_DATA) != 0 ? 0 : 1);
+ return (BP10G_READ_REG(pbpctl_dev, ESDP) &
+BP10G_SDP3_DATA) != 0 ? 0 : 1;
} else if (pbpctl_dev->bp_fiber5) {
ctrl = BPCTL_READ_REG(pbpctl_dev, CTRL_EXT);
@@ -3186,10 +3186,10 @@ static int tx_status(struct bpctl_dev *pbpctl_dev)
if (pbpctl_dev->bp_540) {
ctrl = BP10G_READ_REG(pbpctl_dev, ESDP);
- return ((ctrl & BP10G_SDP1_DATA) != 0 ? 0 : 1);
+ return (ctrl & BP10G_SDP1_DATA) != 0 ? 0 : 1;
}
- return ((ctrl & BPCTLI_CTRL_SWDPIN0) != 0 ? 0 : 1);
+ return (ctrl & BPCTLI_CTRL_SWDPIN0) != 0 ? 0 : 1;
} else
return ((BP10G_READ_REG(pbpctl_dev, ESDP) &
BP10G_SDP0_DATA) != 0 ? 0 : 1);
@@ -3204,8 +3204,8 @@ static int bp_force_link_status(struct bpctl_dev
*pbpctl_dev)
if (DBI_IF_SERIES(pbpctl_dev->subdevice)) {
if ((pbpctl_dev->bp_10g) || (pbpctl_dev->bp_10g9)) {
- return ((BP10G_READ_REG(pbpctl_dev, ESDP) &
-BP10G_SDP1_DIR) != 0 ? 1 : 0);
+ return (BP10G_READ_REG(pbpctl_dev, ESDP) &
+BP10G_SDP1_DIR) != 0 ? 1 : 0;
}
}
@@ -3251,9 +3251,9 @@ int bypass_flag_status(struct bpctl_dev *pbpctl_dev)
if ((pbpctl_dev->bp_caps & BP_CAP)) {
if (pbpctl_dev->bp_ext_ver >= PXG2BPI_VER) {
- return read_reg(pbpctl_dev, STATUS_REG_ADDR)) &
+ return (((read_reg(pbpctl_dev, STATUS_REG_ADDR)) &
BYPASS_FLAG_MASK) ==
-BYPASS_FLAG_MASK) ? 1 : 0);
+BYPASS_FLAG_MASK) ? 1 : 0;
}
}
return BP_NOT_CAP;
@@ -3298,8 +3298,8 @@ int bypass_off_status(struct bpctl_dev *pbpctl_dev)
if (pbpctl_dev->bp_caps & BP_CAP) {
if (pbpctl_dev->bp_ext_ver >= PXG2BPI_VER) {
- return read_reg(pbpctl_dev, STATUS_REG_ADDR)) &
- BYPASS_OFF_MASK) == BYPASS_OFF_MASK) ? 1 : 0);
+ return (((read_reg(pbpctl_dev, STATUS_REG_ADDR)) &
+ BYPASS_OFF_MASK) == BYPASS_OFF_MASK) ? 1 : 0;
}
}
return BP_NOT_CAP;
@@ -,18 +,18 @@ static int bypass_status(struct bpctl_dev *pbpctl_dev)
ctrl_ext = BP10G_READ_REG(pbpctl_dev_b, I2CCTL);
BP10G_WRITE_REG(pbpctl_dev_b, I2CCTL,
(ctrl_ext | BP10G_I2C_CLK_OUT));
- return ((BP10G_READ_REG(pbpctl_dev_b, I2CCTL) &
-BP10G_I2C_CLK_IN) != 0 ? 0 : 1);
+ return (BP10G_READ_REG(pbpctl_dev_b, I2CCTL) &
+BP10G_I2C_CLK_IN) != 0 ? 0 : 1;
} else if (pbpctl_dev->bp_540) {
- return (((BP10G_READ_REG(pbpctl_dev_b, ESDP)) &
-BP10G_SDP0_DATA) != 0 ? 0 : 1);
+ return ((BP10G_READ_REG(pbpctl_dev_b, ESDP)) &
+BP10G_SDP0_DATA) != 0 ? 0 : 1;
}
else if ((pbpctl_dev->bp_fiber5)
|| (pbpc
[PATCH 3/7] staging: et131x: replace magic number bitmask with defined values
Signed-off-by: Mark Einon --- drivers/staging/et131x/et131x.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c index 4d76d79..21132a0 100644 --- a/drivers/staging/et131x/et131x.c +++ b/drivers/staging/et131x/et131x.c @@ -4122,7 +4122,7 @@ static void et131x_isr_handler(struct work_struct *work) if (status & ET_INTR_RXDMA_XFR_DONE) et131x_handle_recv_interrupt(adapter); - status &= 0xffd7; + status |= ~(ET_INTR_TXDMA_ERR | ET_INTR_RXDMA_XFR_DONE); if (!status) goto out; -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/7] staging: et131x: remove two useless debug statements
Signed-off-by: Mark Einon
---
drivers/staging/et131x/et131x.c |3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index 4d5e238..95470e0 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -2302,8 +2302,6 @@ static int et131x_rx_dma_memory_alloc(struct
et131x_adapter *adapter)
"Cannot alloc memory for Packet Status Ring\n");
return -ENOMEM;
}
- pr_info("Packet Status Ring %llx\n",
- (unsigned long long) rx_ring->ps_ring_physaddr);
/* NOTE : dma_alloc_coherent(), used above to alloc DMA regions,
* ALWAYS returns SAC (32-bit) addresses. If DAC (64-bit) addresses
@@ -2322,7 +2320,6 @@ static int et131x_rx_dma_memory_alloc(struct
et131x_adapter *adapter)
return -ENOMEM;
}
rx_ring->num_rfd = NIC_DEFAULT_NUM_RFD;
- pr_info("PRS %llx\n", (unsigned long long)rx_ring->rx_status_bus);
/* The RFDs are going to be put on lists later on, so initialize the
* lists now.
--
1.7.10.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/7] staging: et131x: clear up use of TRUEPHY defines
There are a large number of TRUEPHY defines in the driver, all of which
are unused or unnecessary. Remove / replace these.
As this results in et1310_phy_access_mii_bit() only being used for
reading bits, also change it's name to et1310_phy_read_mii_bit().
Signed-off-by: Mark Einon
---
drivers/staging/et131x/et131x.c | 45 ++-
drivers/staging/et131x/et131x.h | 40 --
2 files changed, 11 insertions(+), 74 deletions(-)
diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index 21132a0..4d5e238 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -1494,10 +1494,10 @@ static int et131x_mii_write(struct et131x_adapter
*adapter, u8 reg, u16 value)
return status;
}
-/* Still used from _mac for BIT_READ */
-static void et1310_phy_access_mii_bit(struct et131x_adapter *adapter,
- u16 action, u16 regnum, u16 bitnum,
- u8 *value)
+static void et1310_phy_read_mii_bit(struct et131x_adapter *adapter,
+ u16 regnum,
+ u16 bitnum,
+ u8 *value)
{
u16 reg;
u16 mask = 1 << bitnum;
@@ -1505,22 +1505,7 @@ static void et1310_phy_access_mii_bit(struct
et131x_adapter *adapter,
/* Read the requested register */
et131x_mii_read(adapter, regnum, ®);
- switch (action) {
- case TRUEPHY_BIT_READ:
- *value = (reg & mask) >> bitnum;
- break;
-
- case TRUEPHY_BIT_SET:
- et131x_mii_write(adapter, regnum, reg | mask);
- break;
-
- case TRUEPHY_BIT_CLEAR:
- et131x_mii_write(adapter, regnum, reg & ~mask);
- break;
-
- default:
- break;
- }
+ *value = (reg & mask) >> bitnum;
}
static void et1310_config_flow_control(struct et131x_adapter *adapter)
@@ -1532,27 +1517,19 @@ static void et1310_config_flow_control(struct
et131x_adapter *adapter)
} else {
char remote_pause, remote_async_pause;
- et1310_phy_access_mii_bit(adapter,
- TRUEPHY_BIT_READ, 5, 10, &remote_pause);
- et1310_phy_access_mii_bit(adapter,
- TRUEPHY_BIT_READ, 5, 11,
- &remote_async_pause);
+ et1310_phy_read_mii_bit(adapter, 5, 10, &remote_pause);
+ et1310_phy_read_mii_bit(adapter, 5, 11, &remote_async_pause);
- if ((remote_pause == TRUEPHY_BIT_SET) &&
- (remote_async_pause == TRUEPHY_BIT_SET)) {
+ if (remote_pause && remote_async_pause) {
adapter->flowcontrol = adapter->wanted_flow;
- } else if ((remote_pause == TRUEPHY_BIT_SET) &&
- (remote_async_pause == TRUEPHY_BIT_CLEAR)) {
+ } else if (remote_pause && !remote_async_pause) {
if (adapter->wanted_flow == FLOW_BOTH)
adapter->flowcontrol = FLOW_BOTH;
else
adapter->flowcontrol = FLOW_NONE;
- } else if ((remote_pause == TRUEPHY_BIT_CLEAR) &&
- (remote_async_pause == TRUEPHY_BIT_CLEAR)) {
+ } else if (!remote_pause && !remote_async_pause) {
adapter->flowcontrol = FLOW_NONE;
- } else {/* if (remote_pause == TRUEPHY_CLEAR_BIT &&
-* remote_async_pause == TRUEPHY_SET_BIT)
-*/
+ } else {
if (adapter->wanted_flow == FLOW_BOTH)
adapter->flowcontrol = FLOW_RXONLY;
else
diff --git a/drivers/staging/et131x/et131x.h b/drivers/staging/et131x/et131x.h
index bbe78a7..2ac6e99 100644
--- a/drivers/staging/et131x/et131x.h
+++ b/drivers/staging/et131x/et131x.h
@@ -1668,43 +1668,3 @@ struct address_map {
#define LED_100TX_SHIFT4
/* MI Register 29 - 31: Reserved Reg(0x1D - 0x1E) */
-
-/* Defines for PHY access routines */
-
-/* Define bit operation flags */
-#define TRUEPHY_BIT_CLEAR 0
-#define TRUEPHY_BIT_SET 1
-#define TRUEPHY_BIT_READ2
-
-/* Define read/write operation flags */
-#ifndef TRUEPHY_READ
-#define TRUEPHY_READ0
-#define TRUEPHY_WRITE 1
-#define TRUEPHY_MASK2
-#endif
-
-/* Define master/slave configuration values */
-#define TRUEPHY_CFG_SLAVE 0
-#define TRUEPHY_CFG_MASTER 1
-
-/* Define MDI/MDI-X settings */
-#define TRUEPHY_MDI 0
-#define TRUEPHY_MDIX1
-#define TRUEPHY_AUTO_MDI_MDIX 2
-
-/* Define 10Base-T link
[PATCH 7/7] staging: et131x: trivial whitespace and line / character reductions
Tweak some whitespace, also remove a few redundant lines and
characters (mainly of type 'if (status != 0)' -> 'if (status)').
Signed-off-by: Mark Einon
---
drivers/staging/et131x/et131x.c | 32 ++--
1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index 479abfd..526369f 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -1039,9 +1039,7 @@ static void et1310_config_mac_regs2(struct et131x_adapter
*adapter)
*/
static int et1310_in_phy_coma(struct et131x_adapter *adapter)
{
- u32 pmcsr;
-
- pmcsr = readl(&adapter->regs->global.pm_csr);
+ u32 pmcsr = readl(&adapter->regs->global.pm_csr);
return ET_PM_PHY_SW_COMA & pmcsr ? 1 : 0;
}
@@ -1930,7 +1928,7 @@ static void et131x_enable_interrupts(struct
et131x_adapter *adapter)
/* Enable all global interrupts */
if (adapter->flowcontrol == FLOW_TXONLY ||
- adapter->flowcontrol == FLOW_BOTH)
+ adapter->flowcontrol == FLOW_BOTH)
mask = INT_MASK_ENABLE;
else
mask = INT_MASK_ENABLE_NO_FLOW;
@@ -2115,7 +2113,7 @@ static inline u32 bump_free_buff_ring(u32
*free_buff_ring, u32 limit)
tmp_free_buff_ring ^= ET_DMA10_WRAP;
}
/* For the 1023 case */
- tmp_free_buff_ring &= (ET_DMA10_MASK|ET_DMA10_WRAP);
+ tmp_free_buff_ring &= (ET_DMA10_MASK | ET_DMA10_WRAP);
*free_buff_ring = tmp_free_buff_ring;
return tmp_free_buff_ring;
}
@@ -2620,7 +2618,7 @@ static struct rfd *nic_rx_pkts(struct et131x_adapter
*adapter)
adapter->stats.unicast_pkts_rcvd++;
}
- if (len == 0) {
+ if (!len) {
rfd->len = 0;
goto out;
}
@@ -3474,7 +3472,7 @@ static void et131x_hwaddr_init(struct et131x_adapter
*adapter)
* the MAC address. At this point the I/O registers have yet to be mapped
*/
static int et131x_pci_init(struct et131x_adapter *adapter,
- struct pci_dev *pdev)
+ struct pci_dev *pdev)
{
u16 max_payload;
int i, rc;
@@ -3607,14 +3605,14 @@ static int et131x_adapter_memory_alloc(struct
et131x_adapter *adapter)
/* Allocate memory for the Tx Ring */
status = et131x_tx_dma_memory_alloc(adapter);
- if (status != 0) {
+ if (status) {
dev_err(&adapter->pdev->dev,
"et131x_tx_dma_memory_alloc FAILED\n");
return status;
}
/* Receive buffer memory allocation */
status = et131x_rx_dma_memory_alloc(adapter);
- if (status != 0) {
+ if (status) {
dev_err(&adapter->pdev->dev,
"et131x_rx_dma_memory_alloc FAILED\n");
et131x_tx_dma_memory_free(adapter);
@@ -3624,8 +3622,7 @@ static int et131x_adapter_memory_alloc(struct
et131x_adapter *adapter)
/* Init receive data structures */
status = et131x_init_recv(adapter);
if (status) {
- dev_err(&adapter->pdev->dev,
- "et131x_init_recv FAILED\n");
+ dev_err(&adapter->pdev->dev, "et131x_init_recv FAILED\n");
et131x_adapter_memory_free(adapter);
}
return status;
@@ -3755,7 +3752,8 @@ static int et131x_mii_probe(struct net_device *netdev)
phydev->advertising = phydev->supported;
adapter->phydev = phydev;
- dev_info(&adapter->pdev->dev, "attached PHY driver [%s]
(mii_bus:phy_addr=%s)\n",
+ dev_info(&adapter->pdev->dev,
+"attached PHY driver [%s] (mii_bus:phy_addr=%s)\n",
phydev->drv->name, dev_name(&phydev->dev));
return 0;
@@ -3767,7 +3765,7 @@ static int et131x_mii_probe(struct net_device *netdev)
* them together with the platform provided device structures.
*/
static struct et131x_adapter *et131x_adapter_init(struct net_device *netdev,
- struct pci_dev *pdev)
+ struct pci_dev *pdev)
{
static const u8 default_mac[] = { 0x00, 0x05, 0x3d, 0x00, 0x02, 0x00 };
@@ -3940,7 +3938,7 @@ static irqreturn_t et131x_isr(int irq, void *dev_id)
status &= ~ET_INTR_WATCHDOG;
}
- if (status == 0) {
+ if (!status) {
/* This interrupt has in some way been "handled" by
* the ISR. Either it was a spurious Rx interrupt, or
* it was a Tx interrupt that has been filtered by
@@ -3996,10 +3994,8 @@ static void et131x_isr_handler(struct work_struct *work)
/* Handle the TXDMA Error interrupt */
if (status & ET_INTR_TXDMA_ERR) {
- u32 txdma_err;
-
/* Following read also clears the register (COR) */
- txdma_err = readl(&iomem
[PATCH 2/7] staging: et131x: Remove unnecessary phydev checks
Several checks for a valid adapter->phydev pointer are made where the
pointer has already been checked previously in the code path.
Remove these redundant checks.
Signed-off-by: Mark Einon
---
drivers/staging/et131x/et131x.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index 8a30726..4d76d79 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -968,7 +968,7 @@ static void et1310_config_mac_regs2(struct et131x_adapter
*adapter)
/* Set up the if mode bits */
cfg2 &= ~ET_MAC_CFG2_IFMODE_MASK;
- if (phydev && phydev->speed == SPEED_1000) {
+ if (phydev->speed == SPEED_1000) {
cfg2 |= ET_MAC_CFG2_IFMODE_1000;
/* Phy mode bit */
ifctrl &= ~ET_MAC_IFCTRL_PHYMODE;
@@ -999,11 +999,11 @@ static void et1310_config_mac_regs2(struct et131x_adapter
*adapter)
cfg2 &= ~ET_MAC_CFG2_IFMODE_FULL_DPLX;
/* Turn on duplex if needed */
- if (phydev && phydev->duplex == DUPLEX_FULL)
+ if (phydev->duplex == DUPLEX_FULL)
cfg2 |= ET_MAC_CFG2_IFMODE_FULL_DPLX;
ifctrl &= ~ET_MAC_IFCTRL_GHDMODE;
- if (phydev && phydev->duplex == DUPLEX_HALF)
+ if (phydev->duplex == DUPLEX_HALF)
ifctrl |= ET_MAC_IFCTRL_GHDMODE;
writel(ifctrl, &mac->if_ctrl);
@@ -2480,9 +2480,6 @@ static void et131x_set_rx_dma_timer(struct et131x_adapter
*adapter)
{
struct phy_device *phydev = adapter->phydev;
- if (!phydev)
- return;
-
/* For version B silicon, we do not use the RxDMA timer for 10 and 100
* Mbits/s line rates. We do not enable and RxDMA interrupt coalescing.
*/
@@ -3771,7 +3768,7 @@ static void et131x_adjust_link(struct net_device *netdev)
if (phydev->link) {
adapter->boot_coma = 20;
- if (phydev && phydev->speed == SPEED_10) {
+ if (phydev->speed == SPEED_10) {
/* NOTE - Is there a way to query this without
* TruePHY?
* && TRU_QueryCoreType(adapter->hTruePhy, 0)==
@@ -3793,7 +3790,7 @@ static void et131x_adjust_link(struct net_device *netdev)
et1310_config_flow_control(adapter);
- if (phydev && phydev->speed == SPEED_1000 &&
+ if (phydev->speed == SPEED_1000 &&
adapter->registry_jumbo_packet > 2048) {
u16 reg;
--
1.7.10.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/7] staging: et131x: improve indenting in et131x_adjust_link()
Negate some 'if' checks to return early, allowing a large block of code
to be un-indented.
Signed-off-by: Mark Einon
---
drivers/staging/et131x/et131x.c | 160 ---
1 file changed, 81 insertions(+), 79 deletions(-)
diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index e3a71d3..8a30726 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -3754,97 +3754,99 @@ static void et131x_adjust_link(struct net_device
*netdev)
struct et131x_adapter *adapter = netdev_priv(netdev);
struct phy_device *phydev = adapter->phydev;
- if (phydev && phydev->link != adapter->link) {
- /* Check to see if we are in coma mode and if
-* so, disable it because we will not be able
-* to read PHY values until we are out.
-*/
- if (et1310_in_phy_coma(adapter))
- et1310_disable_phy_coma(adapter);
-
- adapter->link = phydev->link;
- phy_print_status(phydev);
-
- if (phydev->link) {
- adapter->boot_coma = 20;
- if (phydev && phydev->speed == SPEED_10) {
- /* NOTE - Is there a way to query this without
-* TruePHY?
-* && TRU_QueryCoreType(adapter->hTruePhy, 0)==
-* EMI_TRUEPHY_A13O) {
-*/
- u16 register18;
-
- et131x_mii_read(adapter, PHY_MPHY_CONTROL_REG,
-®ister18);
- et131x_mii_write(adapter, PHY_MPHY_CONTROL_REG,
-register18 | 0x4);
- et131x_mii_write(adapter, PHY_INDEX_REG,
-register18 | 0x8402);
- et131x_mii_write(adapter, PHY_DATA_REG,
-register18 | 511);
- et131x_mii_write(adapter, PHY_MPHY_CONTROL_REG,
-register18);
- }
+ if (!phydev)
+ return;
+ if (phydev->link == adapter->link)
+ return;
- et1310_config_flow_control(adapter);
+ /* Check to see if we are in coma mode and if
+* so, disable it because we will not be able
+* to read PHY values until we are out.
+*/
+ if (et1310_in_phy_coma(adapter))
+ et1310_disable_phy_coma(adapter);
- if (phydev && phydev->speed == SPEED_1000 &&
- adapter->registry_jumbo_packet > 2048) {
- u16 reg;
+ adapter->link = phydev->link;
+ phy_print_status(phydev);
- et131x_mii_read(adapter, PHY_CONFIG, ®);
- reg &= ~ET_PHY_CONFIG_TX_FIFO_DEPTH;
- reg |= ET_PHY_CONFIG_FIFO_DEPTH_32;
- et131x_mii_write(adapter, PHY_CONFIG, reg);
- }
+ if (phydev->link) {
+ adapter->boot_coma = 20;
+ if (phydev && phydev->speed == SPEED_10) {
+ /* NOTE - Is there a way to query this without
+* TruePHY?
+* && TRU_QueryCoreType(adapter->hTruePhy, 0)==
+* EMI_TRUEPHY_A13O) {
+*/
+ u16 register18;
+
+ et131x_mii_read(adapter, PHY_MPHY_CONTROL_REG,
+®ister18);
+ et131x_mii_write(adapter, PHY_MPHY_CONTROL_REG,
+register18 | 0x4);
+ et131x_mii_write(adapter, PHY_INDEX_REG,
+register18 | 0x8402);
+ et131x_mii_write(adapter, PHY_DATA_REG,
+register18 | 511);
+ et131x_mii_write(adapter, PHY_MPHY_CONTROL_REG,
+register18);
+ }
- et131x_set_rx_dma_timer(adapter);
- et1310_config_mac_regs2(adapter);
- } else {
- adapter->boot_coma = 0;
+ et1310_config_flow_control(adapter);
- if (phydev->speed == SPEED_10) {
- /* NOTE - Is there a way to query this without
-* TruePHY?
-* && TRU_QueryCoreType(adapter->hTruePhy, 0) ==
-* EMI_TRUEPHY_A13O)
-
[PATCH 6/7] staging: et131x: remove unhelpful comments
Get rid of some of the more unhelpful comments.
Signed-off-by: Mark Einon
---
drivers/staging/et131x/et131x.c | 204 ++-
1 file changed, 28 insertions(+), 176 deletions(-)
diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index 95470e0..479abfd 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -1351,8 +1351,6 @@ static void et1310_config_macstat_regs(struct
et131x_adapter *adapter)
* @addr: the address of the transceiver
* @reg: the register to read
* @value: pointer to a 16-bit value in which the value will be stored
- *
- * Returns 0 on success, errno on failure (as defined in errno.h)
*/
static int et131x_phy_mii_read(struct et131x_adapter *adapter, u8 addr,
u8 reg, u16 *value)
@@ -1425,10 +1423,6 @@ static int et131x_mii_read(struct et131x_adapter
*adapter, u8 reg, u16 *value)
* @adapter: pointer to our private adapter structure
* @reg: the register to read
* @value: 16-bit value to write
- *
- * FIXME: one caller in netdev still
- *
- * Return 0 on success, errno on failure (as defined in errno.h)
*/
static int et131x_mii_write(struct et131x_adapter *adapter, u8 reg, u16 value)
{
@@ -1538,9 +1532,7 @@ static void et1310_config_flow_control(struct
et131x_adapter *adapter)
}
}
-/* et1310_update_macstat_host_counters - Update the local copy of the
statistics
- * @adapter: pointer to the adapter structure
- */
+/* et1310_update_macstat_host_counters - Update local copy of the statistics */
static void et1310_update_macstat_host_counters(struct et131x_adapter *adapter)
{
struct ce_stats *stats = &adapter->stats;
@@ -1566,7 +1558,6 @@ static void et1310_update_macstat_host_counters(struct
et131x_adapter *adapter)
}
/* et1310_handle_macstat_interrupt
- * @adapter: pointer to the adapter structure
*
* One of the MACSTAT counters has wrapped. Update the local copy of
* the statistics held in the adapter structure, checking the "wrap"
@@ -1676,10 +1667,7 @@ static void et1310_phy_power_switch(struct
et131x_adapter *adapter, bool down)
et131x_mii_write(adapter, MII_BMCR, data);
}
-/* et131x_xcvr_init - Init the phy if we are setting it into force mode
- * @adapter: pointer to our private adapter structure
- *
- */
+/* et131x_xcvr_init - Init the phy if we are setting it into force mode */
static void et131x_xcvr_init(struct et131x_adapter *adapter)
{
u16 lcr2;
@@ -1708,7 +1696,6 @@ static void et131x_xcvr_init(struct et131x_adapter
*adapter)
}
/* et131x_configure_global_regs- configure JAGCore global regs
- * @adapter: pointer to our adapter structure
*
* Used to configure the global registers on the JAGCore
*/
@@ -1753,9 +1740,7 @@ static void et131x_configure_global_regs(struct
et131x_adapter *adapter)
writel(0, ®s->watchdog_timer);
}
-/* et131x_config_rx_dma_regs - Start of Rx_DMA init sequence
- * @adapter: pointer to our adapter structure
- */
+/* et131x_config_rx_dma_regs - Start of Rx_DMA init sequence */
static void et131x_config_rx_dma_regs(struct et131x_adapter *adapter)
{
struct rxdma_regs __iomem *rx_dma = &adapter->regs->rxdma;
@@ -1861,7 +1846,6 @@ static void et131x_config_rx_dma_regs(struct
et131x_adapter *adapter)
}
/* et131x_config_tx_dma_regs - Set up the tx dma section of the JAGCore.
- * @adapter: pointer to our private adapter structure
*
* Configure the transmit engine with the ring buffers we have created
* and prepare it for use.
@@ -1891,11 +1875,7 @@ static void et131x_config_tx_dma_regs(struct
et131x_adapter *adapter)
adapter->tx_ring.send_idx = 0;
}
-/* et131x_adapter_setup - Set the adapter up as per cassini+ documentation
- * @adapter: pointer to our private adapter structure
- *
- * Returns 0 on success, errno on failure (as defined in errno.h)
- */
+/* et131x_adapter_setup - Set the adapter up as per cassini+ documentation */
static void et131x_adapter_setup(struct et131x_adapter *adapter)
{
/* Configure the JAGCore */
@@ -1919,9 +1899,7 @@ static void et131x_adapter_setup(struct et131x_adapter
*adapter)
et131x_xcvr_init(adapter);
}
-/* et131x_soft_reset - Issue a soft reset to the hardware, complete for ET1310
- * @adapter: pointer to our private adapter structure
- */
+/* et131x_soft_reset - Issue soft reset to the hardware, complete for ET1310 */
static void et131x_soft_reset(struct et131x_adapter *adapter)
{
u32 reg;
@@ -1942,7 +1920,6 @@ static void et131x_soft_reset(struct et131x_adapter
*adapter)
}
/* et131x_enable_interrupts- enable interrupt
- * @adapter: et131x device
*
* Enable the appropriate interrupts on the ET131x according to our
* configuration
@@ -1962,7 +1939,6 @@ static void et131x_enable_interrupts(struct
et131x_adapter *adapter)
}
/* et131x_disable_interrupts - interrupt disable
- * @adapter: et131x devic
Re: [PATCH] staging: silicom: fix 'return is not a function, parentheses are not required' in bpctl_mod.c
On Thu, Dec 05, 2013 at 10:23:53PM +0100, Will Tange wrote:
> Fixes warnings regarding redundant parantheses thrown by the checkpatch tool
> in bpctl_mod.c
>
Fair enough, but if you wanted to go clean the returns up further then
you could. Remove all the "!= 0" bits.
> @@ -3125,11 +3125,11 @@ static int tx_status(struct bpctl_dev *pbpctl_dev)
>
> ctrl = BPCTL_READ_REG(pbpctl_dev, CTRL);
> if (pbpctl_dev->bp_i80)
> - return ((ctrl & BPCTLI_CTRL_SWDPIN1) != 0 ? 0 : 1);
> + return (ctrl & BPCTLI_CTRL_SWDPIN1) != 0 ? 0 : 1;
The double negative just makes the code not as not confusing as it could
be. Simpler:
return (ctrl & BPCTLI_CTRL_SWDPIN1) ? 0 : 1;
>
> if ((pbpctl_dev->bp_caps & BP_CAP)) {
> if (pbpctl_dev->bp_ext_ver >= PXG2BPI_VER) {
> - return read_reg(pbpctl_dev, STATUS_REG_ADDR)) &
> + return (((read_reg(pbpctl_dev, STATUS_REG_ADDR)) &
> BYPASS_FLAG_MASK) ==
> - BYPASS_FLAG_MASK) ? 1 : 0);
> + BYPASS_FLAG_MASK) ? 1 : 0;
These super long lines would be better if we introduced a temporary
variable.
reg = read_reg(pbpctl_dev, STATUS_REG_ADDR);
return (reg & BYPASS_FLAG_MASK) == BYPASS_FLAG_MASK;
BYPASS_FLAG_MASK is poorly named. It's actually just a bit or a flag
and not a mask, so it could be renamed.
reg = read_reg(pbpctl_dev, STATUS_REG_ADDR);
return (reg & BP_BYPASS_FLAG) ? 1 : 0;
Which is way simpler than the original and only 2 lines long instead of
4. I don't know that "BP_" is the right prefix... BYPASS_FLAG is too
generic.
> @@ -4730,7 +4730,7 @@ int get_disc_pwup_fn(struct bpctl_dev *pbpctl_dev)
> return -1;
>
> ret = default_pwron_disc_status(pbpctl_dev);
> - return (ret == 0 ? 1 : (ret < 0 ? BP_NOT_CAP : 0));
> + return ret == 0 ? 1 : (ret < 0 ? BP_NOT_CAP : 0);
if (ret < 0)
return BP_NOT_CAP;
if (ret == 0)
return 1;
return 0;
More lines, but simpler to understand than the original.
Think of checkpatch.pl as a pointer to bad code and not that we just
have to silence checkpatch and move on.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 06/31] staging: comedi: das1800: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/das1800.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/das1800.c
b/drivers/staging/comedi/drivers/das1800.c
index 92df02d..37e92a0 100644
--- a/drivers/staging/comedi/drivers/das1800.c
+++ b/drivers/staging/comedi/drivers/das1800.c
@@ -644,7 +644,7 @@ static int das1800_cancel(struct comedi_device *dev, struct
comedi_subdevice *s)
static void das1800_ai_handler(struct comedi_device *dev)
{
struct das1800_private *devpriv = dev->private;
- struct comedi_subdevice *s = &dev->subdevices[0];
+ struct comedi_subdevice *s = dev->read_subdev;
struct comedi_async *async = s->async;
struct comedi_cmd *cmd = &async->cmd;
unsigned int status = inb(dev->iobase + DAS1800_STATUS);
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 31/31] staging: comedi: multiq3: pass subdevice to encoder_reset()
Pass the subdevice pointer to encoder_reset() from the attach instead
of accessing the dev->subdevices array directly to get the pointer.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/multiq3.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/multiq3.c
b/drivers/staging/comedi/drivers/multiq3.c
index 3ca755e..81e6559 100644
--- a/drivers/staging/comedi/drivers/multiq3.c
+++ b/drivers/staging/comedi/drivers/multiq3.c
@@ -197,9 +197,9 @@ static int multiq3_encoder_insn_read(struct comedi_device
*dev,
return n;
}
-static void encoder_reset(struct comedi_device *dev)
+static void encoder_reset(struct comedi_device *dev,
+ struct comedi_subdevice *s)
{
- struct comedi_subdevice *s = &dev->subdevices[4];
int chan;
for (chan = 0; chan < s->n_chan; chan++) {
@@ -282,7 +282,7 @@ static int multiq3_attach(struct comedi_device *dev,
s->maxdata = 0xff;
s->range_table = &range_unknown;
- encoder_reset(dev);
+ encoder_reset(dev, s);
return 0;
}
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 16/31] staging: comedi: adv_pci1710: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adv_pci1710.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 69ab2a6..3957da5 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -731,7 +731,7 @@ static void interrupt_pci1710_every_sample(void *d)
{
struct comedi_device *dev = d;
struct pci1710_private *devpriv = dev->private;
- struct comedi_subdevice *s = &dev->subdevices[0];
+ struct comedi_subdevice *s = dev->read_subdev;
int m;
#ifdef PCI171x_PARANOIDCHECK
const struct boardtype *this_board = comedi_board(dev);
@@ -859,7 +859,7 @@ static void interrupt_pci1710_half_fifo(void *d)
struct comedi_device *dev = d;
const struct boardtype *this_board = comedi_board(dev);
struct pci1710_private *devpriv = dev->private;
- struct comedi_subdevice *s = &dev->subdevices[0];
+ struct comedi_subdevice *s = dev->read_subdev;
int m, samplesinbuf;
m = inw(dev->iobase + PCI171x_STATUS);
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 00/31] staging: comedi: cleanup irq requests
The comedi subsystem only requires the drivers to support interrupts if one or more of the subdevices support async commands. Since this is optional: 1) don't fail the attach if the irq is not available 2) only hookup the async command support if the irq is available 3) remove any async command init from subdevices that don't need it 4) remove any unnecessary sanity checks in the async command functions Make use of the comedi_device 'read_subdev' and 'write_subdev' pointers instead of accessing the dev->subdevices array directly with "magic" numbers to the correct subdevice. Also, remove any unnecessary debug noise associated with the irq requests. This series was originally posted on 3 Dec 2013 as: [PATCH 00/51] staging: comedi: cleanup irq requests The first 17 patches of that series hare already been applied to Greg Kroah-Hartman's staging tree. This series is a repost of the remaining patches after addressing some issues pointed out by Ian Abbott. H Hartley Sweeten (31): staging: comedi: ni_at_2150: tidy up irq/dma request staging: comedi: me4000: use dev->read_subdev staging: comedi: me4000: refactor request_irq() during attach staging: comedi: me4000: remove unnecessary check in the irq handler staging: comedi: das1800: tidy up irq request staging: comedi: das1800: use dev->read_subdev staging: comedi: das16m1: remove unnecessary 'dev->irq' test staging: comedi: adl_pci9111: fix incorrect irq passed to request_irq() staging: comedi: adl_pci9111: the irq is only needed for async command support staging: comedi: dt2814: use dev->read_subdev staging: comedi: dt282x: use dev->read_subdev staging: comedi: dt282x: use dev->write_subdev staging: comedi: amplc_pci230: tidy up irq request staging: comedi: adl_pci9118: tidy up irq request staging: comedi: adv_pci1710: only init async command members when needed staging: comedi: adv_pci1710: use dev->read_subdev staging: comedi: dt3000: don't fail attach if irq is not available staging: comedi: dt3000: use dev->read_subdev staging: comedi: s626: use dev->read_subdev staging: comedi: hwrdv_apci3120: use dev->read_subdev staging: comedi: hwrdv_apci3200: use dev->read_subdev staging: comedi: adl_pci9118: use dev->read_subdev staging: comedi: amplc_pc236: use dev->read_subdev staging: comedi: amplc_pci224: use dev->write_subdev staging: comedi: ni_65xx: use dev->read_subdev staging: comedi: ni_atmio16d: use dev->read_subdev staging: comedi: rtd520: use dev->read_subdev staging: comedi: ni_pcidio: factor board reset out of attach staging: comedi: ni_pcidio: use dev->read_subdev staging: comedi: ni_pcidio: request_irq() before seting up subdevices staging: comedi: multiq3: pass subdevice to encoder_reset() .../comedi/drivers/addi-data/hwdrv_apci3120.c | 6 +- .../comedi/drivers/addi-data/hwdrv_apci3200.c | 2 +- drivers/staging/comedi/drivers/adl_pci9111.c | 31 drivers/staging/comedi/drivers/adl_pci9118.c | 45 +-- drivers/staging/comedi/drivers/adv_pci1710.c | 10 +-- drivers/staging/comedi/drivers/amplc_pc236.c | 2 +- drivers/staging/comedi/drivers/amplc_pci224.c | 2 +- drivers/staging/comedi/drivers/amplc_pci230.c | 27 +++ drivers/staging/comedi/drivers/das16m1.c | 5 -- drivers/staging/comedi/drivers/das1800.c | 88 ++ drivers/staging/comedi/drivers/dt2814.c| 4 +- drivers/staging/comedi/drivers/dt282x.c| 10 +-- drivers/staging/comedi/drivers/dt3000.c| 29 +++ drivers/staging/comedi/drivers/me4000.c| 37 - drivers/staging/comedi/drivers/multiq3.c | 6 +- drivers/staging/comedi/drivers/ni_65xx.c | 2 +- drivers/staging/comedi/drivers/ni_at_a2150.c | 79 --- drivers/staging/comedi/drivers/ni_atmio16d.c | 2 +- drivers/staging/comedi/drivers/ni_pcidio.c | 64 +--- drivers/staging/comedi/drivers/rtd520.c| 2 +- drivers/staging/comedi/drivers/s626.c | 8 +- 21 files changed, 206 insertions(+), 255 deletions(-) -- 1.8.4.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 29/31] staging: comedi: ni_pcidio: use dev->read_subdev
Use the dev->read_subdev that was setup in the device attach instead
of accessing the dev->subdevices array directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/ni_pcidio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/ni_pcidio.c
b/drivers/staging/comedi/drivers/ni_pcidio.c
index ef72fc7..30c46a3 100644
--- a/drivers/staging/comedi/drivers/ni_pcidio.c
+++ b/drivers/staging/comedi/drivers/ni_pcidio.c
@@ -384,7 +384,7 @@ static irqreturn_t nidio_interrupt(int irq, void *d)
{
struct comedi_device *dev = d;
struct nidio96_private *devpriv = dev->private;
- struct comedi_subdevice *s = &dev->subdevices[0];
+ struct comedi_subdevice *s = dev->read_subdev;
struct comedi_async *async = s->async;
struct mite_struct *mite = devpriv->mite;
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 03/31] staging: comedi: me4000: refactor request_irq() during attach
Do the request_irq() before setting up the subdevices. This removes an
indent level and makes the code a bit cleaner.
Also, remove the dev_warn() noise about the irq.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/me4000.c | 29 +
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/comedi/drivers/me4000.c
b/drivers/staging/comedi/drivers/me4000.c
index e4afca4..cb1032f 100644
--- a/drivers/staging/comedi/drivers/me4000.c
+++ b/drivers/staging/comedi/drivers/me4000.c
@@ -1505,6 +1505,13 @@ static int me4000_auto_attach(struct comedi_device *dev,
me4000_reset(dev);
+ if (pcidev->irq > 0) {
+ result = request_irq(pcidev->irq, me4000_ai_isr, IRQF_SHARED,
+ dev->board_name, dev);
+ if (result == 0)
+ dev->irq = pcidev->irq;
+ }
+
result = comedi_alloc_subdevices(dev, 4);
if (result)
return result;
@@ -1525,22 +1532,12 @@ static int me4000_auto_attach(struct comedi_device *dev,
s->range_table = &me4000_ai_range;
s->insn_read = me4000_ai_insn_read;
- if (pcidev->irq > 0) {
- if (request_irq(pcidev->irq, me4000_ai_isr,
- IRQF_SHARED, dev->board_name, dev)) {
- dev_warn(dev->class_dev,
- "request_irq failed\n");
- } else {
- dev->read_subdev = s;
- s->subdev_flags |= SDF_CMD_READ;
- s->cancel = me4000_ai_cancel;
- s->do_cmdtest = me4000_ai_do_cmd_test;
- s->do_cmd = me4000_ai_do_cmd;
-
- dev->irq = pcidev->irq;
- }
- } else {
- dev_warn(dev->class_dev, "No interrupt available\n");
+ if (dev->irq) {
+ dev->read_subdev = s;
+ s->subdev_flags |= SDF_CMD_READ;
+ s->cancel = me4000_ai_cancel;
+ s->do_cmdtest = me4000_ai_do_cmd_test;
+ s->do_cmd = me4000_ai_do_cmd;
}
} else {
s->type = COMEDI_SUBD_UNUSED;
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 14/31] staging: comedi: adl_pci9118: tidy up irq request
Clean up the irq request in the attach of this driver and remove the
dev_{level} noise.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/adl_pci9118.c | 43 +++-
1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c
b/drivers/staging/comedi/drivers/adl_pci9118.c
index 97343cc..7e66978 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -1965,7 +1965,6 @@ static int pci9118_common_attach(struct comedi_device
*dev, int disable_irq,
struct pci_dev *pcidev = comedi_to_pci_dev(dev);
struct comedi_subdevice *s;
int ret, pages, i;
- unsigned int irq;
u16 u16w;
dev->board_name = this_board->name;
@@ -2036,12 +2035,18 @@ static int pci9118_common_attach(struct comedi_device
*dev, int disable_irq,
pci_write_config_word(pcidev, PCI_COMMAND, u16w | 64);
/* Enable parity check for parity error */
+ if (!disable_irq && pcidev->irq) {
+ ret = request_irq(pcidev->irq, interrupt_pci9118, IRQF_SHARED,
+ dev->board_name, dev);
+ if (ret == 0)
+ dev->irq = pcidev->irq;
+ }
+
ret = comedi_alloc_subdevices(dev, 4);
if (ret)
return ret;
s = &dev->subdevices[0];
- dev->read_subdev = s;
s->type = COMEDI_SUBD_AI;
s->subdev_flags = SDF_READABLE | SDF_COMMON | SDF_GROUND | SDF_DIFF;
if (devpriv->usemux)
@@ -2050,11 +2055,17 @@ static int pci9118_common_attach(struct comedi_device
*dev, int disable_irq,
s->n_chan = this_board->n_aichan;
s->maxdata = this_board->ai_maxdata;
- s->len_chanlist = this_board->n_aichanlist;
s->range_table = this_board->rangelist_ai;
- s->cancel = pci9118_ai_cancel;
s->insn_read = pci9118_insn_read_ai;
- s->munge = pci9118_ai_munge;
+ if (dev->irq) {
+ dev->read_subdev = s;
+ s->subdev_flags |= SDF_CMD_READ;
+ s->len_chanlist = this_board->n_aichanlist;
+ s->do_cmdtest = pci9118_ai_cmdtest;
+ s->do_cmd = pci9118_ai_cmd;
+ s->cancel = pci9118_ai_cancel;
+ s->munge = pci9118_ai_munge;
+ }
s = &dev->subdevices[1];
s->type = COMEDI_SUBD_AO;
@@ -2100,27 +2111,7 @@ static int pci9118_common_attach(struct comedi_device
*dev, int disable_irq,
break;
}
- if (disable_irq)
- irq = 0;
- else
- irq = pcidev->irq;
- if (irq > 0) {
- if (request_irq(irq, interrupt_pci9118, IRQF_SHARED,
- dev->board_name, dev)) {
- dev_warn(dev->class_dev,
-"unable to allocate IRQ %u, DISABLING IT\n",
-irq);
- } else {
- dev->irq = irq;
- /* Enable AI commands */
- s = &dev->subdevices[0];
- s->subdev_flags |= SDF_CMD_READ;
- s->do_cmdtest = pci9118_ai_cmdtest;
- s->do_cmd = pci9118_ai_cmd;
- }
- }
-
- pci9118_report_attach(dev, irq);
+ pci9118_report_attach(dev, dev->irq);
return 0;
}
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 28/31] staging: comedi: ni_pcidio: factor board reset out of attach
Factor the code that resets the board and disables the interrupts out
of the attach.
Move the reset so it happens before the subdevices are initialized.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/ni_pcidio.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_pcidio.c
b/drivers/staging/comedi/drivers/ni_pcidio.c
index 645a4d8..ef72fc7 100644
--- a/drivers/staging/comedi/drivers/ni_pcidio.c
+++ b/drivers/staging/comedi/drivers/ni_pcidio.c
@@ -949,6 +949,19 @@ static int pci_6534_upload_firmware(struct comedi_device
*dev)
return ret;
}
+static void nidio_reset_board(struct comedi_device *dev)
+{
+ struct nidio96_private *devpriv = dev->private;
+ void __iomem *daq_mmio = devpriv->mite->daq_io_addr;
+
+ writel(0, daq_mmio + Port_IO(0));
+ writel(0, daq_mmio + Port_Pin_Directions(0));
+ writel(0, daq_mmio + Port_Pin_Mask(0));
+
+ /* disable interrupts on board */
+ writeb(0, daq_mmio + Master_DMA_And_Interrupt_Control);
+}
+
static int nidio_auto_attach(struct comedi_device *dev,
unsigned long context)
{
@@ -996,6 +1009,8 @@ static int nidio_auto_attach(struct comedi_device *dev,
return ret;
}
+ nidio_reset_board(dev);
+
ret = comedi_alloc_subdevices(dev, 1);
if (ret)
return ret;
@@ -1023,15 +1038,6 @@ static int nidio_auto_attach(struct comedi_device *dev,
s->async_dma_dir = DMA_BIDIRECTIONAL;
s->poll = &ni_pcidio_poll;
- writel(0, devpriv->mite->daq_io_addr + Port_IO(0));
- writel(0, devpriv->mite->daq_io_addr + Port_Pin_Directions(0));
- writel(0, devpriv->mite->daq_io_addr + Port_Pin_Mask(0));
-
- /* disable interrupts on board */
- writeb(0x00,
- devpriv->mite->daq_io_addr +
- Master_DMA_And_Interrupt_Control);
-
irq = mite_irq(devpriv->mite);
if (irq) {
ret = request_irq(irq, nidio_interrupt, IRQF_SHARED,
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: et131x: replace magic number bitmask with defined values
Signed-off-by: Mark Einon --- I noticed the obvious typo just after sending. Now fixed. drivers/staging/et131x/et131x.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c index 4d76d79..2f3c84a 100644 --- a/drivers/staging/et131x/et131x.c +++ b/drivers/staging/et131x/et131x.c @@ -4122,7 +4122,7 @@ static void et131x_isr_handler(struct work_struct *work) if (status & ET_INTR_RXDMA_XFR_DONE) et131x_handle_recv_interrupt(adapter); - status &= 0xffd7; + status &= ~(ET_INTR_TXDMA_ERR | ET_INTR_RXDMA_XFR_DONE); if (!status) goto out; -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: silicom: fix 'return is not a function, parentheses are not required' in bpctl_mod.c
On Fri, 2013-12-06 at 01:50 +0300, Dan Carpenter wrote: > On Thu, Dec 05, 2013 at 10:23:53PM +0100, Will Tange wrote: > > Fixes warnings regarding redundant parantheses thrown by the checkpatch > > tool in bpctl_mod.c [] > if (ret < 0) > return BP_NOT_CAP; > if (ret == 0) > return 1; > return 0; > > More lines, but simpler to understand than the original. > > Think of checkpatch.pl as a pointer to bad code and not that we just > have to silence checkpatch and move on. So true. If 0 is the expected ret value and 1 is the expected function return for not-errored use, I suggest changing the last bit to: if (ret < 0) return BP_NOT_CAP; else if (ret > 0) return 0; return 1; so that the error conditions are done first and the normal return is at the bottom of the function. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: silicom: fix 'return is not a function, parentheses are not required' in bpctl_mod.c
On Thu, Dec 05, 2013 at 03:09:15PM -0800, Joe Perches wrote: > On Fri, 2013-12-06 at 01:50 +0300, Dan Carpenter wrote: > > On Thu, Dec 05, 2013 at 10:23:53PM +0100, Will Tange wrote: > > > Fixes warnings regarding redundant parantheses thrown by the checkpatch > > > tool in bpctl_mod.c > [] > > if (ret < 0) > > return BP_NOT_CAP; > > if (ret == 0) > > return 1; > > return 0; > > > > More lines, but simpler to understand than the original. > > > > Think of checkpatch.pl as a pointer to bad code and not that we just > > have to silence checkpatch and move on. > > So true. > > If 0 is the expected ret value and 1 is the > expected function return for not-errored use, > I suggest changing the last bit to: > > if (ret < 0) > return BP_NOT_CAP; > else if (ret > 0) > return 0; > > return 1; > > so that the error conditions are done first > and the normal return is at the bottom of > the function. In this function, -1 means fail, 1 means "on" and 0 means "off". I sorted them from lowest to highest: negative, zero and greater than zero. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/13] staging: comedi: pcmuio: add inline helpers to get the 'iobase', 'asic', and 'port'
To reduce the potential for bugs, and better document the code, introduce
some inline helper functions to consolidate the calculations needed to get
the 'iobase' for a given asic and the 'asic' and 'port' associated with a
given subdevice.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 50 -
1 file changed, 37 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index 3dbc1d3..6ee876d 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -141,12 +141,36 @@ struct pcmuio_private {
unsigned int irq2;
};
+static inline unsigned long pcmuio_asic_iobase(struct comedi_device *dev,
+ int asic)
+{
+ return dev->iobase + (asic * PCMUIO_ASIC_IOSIZE);
+}
+
+static inline int pcmuio_subdevice_to_asic(struct comedi_subdevice *s)
+{
+ /*
+* subdevice 0 and 1 are handled by the first asic
+* subdevice 2 and 3 are handled by the second asic
+*/
+ return s->index / 2;
+}
+
+static inline int pcmuio_subdevice_to_port(struct comedi_subdevice *s)
+{
+ /*
+* subdevice 0 and 2 use port registers 0-2
+* subdevice 1 and 3 use port registers 3-5
+*/
+ return (s->index % 2) ? 3 : 0;
+}
+
static void pcmuio_write(struct comedi_device *dev, unsigned int val,
int asic, int page, int port)
{
struct pcmuio_private *devpriv = dev->private;
struct pcmuio_asic *chip = &devpriv->asics[asic];
- unsigned long iobase = dev->iobase + (asic * PCMUIO_ASIC_IOSIZE);
+ unsigned long iobase = pcmuio_asic_iobase(dev, asic);
unsigned long flags;
spin_lock_irqsave(&chip->pagelock, flags);
@@ -169,7 +193,7 @@ static unsigned int pcmuio_read(struct comedi_device *dev,
{
struct pcmuio_private *devpriv = dev->private;
struct pcmuio_asic *chip = &devpriv->asics[asic];
- unsigned long iobase = dev->iobase + (asic * PCMUIO_ASIC_IOSIZE);
+ unsigned long iobase = pcmuio_asic_iobase(dev, asic);
unsigned long flags;
unsigned int val;
@@ -205,9 +229,9 @@ static int pcmuio_dio_insn_bits(struct comedi_device *dev,
struct comedi_insn *insn,
unsigned int *data)
{
+ int asic = pcmuio_subdevice_to_asic(s);
+ int port = pcmuio_subdevice_to_port(s);
unsigned int chanmask = (1 << s->n_chan) - 1;
- int asic = s->index / 2;
- int port = (s->index % 2) ? 3 : 0;
unsigned int mask;
unsigned int val;
@@ -240,8 +264,8 @@ static int pcmuio_dio_insn_config(struct comedi_device *dev,
struct comedi_insn *insn,
unsigned int *data)
{
- int asic = s->index / 2;
- int port = (s->index % 2) ? 3 : 0;
+ int asic = pcmuio_subdevice_to_asic(s);
+ int port = pcmuio_subdevice_to_port(s);
int ret;
ret = comedi_dio_insn_config(dev, s, insn, data, 0);
@@ -275,7 +299,7 @@ static void pcmuio_stop_intr(struct comedi_device *dev,
struct comedi_subdevice *s)
{
struct pcmuio_private *devpriv = dev->private;
- int asic = s->index / 2;
+ int asic = pcmuio_subdevice_to_asic(s);
struct pcmuio_asic *chip = &devpriv->asics[asic];
chip->enabled_mask = 0;
@@ -291,7 +315,7 @@ static void pcmuio_handle_intr_subdev(struct comedi_device
*dev,
unsigned triggered)
{
struct pcmuio_private *devpriv = dev->private;
- int asic = s->index / 2;
+ int asic = pcmuio_subdevice_to_asic(s);
struct pcmuio_asic *chip = &devpriv->asics[asic];
unsigned int len = s->async->cmd.chanlist_len;
unsigned oldevents = s->async->events;
@@ -347,7 +371,7 @@ static int pcmuio_handle_asic_interrupt(struct
comedi_device *dev, int asic)
{
/* there are could be two asics so we can't use dev->read_subdev */
struct comedi_subdevice *s = &dev->subdevices[asic * 2];
- unsigned long iobase = dev->iobase + (asic * PCMUIO_ASIC_IOSIZE);
+ unsigned long iobase = pcmuio_asic_iobase(dev, asic);
unsigned int val;
/* are there any interrupts pending */
@@ -383,7 +407,7 @@ static int pcmuio_start_intr(struct comedi_device *dev,
struct comedi_subdevice *s)
{
struct pcmuio_private *devpriv = dev->private;
- int asic = s->index / 2;
+ int asic = pcmuio_subdevice_to_asic(s);
struct pcmuio_asic *chip = &devpriv->asics[asic];
if (!chip->continuous && chip->stop_count == 0) {
@@ -419,7 +443,7 @@ static int pcmuio_start_intr(struct comedi_device *dev,
static int pcmuio_cancel(struct comedi_device *dev, struct
[PATCH 12/13] staging: comedi: pcmuio: tidy up pcmuio_attach()
Clean up the local variables, 'sdev_no' and 'asic' are both used in
simple for () loops. Use the local variable 'i' for both cases. The
'n_subdevs' variable is only used in one place, just remove it.
For aesthetics, add some whitespace to the subdevice init and reorder
it to follow the more typical style in comedi drivers.
Remove the unnecessary init of s->len_chanlist for subdevices that do
not support async commands (interrupts). The core will default it to
the correct value.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 43 +++--
1 file changed, 19 insertions(+), 24 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index 32da92f..edcac26 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -591,8 +591,8 @@ static int pcmuio_attach(struct comedi_device *dev, struct
comedi_devconfig *it)
const struct pcmuio_board *board = comedi_board(dev);
struct comedi_subdevice *s;
struct pcmuio_private *devpriv;
- int sdev_no, n_subdevs, asic;
int ret;
+ int i;
ret = comedi_request_region(dev, it->options[0],
board->num_asics * PCMUIO_ASIC_IOSIZE);
@@ -603,8 +603,8 @@ static int pcmuio_attach(struct comedi_device *dev, struct
comedi_devconfig *it)
if (!devpriv)
return -ENOMEM;
- for (asic = 0; asic < PCMUIO_MAX_ASICS; ++asic) {
- struct pcmuio_asic *chip = &devpriv->asics[asic];
+ for (i = 0; i < PCMUIO_MAX_ASICS; ++i) {
+ struct pcmuio_asic *chip = &devpriv->asics[i];
spin_lock_init(&chip->pagelock);
spin_lock_init(&chip->spinlock);
@@ -633,34 +633,29 @@ static int pcmuio_attach(struct comedi_device *dev,
struct comedi_devconfig *it)
}
}
- n_subdevs = board->num_asics * 2;
-
- ret = comedi_alloc_subdevices(dev, n_subdevs);
+ ret = comedi_alloc_subdevices(dev, board->num_asics * 2);
if (ret)
return ret;
- for (sdev_no = 0; sdev_no < (int)dev->n_subdevices; ++sdev_no) {
- s = &dev->subdevices[sdev_no];
- s->maxdata = 1;
- s->range_table = &range_digital;
- s->subdev_flags = SDF_READABLE | SDF_WRITABLE;
- s->type = COMEDI_SUBD_DIO;
- s->insn_bits = pcmuio_dio_insn_bits;
- s->insn_config = pcmuio_dio_insn_config;
- s->n_chan = 24;
+ for (i = 0; i < dev->n_subdevices; ++i) {
+ s = &dev->subdevices[i];
+ s->type = COMEDI_SUBD_DIO;
+ s->subdev_flags = SDF_READABLE | SDF_WRITABLE;
+ s->n_chan = 24;
+ s->maxdata = 1;
+ s->range_table = &range_digital;
+ s->insn_bits= pcmuio_dio_insn_bits;
+ s->insn_config = pcmuio_dio_insn_config;
/* subdevices 0 and 2 can suppport interrupts */
- if ((sdev_no == 0 && dev->irq) ||
- (sdev_no == 2 && devpriv->irq2)) {
+ if ((i == 0 && dev->irq) || (i == 2 && devpriv->irq2)) {
/* setup the interrupt subdevice */
dev->read_subdev = s;
- s->subdev_flags |= SDF_CMD_READ;
- s->cancel = pcmuio_cancel;
- s->do_cmd = pcmuio_cmd;
- s->do_cmdtest = pcmuio_cmdtest;
- s->len_chanlist = s->n_chan;
- } else {
- s->len_chanlist = 1;
+ s->subdev_flags |= SDF_CMD_READ;
+ s->len_chanlist = s->n_chan;
+ s->cancel = pcmuio_cancel;
+ s->do_cmd = pcmuio_cmd;
+ s->do_cmdtest = pcmuio_cmdtest;
}
}
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 06/13] staging: comedi: pcmuio: fix pcmuio_dio_insn_bits()
This dio subdevice (*insn_bits) function does not follow the "norm"
for comedi drivers. It also _appears_ to return the incorrect state
of the channels.
Use the comedi_dio_update_state() helper to handle the boilerplate
for updating the output channel state. Due to the hardware we then
need to invert the state and mask the input channels before updating
the outputs.
Then read the hardware and invert the result to get the current true
state of the dio channels.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 35 +++--
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index fb0cbf0..2dca119 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -202,30 +202,35 @@ static unsigned int pcmuio_read(struct comedi_device *dev,
*/
static int pcmuio_dio_insn_bits(struct comedi_device *dev,
struct comedi_subdevice *s,
- struct comedi_insn *insn, unsigned int *data)
+ struct comedi_insn *insn,
+ unsigned int *data)
{
- unsigned int mask = data[0] & s->io_bits; /* outputs only */
- unsigned int bits = data[1];
+ unsigned int chanmask = (1 << s->n_chan) - 1;
int asic = s->index / 2;
int port = (s->index % 2) ? 3 : 0;
+ unsigned int mask;
unsigned int val;
- /* get inverted state of the channels from the port */
- val = pcmuio_read(dev, asic, 0, port);
-
- /* get the true state of the channels */
- s->state = val ^ ((0x1 << s->n_chan) - 1);
-
+ mask = comedi_dio_update_state(s, data);
if (mask) {
- s->state &= ~mask;
- s->state |= (mask & bits);
-
- /* invert the state and update the channels */
- val = s->state ^ ((0x1 << s->n_chan) - 1);
+ /*
+* Outputs are inverted, invert the state and
+* update the channels.
+*
+* The s->io_bits mask makes sure the input channels
+* are '0' so that the outputs pins stay in a high
+* z-state.
+*/
+ val = ~s->state & chanmask;
+ val &= s->io_bits;
pcmuio_write(dev, val, asic, 0, port);
}
- data[1] = s->state;
+ /* get inverted state of the channels from the port */
+ val = pcmuio_read(dev, asic, 0, port);
+
+ /* return the true state of the channels */
+ data[1] = ~val & chanmask;
return insn->n;
}
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/13] staging: comedi: pcmuio: remove subdevice private data
The subdevice private data is only needed for each 'asic' not for each
subdevice. Since the 'asic' can be calculated easily from the subdevice
we can merge the subdevice private data members directly into the
private data.
This removes the need to kcalloc/free the subdevice private data and
saves a bit of space.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 132
1 file changed, 65 insertions(+), 67 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index 57a52da..fb0cbf0 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -127,26 +127,17 @@ static const struct pcmuio_board pcmuio_boards[] = {
},
};
-struct pcmuio_subdev_private {
- /* The below is only used for intr subdevices */
- struct {
- /*
-* subdev-relative channel mask for channels
-* we are interested in
-*/
- int enabled_mask;
- int active;
- int stop_count;
- int continuous;
- spinlock_t spinlock;
- } intr;
+struct pcmuio_asic {
+ spinlock_t pagelock;
+ spinlock_t spinlock;
+ int enabled_mask;
+ int active;
+ int stop_count;
+ int continuous;
};
struct pcmuio_private {
- struct {
- spinlock_t pagelock;
- } asics[PCMUIO_MAX_ASICS];
- struct pcmuio_subdev_private *sprivs;
+ struct pcmuio_asic asics[PCMUIO_MAX_ASICS];
unsigned int irq2;
};
@@ -154,10 +145,11 @@ static void pcmuio_write(struct comedi_device *dev,
unsigned int val,
int asic, int page, int port)
{
struct pcmuio_private *devpriv = dev->private;
+ struct pcmuio_asic *chip = &devpriv->asics[asic];
unsigned long iobase = dev->iobase + (asic * PCMUIO_ASIC_IOSIZE);
unsigned long flags;
- spin_lock_irqsave(&devpriv->asics[asic].pagelock, flags);
+ spin_lock_irqsave(&chip->pagelock, flags);
if (page == 0) {
/* Port registers are valid for any page */
outb(val & 0xff, iobase + PCMUIO_PORT_REG(port + 0));
@@ -169,18 +161,19 @@ static void pcmuio_write(struct comedi_device *dev,
unsigned int val,
outb((val >> 8) & 0xff, iobase + PCMUIO_PAGE_REG(1));
outb((val >> 16) & 0xff, iobase + PCMUIO_PAGE_REG(2));
}
- spin_unlock_irqrestore(&devpriv->asics[asic].pagelock, flags);
+ spin_unlock_irqrestore(&chip->pagelock, flags);
}
static unsigned int pcmuio_read(struct comedi_device *dev,
int asic, int page, int port)
{
struct pcmuio_private *devpriv = dev->private;
+ struct pcmuio_asic *chip = &devpriv->asics[asic];
unsigned long iobase = dev->iobase + (asic * PCMUIO_ASIC_IOSIZE);
unsigned long flags;
unsigned int val;
- spin_lock_irqsave(&devpriv->asics[asic].pagelock, flags);
+ spin_lock_irqsave(&chip->pagelock, flags);
if (page == 0) {
/* Port registers are valid for any page */
val = inb(iobase + PCMUIO_PORT_REG(port + 0));
@@ -192,7 +185,7 @@ static unsigned int pcmuio_read(struct comedi_device *dev,
val |= (inb(iobase + PCMUIO_PAGE_REG(1)) << 8);
val |= (inb(iobase + PCMUIO_PAGE_REG(2)) << 16);
}
- spin_unlock_irqrestore(&devpriv->asics[asic].pagelock, flags);
+ spin_unlock_irqrestore(&chip->pagelock, flags);
return val;
}
@@ -276,11 +269,12 @@ static void pcmuio_reset(struct comedi_device *dev)
static void pcmuio_stop_intr(struct comedi_device *dev,
struct comedi_subdevice *s)
{
- struct pcmuio_subdev_private *subpriv = s->private;
+ struct pcmuio_private *devpriv = dev->private;
int asic = s->index / 2;
+ struct pcmuio_asic *chip = &devpriv->asics[asic];
- subpriv->intr.enabled_mask = 0;
- subpriv->intr.active = 0;
+ chip->enabled_mask = 0;
+ chip->active = 0;
s->async->inttrig = NULL;
/* disable all intrs for this subdev.. */
@@ -291,7 +285,9 @@ static void pcmuio_handle_intr_subdev(struct comedi_device
*dev,
struct comedi_subdevice *s,
unsigned triggered)
{
- struct pcmuio_subdev_private *subpriv = s->private;
+ struct pcmuio_private *devpriv = dev->private;
+ int asic = s->index / 2;
+ struct pcmuio_asic *chip = &devpriv->asics[asic];
unsigned int len = s->async->cmd.chanlist_len;
unsigned oldevents = s->async->events;
unsigned int val = 0;
@@ -299,15 +295,15 @@ static void pcmuio_handle_intr_subdev(struct
comedi_device *dev,
unsigned mytrig;
uns
[PATCH 02/13] staging: comedi: pcmuio: spinlock protect pcmuio_{write, read}()
Currently only the pcmuio_handle_asic_interrupt() function uses the
spinlock in the private data to protect the read of the paged interrupt
id registers. All accesses to the paged registers should be protected
to ensure that the page is not changed until the access is complete.
Move the lock/unlock into the pcmuio_{write,read}() functions to make
sure the access completes correctly. Rename the spinlock to variable
to clarify its use.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index dfa7280..f7032fe 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -146,7 +146,7 @@ struct pcmuio_subdev_private {
struct pcmuio_private {
struct {
- spinlock_t spinlock;
+ spinlock_t pagelock;
} asics[PCMUIO_MAX_ASICS];
struct pcmuio_subdev_private *sprivs;
unsigned int irq2;
@@ -155,8 +155,11 @@ struct pcmuio_private {
static void pcmuio_write(struct comedi_device *dev, unsigned int val,
int asic, int page, int port)
{
+ struct pcmuio_private *devpriv = dev->private;
unsigned long iobase = dev->iobase + (asic * PCMUIO_ASIC_IOSIZE);
+ unsigned long flags;
+ spin_lock_irqsave(&devpriv->asics[asic].pagelock, flags);
if (page == 0) {
/* Port registers are valid for any page */
outb(val & 0xff, iobase + PCMUIO_PORT_REG(port + 0));
@@ -168,14 +171,18 @@ static void pcmuio_write(struct comedi_device *dev,
unsigned int val,
outb((val >> 8) & 0xff, iobase + PCMUIO_PAGE_REG(1));
outb((val >> 16) & 0xff, iobase + PCMUIO_PAGE_REG(2));
}
+ spin_unlock_irqrestore(&devpriv->asics[asic].pagelock, flags);
}
static unsigned int pcmuio_read(struct comedi_device *dev,
int asic, int page, int port)
{
+ struct pcmuio_private *devpriv = dev->private;
unsigned long iobase = dev->iobase + (asic * PCMUIO_ASIC_IOSIZE);
+ unsigned long flags;
unsigned int val;
+ spin_lock_irqsave(&devpriv->asics[asic].pagelock, flags);
if (page == 0) {
/* Port registers are valid for any page */
val = inb(iobase + PCMUIO_PORT_REG(port + 0));
@@ -187,6 +194,7 @@ static unsigned int pcmuio_read(struct comedi_device *dev,
val |= (inb(iobase + PCMUIO_PAGE_REG(1)) << 8);
val |= (inb(iobase + PCMUIO_PAGE_REG(2)) << 16);
}
+ spin_unlock_irqrestore(&devpriv->asics[asic].pagelock, flags);
return val;
}
@@ -346,17 +354,13 @@ done:
static int pcmuio_handle_asic_interrupt(struct comedi_device *dev, int asic)
{
- struct pcmuio_private *devpriv = dev->private;
struct pcmuio_subdev_private *subpriv;
unsigned long iobase = dev->iobase + (asic * PCMUIO_ASIC_IOSIZE);
unsigned int triggered = 0;
int got1 = 0;
- unsigned long flags;
unsigned char int_pend;
int i;
- spin_lock_irqsave(&devpriv->asics[asic].spinlock, flags);
-
int_pend = inb(iobase + PCMUIO_INT_PENDING_REG) & 0x07;
if (int_pend) {
triggered = pcmuio_read(dev, asic, PCMUIO_PAGE_INT_ID, 0);
@@ -365,8 +369,6 @@ static int pcmuio_handle_asic_interrupt(struct
comedi_device *dev, int asic)
++got1;
}
- spin_unlock_irqrestore(&devpriv->asics[asic].spinlock, flags);
-
if (triggered) {
struct comedi_subdevice *s;
/* TODO here: dispatch io lines to subdevs with commands.. */
@@ -598,7 +600,7 @@ static int pcmuio_attach(struct comedi_device *dev, struct
comedi_devconfig *it)
return -ENOMEM;
for (asic = 0; asic < PCMUIO_MAX_ASICS; ++asic)
- spin_lock_init(&devpriv->asics[asic].spinlock);
+ spin_lock_init(&devpriv->asics[asic].pagelock);
pcmuio_reset(dev);
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 10/13] staging: comedi: pcmuio: fix types of some private data variables
The 'enabled_mask' is a bit mask of the channels that are enabled
for interrupt detection and should be an unsigned int.
The 'stop_count' is a >= 0 value that is set by the unsigned int
cmd->stop_arg. Make it an unsigned int.
The 'active' and 'continuous' members are flags. Make them unsigned
int bit-fields to save a bit of space.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index c83f5bd..b86eb1e 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -130,10 +130,10 @@ static const struct pcmuio_board pcmuio_boards[] = {
struct pcmuio_asic {
spinlock_t pagelock;/* protects the page registers */
spinlock_t spinlock;/* protects member variables */
- int enabled_mask;
- int active;
- int stop_count;
- int continuous;
+ unsigned int enabled_mask;
+ unsigned int stop_count;
+ unsigned int active:1;
+ unsigned int continuous:1;
};
struct pcmuio_private {
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/13] staging: comedi: pcmuio: cleanup driver
I orignally posted this series on 07/24/2013. At that time Ian Abbott had
some comments on it. I finally got around to addressing them.
This series cleans up the interrupt support and private data usage. It
also does a bit of cleanup to clarify the driver.
H Hartley Sweeten (13):
staging: comedi: pcmuio: fix interrupt requests
staging: comedi: pcmuio: spinlock protect pcmuio_{write,read}()
staging: comedi: pcmuio: tidy up pcmuio_handle_asic_interrupt()
staging: comedi: pcmuio: remove 'asic' member from subdevice private data
staging: comedi: pcmuio: remove subdevice private data
staging: comedi: pcmuio: fix pcmuio_dio_insn_bits()
staging: comedi: pcmuio: remove unnecessary mask of triggered channels
staging: comedi: pcmuio: add inline helpers to get the 'iobase', 'asic', and
'port'
staging: comedi: pcmuio: document the spinlock_t variables
staging: comedi: pcmuio: fix types of some private data variables
staging: comedi: pcmuio: remove unneeded include
staging: comedi: pcmuio: tidy up pcmuio_attach()
staging: comedi: pcmuio: tidy up pcmuio_start_intr()
drivers/staging/comedi/drivers/pcmuio.c | 428
1 file changed, 213 insertions(+), 215 deletions(-)
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 13/13] staging: comedi: pcmuio: tidy up pcmuio_start_intr()
Refactor this function a bit to remove the need for an extra indent
level and cleanup some of the odd line breaks.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 44 ++---
1 file changed, 24 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index edcac26..a8f390f 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -410,34 +410,38 @@ static int pcmuio_start_intr(struct comedi_device *dev,
struct pcmuio_private *devpriv = dev->private;
int asic = pcmuio_subdevice_to_asic(s);
struct pcmuio_asic *chip = &devpriv->asics[asic];
+ struct comedi_cmd *cmd = &s->async->cmd;
+ unsigned int bits = 0;
+ unsigned int pol_bits = 0;
+ int i;
if (!chip->continuous && chip->stop_count == 0) {
/* An empty acquisition! */
s->async->events |= COMEDI_CB_EOA;
chip->active = 0;
return 1;
- } else {
- struct comedi_cmd *cmd = &s->async->cmd;
- unsigned bits = 0, pol_bits = 0, n;
-
- chip->enabled_mask = 0;
- chip->active = 1;
- if (cmd->chanlist) {
- for (n = 0; n < cmd->chanlist_len; n++) {
- bits |= (1U << CR_CHAN(cmd->chanlist[n]));
- pol_bits |= (CR_AREF(cmd->chanlist[n])
-|| CR_RANGE(cmd->
-chanlist[n]) ? 1U : 0U)
- << CR_CHAN(cmd->chanlist[n]);
- }
- }
- bits &= ((0x1 << s->n_chan) - 1);
- chip->enabled_mask = bits;
+ }
- /* set pol and enab intrs for this subdev.. */
- pcmuio_write(dev, pol_bits, asic, PCMUIO_PAGE_POL, 0);
- pcmuio_write(dev, bits, asic, PCMUIO_PAGE_ENAB, 0);
+ chip->enabled_mask = 0;
+ chip->active = 1;
+ if (cmd->chanlist) {
+ for (i = 0; i < cmd->chanlist_len; i++) {
+ unsigned int chanspec = cmd->chanlist[i];
+ unsigned int chan = CR_CHAN(chanspec);
+ unsigned int range = CR_RANGE(chanspec);
+ unsigned int aref = CR_AREF(chanspec);
+
+ bits |= (1 << chan);
+ pol_bits |= ((aref || range) ? 1 : 0) << chan;
+ }
}
+ bits &= ((1 << s->n_chan) - 1);
+ chip->enabled_mask = bits;
+
+ /* set pol and enab intrs for this subdev.. */
+ pcmuio_write(dev, pol_bits, asic, PCMUIO_PAGE_POL, 0);
+ pcmuio_write(dev, bits, asic, PCMUIO_PAGE_ENAB, 0);
+
return 0;
}
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/13] staging: comedi: pcmuio: tidy up pcmuio_handle_asic_interrupt()
Unfortunatly, since there could be two asics, we can't use dev->read_subdev
to get the subdevice. But, the comedi_subdevice associated with the 'asic'
can easily be calculated. This allows removing the for () loop that searched
for the correct subdevice.
Tidy up the function.
---
drivers/staging/comedi/drivers/pcmuio.c | 44 +++--
1 file changed, 15 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index f7032fe..8752d4d 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -354,38 +354,24 @@ done:
static int pcmuio_handle_asic_interrupt(struct comedi_device *dev, int asic)
{
- struct pcmuio_subdev_private *subpriv;
+ /* there are could be two asics so we can't use dev->read_subdev */
+ struct comedi_subdevice *s = &dev->subdevices[asic * 2];
unsigned long iobase = dev->iobase + (asic * PCMUIO_ASIC_IOSIZE);
- unsigned int triggered = 0;
- int got1 = 0;
- unsigned char int_pend;
- int i;
-
- int_pend = inb(iobase + PCMUIO_INT_PENDING_REG) & 0x07;
- if (int_pend) {
- triggered = pcmuio_read(dev, asic, PCMUIO_PAGE_INT_ID, 0);
- pcmuio_write(dev, 0, asic, PCMUIO_PAGE_INT_ID, 0);
+ unsigned int val;
- ++got1;
- }
+ /* are there any interrupts pending */
+ val = inb(iobase + PCMUIO_INT_PENDING_REG) & 0x07;
+ if (!val)
+ return 0;
- if (triggered) {
- struct comedi_subdevice *s;
- /* TODO here: dispatch io lines to subdevs with commands.. */
- for (i = 0; i < dev->n_subdevices; i++) {
- s = &dev->subdevices[i];
- subpriv = s->private;
- if (subpriv->intr.asic == asic) {
- /*
-* This is an interrupt subdev, and it
-* matches this asic!
-*/
- pcmuio_handle_intr_subdev(dev, s,
- triggered);
- }
- }
- }
- return got1;
+ /* get, and clear, the pending interrupts */
+ val = pcmuio_read(dev, asic, PCMUIO_PAGE_INT_ID, 0);
+ pcmuio_write(dev, 0, asic, PCMUIO_PAGE_INT_ID, 0);
+
+ /* handle the pending interrupts */
+ pcmuio_handle_intr_subdev(dev, s, val);
+
+ return 1;
}
static irqreturn_t pcmuio_interrupt(int irq, void *d)
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/13] staging: comedi: pcmuio: remove 'asic' member from subdevice private data
The 'asic' associated with a subdevice can be easily calculated. The
functions that use this member in the subdevice private data can only
be called by the subdevices that support interrupts. Just calculate
the 'asic' when needed and remove the member variable and sanity checks.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 18 +++---
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index 8752d4d..57a52da 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -130,8 +130,6 @@ static const struct pcmuio_board pcmuio_boards[] = {
struct pcmuio_subdev_private {
/* The below is only used for intr subdevices */
struct {
- /* if non-negative, this subdev has an interrupt asic */
- int asic;
/*
* subdev-relative channel mask for channels
* we are interested in
@@ -279,11 +277,7 @@ static void pcmuio_stop_intr(struct comedi_device *dev,
struct comedi_subdevice *s)
{
struct pcmuio_subdev_private *subpriv = s->private;
- int asic;
-
- asic = subpriv->intr.asic;
- if (asic < 0)
- return; /* not an interrupt subdev */
+ int asic = s->index / 2;
subpriv->intr.enabled_mask = 0;
subpriv->intr.active = 0;
@@ -399,14 +393,10 @@ static int pcmuio_start_intr(struct comedi_device *dev,
subpriv->intr.active = 0;
return 1;
} else {
- unsigned bits = 0, pol_bits = 0, n;
- int asic;
struct comedi_cmd *cmd = &s->async->cmd;
+ int asic = s->index / 2;
+ unsigned bits = 0, pol_bits = 0, n;
- asic = subpriv->intr.asic;
- if (asic < 0)
- return 1; /* not an interrupt
- subdev */
subpriv->intr.enabled_mask = 0;
subpriv->intr.active = 1;
if (cmd->chanlist) {
@@ -636,7 +626,6 @@ static int pcmuio_attach(struct comedi_device *dev, struct
comedi_devconfig *it)
if ((sdev_no == 0 && dev->irq) ||
(sdev_no == 2 && devpriv->irq2)) {
/* setup the interrupt subdevice */
- subpriv->intr.asic = sdev_no / 2;
dev->read_subdev = s;
s->subdev_flags |= SDF_CMD_READ;
s->cancel = pcmuio_cancel;
@@ -644,7 +633,6 @@ static int pcmuio_attach(struct comedi_device *dev, struct
comedi_devconfig *it)
s->do_cmdtest = pcmuio_cmdtest;
s->len_chanlist = s->n_chan;
} else {
- subpriv->intr.asic = -1;
s->len_chanlist = 1;
}
spin_lock_init(&subpriv->intr.spinlock);
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/13] staging: comedi: pcmuio: fix interrupt requests
Legacy (ISA) interrupts are not sharable so this driver should not
be passing the IRQF_SHARED flag when requesting the interrupts.
This driver supports two board types:
PCM-UIO48 with one asic (one interrupt source)
PCM-UIO96 with two asics (two interrupt sources)
The PCM-UIO96 has a jumper that allows the two interrupt sources to
share an interrupt. This is safe for legacy interrupts as long as
the "shared" interrupt is handled by a single driver.
Modify the request_irq() code in this driver to correctly request the
interrupts. For the PCM-UI048 case (one asic) only one request_irq()
is needed. For the PCM-UIO96 (two asics) there are three cases:
1) irq is shared, one request_irq() call
2) only one asic has an irq, one request_irq() call
3) two irqs, two request_irq() calls
The irq for the first asic (dev->irq) will be requested during the
attach if required. The comedi core will handle the freeing of this
irq during the detach.
The irq for the second asic (devpriv->irq2) will also be requested
during the attach if required. The freeing of this irq will be
handled by the driver during the detach.
Move the board reset and interrupt request code so it occurs early
in the attach. We can then check dev->irq and devpriv->irq2 to see
if the subdevice command support actually needs to be initialized.
This also simplifies the interrupt handler. The irq can be simply
checked against dev->irq and devpriv->irq2 to see which asic caused
the interrupt.
Add a call to pcmuio_reset() in the (*detach) to make sure the
interrupts are disabled before freeing the irqs.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 81 -
1 file changed, 40 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index 954fa96..dfa7280 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -146,10 +146,10 @@ struct pcmuio_subdev_private {
struct pcmuio_private {
struct {
- unsigned int irq;
spinlock_t spinlock;
} asics[PCMUIO_MAX_ASICS];
struct pcmuio_subdev_private *sprivs;
+ unsigned int irq2;
};
static void pcmuio_write(struct comedi_device *dev, unsigned int val,
@@ -390,19 +390,14 @@ static irqreturn_t pcmuio_interrupt(int irq, void *d)
{
struct comedi_device *dev = d;
struct pcmuio_private *devpriv = dev->private;
- int got1 = 0;
- int asic;
+ int handled = 0;
- for (asic = 0; asic < PCMUIO_MAX_ASICS; ++asic) {
- if (irq == devpriv->asics[asic].irq) {
- /* it is an interrupt for ASIC #asic */
- if (pcmuio_handle_asic_interrupt(dev, asic))
- got1++;
- }
- }
- if (!got1)
- return IRQ_NONE;/* interrupt from other source */
- return IRQ_HANDLED;
+ if (irq == dev->irq)
+ handled += pcmuio_handle_asic_interrupt(dev, 0);
+ if (irq == devpriv->irq2)
+ handled += pcmuio_handle_asic_interrupt(dev, 1);
+
+ return handled ? IRQ_HANDLED : IRQ_NONE;
}
static int pcmuio_start_intr(struct comedi_device *dev,
@@ -591,12 +586,8 @@ static int pcmuio_attach(struct comedi_device *dev, struct
comedi_devconfig *it)
struct pcmuio_private *devpriv;
struct pcmuio_subdev_private *subpriv;
int sdev_no, n_subdevs, asic;
- unsigned int irq[PCMUIO_MAX_ASICS];
int ret;
- irq[0] = it->options[1];
- irq[1] = it->options[2];
-
ret = comedi_request_region(dev, it->options[0],
board->num_asics * PCMUIO_ASIC_IOSIZE);
if (ret)
@@ -609,6 +600,29 @@ static int pcmuio_attach(struct comedi_device *dev, struct
comedi_devconfig *it)
for (asic = 0; asic < PCMUIO_MAX_ASICS; ++asic)
spin_lock_init(&devpriv->asics[asic].spinlock);
+ pcmuio_reset(dev);
+
+ if (it->options[1]) {
+ /* request the irq for the 1st asic */
+ ret = request_irq(it->options[1], pcmuio_interrupt, 0,
+ dev->board_name, dev);
+ if (ret == 0)
+ dev->irq = it->options[1];
+ }
+
+ if (board->num_asics == 2) {
+ if (it->options[2] == dev->irq) {
+ /* the same irq (or none) is used by both asics */
+ devpriv->irq2 = it->options[2];
+ } else if (it->options[2]) {
+ /* request the irq for the 2nd asic */
+ ret = request_irq(it->options[2], pcmuio_interrupt, 0,
+ dev->board_name, dev);
+ if (ret == 0)
+ devpriv->irq2 = it->options[2];
+
[PATCH 10/13] staging: comedi: pcmuio: fix types of some private data variables
The 'enabled_mask' is a bit mask of the channels that are enabled
for interrupt detection and should be an unsigned int.
The 'stop_count' is a >= 0 value that is set by the unsigned int
cmd->stop_arg. Make it an unsigned int.
The 'active' and 'continuous' members are flags. Make them unsigned
int bit-fields to save a bit of space.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index c83f5bd..b86eb1e 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -130,10 +130,10 @@ static const struct pcmuio_board pcmuio_boards[] = {
struct pcmuio_asic {
spinlock_t pagelock;/* protects the page registers */
spinlock_t spinlock;/* protects member variables */
- int enabled_mask;
- int active;
- int stop_count;
- int continuous;
+ unsigned int enabled_mask;
+ unsigned int stop_count;
+ unsigned int active:1;
+ unsigned int continuous:1;
};
struct pcmuio_private {
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/13] staging: comedi: pcmuio: document the spinlock_t variables
Add some comments about the two spinlock_t variables in the private
data. Also, add come comments for the functions that do not need to
lock/unlock the spinlock.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index 6ee876d..c83f5bd 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -128,8 +128,8 @@ static const struct pcmuio_board pcmuio_boards[] = {
};
struct pcmuio_asic {
- spinlock_t pagelock;
- spinlock_t spinlock;
+ spinlock_t pagelock;/* protects the page registers */
+ spinlock_t spinlock;/* protects member variables */
int enabled_mask;
int active;
int stop_count;
@@ -295,6 +295,7 @@ static void pcmuio_reset(struct comedi_device *dev)
}
}
+/* chip->spinlock is already locked */
static void pcmuio_stop_intr(struct comedi_device *dev,
struct comedi_subdevice *s)
{
@@ -403,6 +404,7 @@ static irqreturn_t pcmuio_interrupt(int irq, void *d)
return handled ? IRQ_HANDLED : IRQ_NONE;
}
+/* chip->spinlock is already locked */
static int pcmuio_start_intr(struct comedi_device *dev,
struct comedi_subdevice *s)
{
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 10/13] staging: comedi: pcmuio: fix types of some private data variables
On Thursday, December 05, 2013 4:54 PM, H Hartley Sweeten wrote: > Subject: [PATCH 10/13] staging: comedi: pcmuio: fix types of some private > data variables Oops.. This one got posted twice. Sorry about that. Hartley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: silicom: fix 'return is not a function, parentheses are not required' in bpctl_mod.c
On Fri, 2013-12-06 at 02:21 +0300, Dan Carpenter wrote: > On Thu, Dec 05, 2013 at 03:09:15PM -0800, Joe Perches wrote: > > On Fri, 2013-12-06 at 01:50 +0300, Dan Carpenter wrote: > > > On Thu, Dec 05, 2013 at 10:23:53PM +0100, Will Tange wrote: > > > > Fixes warnings regarding redundant parantheses thrown by the checkpatch > > > > tool in bpctl_mod.c > > [] > > > if (ret < 0) > > > return BP_NOT_CAP; > > > if (ret == 0) > > > return 1; > > > return 0; > > > > > > More lines, but simpler to understand than the original. > > > > > > Think of checkpatch.pl as a pointer to bad code and not that we just > > > have to silence checkpatch and move on. > > > > So true. > > > > If 0 is the expected ret value and 1 is the > > expected function return for not-errored use, > > I suggest changing the last bit to: > > > > if (ret < 0) > > return BP_NOT_CAP; > > else if (ret > 0) > > return 0; > > > > return 1; > > > > so that the error conditions are done first > > and the normal return is at the bottom of > > the function. > > In this function, -1 means fail, 1 means "on" and 0 means "off". I > sorted them from lowest to highest: negative, zero and greater than > zero. Ah. Then maybe use a single ?: or a ! instead return ret ? 0 : 1; or return !ret; cheers, Joe ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 28/39] staging: remove DEFINE_PCI_DEVICE_TABLE macro
On Thu, Dec 5, 2013 at 11:43 PM, Greg Kroah-Hartman wrote: > On Thu, Dec 05, 2013 at 05:06:33PM +0800, ZHAO Gang wrote: >> On Tue, Dec 3, 2013 at 7:26 AM, Jingoo Han wrote: >> > Don't use DEFINE_PCI_DEVICE_TABLE macro, because this macro >> > is not preferred. >> > >> > Signed-off-by: Jingoo Han >> > >> >> I think you misunderstood the checkpatch.pl warning, it tells you what >> to do, not what not to do. >> >> WARNING: Use DEFINE_PCI_DEVICE_TABLE for struct pci_device_id >> >> This means use DEFINE_PCI_DEVICE_TABLE to replace struct >> pci_device_id, not reverse. > > No, checkpatch is wrong, and is being fixed, this patch is correct. Oh, I didn't notice the recent checkpatch update, sorry for the noise. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 11/13] staging: comedi: pcmuio: remove unneeded include
The header is no longer needed by this driver. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/pcmuio.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/pcmuio.c b/drivers/staging/comedi/drivers/pcmuio.c index b86eb1e..32da92f 100644 --- a/drivers/staging/comedi/drivers/pcmuio.c +++ b/drivers/staging/comedi/drivers/pcmuio.c @@ -75,7 +75,6 @@ #include #include -#include #include "../comedidev.h" -- 1.8.4.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/13] staging: comedi: pcmuio: remove unnecessary mask of triggered channels
The 'triggered' value is read directly from the three trigger id
registers and does not have any extra data that needs masked off.
Remove the 'mytrig' local variable and just use 'triggered' directly.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/pcmuio.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c
b/drivers/staging/comedi/drivers/pcmuio.c
index 2dca119..3dbc1d3 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -297,7 +297,6 @@ static void pcmuio_handle_intr_subdev(struct comedi_device
*dev,
unsigned oldevents = s->async->events;
unsigned int val = 0;
unsigned long flags;
- unsigned mytrig;
unsigned int i;
spin_lock_irqsave(&chip->spinlock, flags);
@@ -305,16 +304,13 @@ static void pcmuio_handle_intr_subdev(struct
comedi_device *dev,
if (!chip->active)
goto done;
- mytrig = triggered;
- mytrig &= ((0x1 << s->n_chan) - 1);
-
- if (!(mytrig & chip->enabled_mask))
+ if (!(triggered & chip->enabled_mask))
goto done;
for (i = 0; i < len; i++) {
unsigned int chan = CR_CHAN(s->async->cmd.chanlist[i]);
- if (mytrig & (1U << chan))
- val |= (1U << i);
+ if (triggered & (1 << chan))
+ val |= (1 << i);
}
/* Write the scan to the buffer. */
--
1.8.4.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH -next] staging: slicoss: Remove last reference to compare_ether_addr
And use the normal is__ether_addr functions and ETH_ALEN too.
Signed-off-by: Joe Perches
---
drivers/staging/slicoss/README| 1 -
drivers/staging/slicoss/slicoss.c | 21 -
2 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/slicoss/README b/drivers/staging/slicoss/README
index cb04a87..53052c4 100644
--- a/drivers/staging/slicoss/README
+++ b/drivers/staging/slicoss/README
@@ -14,7 +14,6 @@ TODO:
- use net_device_ops
- use dev->stats rather than adapter->stats
- don't cast netdev_priv it is already void
- - use compare_ether_addr
- GET RID OF MACROS
- work on all architectures
- without CONFIG_X86_64 confusion
diff --git a/drivers/staging/slicoss/slicoss.c
b/drivers/staging/slicoss/slicoss.c
index f340cb9..1426ca4 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -595,15 +595,12 @@ static void slic_adapter_set_hwaddr(struct adapter
*adapter)
memcpy(adapter->macaddr,
card->config.MacInfo[adapter->functionnumber].macaddrA,
sizeof(struct slic_config_mac));
- if (!(adapter->currmacaddr[0] || adapter->currmacaddr[1] ||
- adapter->currmacaddr[2] || adapter->currmacaddr[3] ||
- adapter->currmacaddr[4] || adapter->currmacaddr[5])) {
- memcpy(adapter->currmacaddr, adapter->macaddr, 6);
- }
- if (adapter->netdev) {
+ if (is_zero_ether_addr(adapter->currmacaddr))
+ memcpy(adapter->currmacaddr, adapter->macaddr,
+ ETH_ALEN);
+ if (adapter->netdev)
memcpy(adapter->netdev->dev_addr, adapter->currmacaddr,
- 6);
- }
+ ETH_ALEN);
}
}
@@ -767,13 +764,11 @@ static bool slic_mac_filter(struct adapter *adapter,
{
struct net_device *netdev = adapter->netdev;
u32 opts = adapter->macopts;
- u32 *dhost4 = (u32 *)ðer_frame->ether_dhost[0];
- u16 *dhost2 = (u16 *)ðer_frame->ether_dhost[4];
if (opts & MAC_PROMISC)
return true;
- if ((*dhost4 == 0x) && (*dhost2 == 0x)) {
+ if (is_broadcast_ether_addr(ether_frame->ether_dhost)) {
if (opts & MAC_BCAST) {
adapter->rcv_broadcasts++;
return true;
@@ -782,7 +777,7 @@ static bool slic_mac_filter(struct adapter *adapter,
}
}
- if (ether_frame->ether_dhost[0] & 0x01) {
+ if (is_multicast_ether_addr(ether_frame->ether_dhost)) {
if (opts & MAC_ALLMCAST) {
adapter->rcv_multicasts++;
netdev->stats.multicast++;
@@ -2335,7 +2330,7 @@ static int slic_mcast_add_list(struct adapter *adapter,
char *address)
if (mcaddr == NULL)
return 1;
- memcpy(mcaddr->address, address, 6);
+ memcpy(mcaddr->address, address, ETH_ALEN);
mcaddr->next = adapter->mcastaddrs;
adapter->mcastaddrs = mcaddr;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper
Hi Greg, On 01.12.2013 19:07, Ivaylo DImitrov wrote: From: Ivaylo Dimitrov Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't need to be exported. It can also be made way simpler by using sscanf. Signed-off-by: Ivaylo Dimitrov --- drivers/staging/tidspbridge/Makefile |2 +- drivers/staging/tidspbridge/gen/uuidutil.c | 85 .../tidspbridge/include/dspbridge/uuidutil.h | 18 drivers/staging/tidspbridge/rmgr/dbdcd.c | 42 +- 4 files changed, 39 insertions(+), 108 deletions(-) delete mode 100644 drivers/staging/tidspbridge/gen/uuidutil.c I guess the initial mail somehow didn't make it through your spam filter: https://lkml.org/lkml/2013/12/1/70 Regards, Ivo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: silicom: fix 'return is not a function, parentheses are not required' in bpctl_mod.c
On Thu, Dec 05, 2013 at 03:29:22PM -0800, Joe Perches wrote: > On Fri, 2013-12-06 at 02:21 +0300, Dan Carpenter wrote: > > On Thu, Dec 05, 2013 at 03:09:15PM -0800, Joe Perches wrote: > > > On Fri, 2013-12-06 at 01:50 +0300, Dan Carpenter wrote: > > > > On Thu, Dec 05, 2013 at 10:23:53PM +0100, Will Tange wrote: > > Ah. > > Then maybe use a single ?: or a ! instead > > return ret ? 0 : 1; > or > return !ret; > You still have to handle the negative. if (ret < 0) return ret; return !ret; regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: silicom: fix 'return is not a function, parentheses are not required' in bpctl_mod.c
On Fri, 2013-12-06 at 10:11 +0300, Dan Carpenter wrote: > On Thu, Dec 05, 2013 at 03:29:22PM -0800, Joe Perches wrote: > > On Fri, 2013-12-06 at 02:21 +0300, Dan Carpenter wrote: > > > On Thu, Dec 05, 2013 at 03:09:15PM -0800, Joe Perches wrote: > > > > On Fri, 2013-12-06 at 01:50 +0300, Dan Carpenter wrote: > > > > > On Thu, Dec 05, 2013 at 10:23:53PM +0100, Will Tange wrote: > > > > Ah. > > > > Then maybe use a single ?: or a ! instead > > > > return ret ? 0 : 1; > > or > > return !ret; > > > > You still have to handle the negative. > > if (ret < 0) > return ret; > > return !ret; Of course. Apologies for not being clear. I meant using that ?: or ! after the first if (ret < 0) as you wrote. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper
On Fri, Dec 06, 2013 at 08:05:38AM +0200, Ivajlo Dimitrov wrote: > Hi Greg, > > On 01.12.2013 19:07, Ivaylo DImitrov wrote: > >From: Ivaylo Dimitrov > > > >Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't > >need to be exported. It can also be made way simpler by using sscanf. > > > >Signed-off-by: Ivaylo Dimitrov > >--- > > drivers/staging/tidspbridge/Makefile |2 +- > > drivers/staging/tidspbridge/gen/uuidutil.c | 85 > > > > .../tidspbridge/include/dspbridge/uuidutil.h | 18 > > drivers/staging/tidspbridge/rmgr/dbdcd.c | 42 +- > > 4 files changed, 39 insertions(+), 108 deletions(-) > > delete mode 100644 drivers/staging/tidspbridge/gen/uuidutil.c > > > > I guess the initial mail somehow didn't make it through your spam filter: > https://lkml.org/lkml/2013/12/1/70 > It's too early to start resending. Wait for another week at least. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper
My other patch (the one that fixes the security issue) was already applied, albeit it was sent after this one, see https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/commit/?h=staging-linus&id=559c71fe5dc3bf2ecc55afb336145db7f0abf810 , that is why I think there is some problem with the mail itself. Sure I will wait :) regards, Ivo On 06.12.2013 09:32, Dan Carpenter wrote: On Fri, Dec 06, 2013 at 08:05:38AM +0200, Ivajlo Dimitrov wrote: Hi Greg, On 01.12.2013 19:07, Ivaylo DImitrov wrote: From: Ivaylo Dimitrov Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't need to be exported. It can also be made way simpler by using sscanf. Signed-off-by: Ivaylo Dimitrov --- drivers/staging/tidspbridge/Makefile |2 +- drivers/staging/tidspbridge/gen/uuidutil.c | 85 .../tidspbridge/include/dspbridge/uuidutil.h | 18 drivers/staging/tidspbridge/rmgr/dbdcd.c | 42 +- 4 files changed, 39 insertions(+), 108 deletions(-) delete mode 100644 drivers/staging/tidspbridge/gen/uuidutil.c I guess the initial mail somehow didn't make it through your spam filter: https://lkml.org/lkml/2013/12/1/70 It's too early to start resending. Wait for another week at least. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
