Re: [PATCH 2/3] Staging rtl8192e: Fixing checkpatch error in rtllib_debug.h
On Sun, Jan 05, 2014 at 01:25:36PM +0100, Andreas Frembs wrote: In rtllib_debug.h we fixed the following checkpatch error: ERROR: Macros with complex values should be enclosed in parenthesis We fixed this with a do {} while (0), because otherwise the compiler complained. Signed-off-by: Andreas Frembs andreas.fre...@studium.uni-erlangen.de Signed-off-by: Matthias Schoepe matthias.scho...@studium.uni-erlangen.de It would be better to replace these with WARN_ON(). I haven't looked but I also suspect that some of these asserts might be bogus and should be removed. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2] Staging: rtl8187se: fix styling issues in r8180_wx.c
On Mon, Jan 06, 2014 at 12:38:51AM -0500, Dan LaManna wrote: This is a patch to the r8180_wx.c which fixes various whitespace issues, brace issues, casting/declaration syntax issues, and increases clarity in multi-line return statement. Looks good. Reviewed-by: Dan Carpenter dan.carpen...@oracle.com regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: comedi: fixed style issue Fixed a coding style issue Signed-off-by: Aruna Hewapathirane aruna.hewapathir...@gmail.com
On Mon, Jan 06, 2014 at 04:18:05AM -0500, Aruna Hewapathirane wrote: my apologies, it's my very first patch :-) No worries. Take your time. We were all newbies once. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: comedi: fix auto-unconfig kernel error
On 2013-12-28 21:31, Bernd Porr wrote: Merging the un-registering of both the subdevices and the main comedi device into one function and the module which actually associated with it. The kernel oops observed before was because the main device was un-registered first and then the subdevices which were then no longer valid. This has been also tested with 'comedi_config -r' for both autoconfigured and legacy devices. Signed-off-by: Bernd Porr m...@berndporr.me.uk --- drivers/staging/comedi/comedi_fops.c | 19 +++ drivers/staging/comedi/drivers.c | 18 -- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index f3d59e2..2680b72 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -141,7 +141,26 @@ static struct comedi_device *comedi_clear_board_minor(unsigned minor) static void comedi_free_board_dev(struct comedi_device *dev) { + int i; + struct comedi_subdevice *s; + if (dev) { + if (dev-subdevices) { + for (i = 0; i dev-n_subdevices; i++) { + s = dev-subdevices[i]; + if (s-runflags SRF_FREE_SPRIV) + kfree(s-private); + comedi_free_subdevice_minor(s); + if (s-async) { + comedi_buf_alloc(dev, s, 0); + kfree(s-async); + } + } + kfree(dev-subdevices); + dev-subdevices = NULL; + dev-n_subdevices = 0; + } + if (dev-class_dev) { device_destroy(comedi_class, MKDEV(COMEDI_MAJOR, dev-minor)); diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c index 4964d2a..d6dc58a 100644 --- a/drivers/staging/comedi/drivers.c +++ b/drivers/staging/comedi/drivers.c @@ -97,24 +97,6 @@ EXPORT_SYMBOL_GPL(comedi_alloc_subdevices); static void cleanup_device(struct comedi_device *dev) { - int i; - struct comedi_subdevice *s; - - if (dev-subdevices) { - for (i = 0; i dev-n_subdevices; i++) { - s = dev-subdevices[i]; - if (s-runflags SRF_FREE_SPRIV) - kfree(s-private); - comedi_free_subdevice_minor(s); - if (s-async) { - comedi_buf_alloc(dev, s, 0); - kfree(s-async); - } - } - kfree(dev-subdevices); - dev-subdevices = NULL; - dev-n_subdevices = 0; - } kfree(dev-private); dev-private = NULL; dev-driver = NULL; This doesn't apply to linux-next any more. (For example, cleanup_device() function was renamed amongst other stuff.) I've also fixed a load of stuff related to this bug since this patch was applicable, so possibly the bug has been squashed already. Bernd, can you reproduce the problem on the latest tagged linux-next branch? Also, the removal of all that code from cleanup_device() (since renamed to comedi_device_detach_cleanup()) would stop the COMEDI_DEVCONFIG ioctl with NULL argument (e.g. via the 'comedi_config -r' command to detach a device) cleaning up after the detachment, especially for the preallocated legacy comedi devices that hang around after the detachment, ready to be reattached via another instance of the ioctl call. -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] staging: comedi: report success/failure of autoconfig
On 2013-12-28 21:32, Bernd Porr wrote: Added success message to the driver autoconfig and error message in case it fails. A success message is required so that the user can find out which comedi driver has been associated with which udev device. This also makes troubleshooting much easier when more than one card is in the computer or a mix of USB and PCI devices. As Ian suggested we should report both the driver and the board which might have different names, esp if one driver covers a range of different boards. Signed-off-by: Bernd Porr m...@berndporr.me.uk --- drivers/staging/comedi/drivers.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c index d6dc58a..59a8909 100644 --- a/drivers/staging/comedi/drivers.c +++ b/drivers/staging/comedi/drivers.c @@ -580,8 +580,12 @@ int comedi_auto_config(struct device *hardware_device, } dev = comedi_alloc_board_minor(hardware_device); - if (IS_ERR(dev)) + if (IS_ERR(dev)) { + dev_warn(hardware_device, +driver '%s' could not create device.\n, +driver-driver_name); return PTR_ERR(dev); + } /* Note: comedi_alloc_board_minor() locked dev-mutex. */ dev-driver = driver; @@ -593,8 +597,19 @@ int comedi_auto_config(struct device *hardware_device, comedi_device_detach(dev); mutex_unlock(dev-mutex); - if (ret 0) + if (ret 0) { + dev_warn(hardware_device, +driver '%s' faild to auto-configure device.\n, A typo here: 'faild' should be 'failed'. +driver-driver_name); comedi_release_hardware_device(hardware_device); + } else { + /* class_dev should be set properly here + after a successful auto config */ The preferred style for multi-line comments is: /* * blah blah * blah blah */ + dev_info(dev-class_dev, +driver '%s' has successfully +auto-configured '%s'.\n, +driver-driver_name, dev-board_name); + } I know the checkpatch.pl script used to complain about concatenated string literals, preferring a single string literal even if the code went over 80 columns as a result. It doesn't seem to complain here, but I think it's still recommended practice. (Could someone clarify the recommended practice here?) return ret; } EXPORT_SYMBOL_GPL(comedi_auto_config); -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: comedi: drivers: streamlined auto attach with main comedi
It would be better to put usbduxsigma: in the patch title instead of drivers: since it's only for a single driver. On 2013-12-28 21:32, Bernd Porr wrote: Removed the word attached from the ADC_zero output which is now reported by comedi itself at the end of the auto attach. A negative value of the offset is an error and should be reported to comedi auto config as an error. Output only the offset if no error has been reported. Signed-off-by: Bernd Porr m...@berndporr.me.uk --- drivers/staging/comedi/drivers/usbduxsigma.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c b/drivers/staging/comedi/drivers/usbduxsigma.c index a5363de..125eae5 100644 --- a/drivers/staging/comedi/drivers/usbduxsigma.c +++ b/drivers/staging/comedi/drivers/usbduxsigma.c @@ -1656,11 +1656,14 @@ static int usbduxsigma_auto_attach(struct comedi_device *dev, } offset = usbduxsigma_getstatusinfo(dev, 0); - if (offset 0) + if (offset 0) { + /* a neg ADC value in comedi terms is a device error */ dev_err(dev-class_dev, - Communication to USBDUXSIGMA failed! Check firmware and cabling\n); + Communication to USBDUXSIGMA failed! Check firmware and cabling.\n); + return offset; + } - dev_info(dev-class_dev, attached, ADC_zero = %x\n, offset); + dev_info(dev-class_dev, ADC_zero = %x\n, offset); return 0; } Ideally, it would be two patches, one to return the error and the other to correct the messages, but doesn't matter too much. Reviewed-by: Ian Abbott abbo...@mev.co.uk -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] staging: comedi: report success/failure of autoconfig
On Mon, Jan 06, 2014 at 11:56:22AM +, Ian Abbott wrote: I know the checkpatch.pl script used to complain about concatenated string literals, preferring a single string literal even if the code went over 80 columns as a result. It doesn't seem to complain here, but I think it's still recommended practice. (Could someone clarify the recommended practice here?) Better to put everything as a string literal so we can grep for it. I think that is still the fashion. :) regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC PATCH 2/3] staging: imx-drm-core: Use graph to find connection between crtc and encoder
Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/staging/imx-drm/imx-drm-core.c | 44 -- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c index 2490dc3..2c20434 100644 --- a/drivers/staging/imx-drm/imx-drm-core.c +++ b/drivers/staging/imx-drm/imx-drm-core.c @@ -23,6 +23,7 @@ #include drm/drm_crtc_helper.h #include drm/drm_gem_cma_helper.h #include drm/drm_fb_cma_helper.h +#include media/v4l2-of.h #include imx-drm.h @@ -420,9 +421,23 @@ EXPORT_SYMBOL_GPL(imx_drm_remove_crtc); * or removed once the DRM device has been fully initialised. */ static uint32_t imx_drm_find_crtc_mask(struct imx_drm_device *imxdrm, - void *cookie, int id) + struct device_node *endpoint) { + struct device_node *remote_port; + void *cookie; unsigned i; + int id = 0; + + remote_port = v4l2_of_get_remote_port(endpoint); + if (remote_port) + of_property_read_u32(remote_port, reg, id); + else + return 0; + cookie = remote_port-parent; + of_node_put(remote_port); + + /* IPU specific: CSI0/1 at 0/1, DI0/1 at 2/3 */ + id -= 2; for (i = 0; i MAX_CRTC; i++) { struct imx_drm_crtc *imx_drm_crtc = imxdrm-crtc[i]; @@ -438,24 +453,21 @@ int imx_drm_encoder_parse_of(struct drm_device *drm, struct drm_encoder *encoder, struct device_node *np) { struct imx_drm_device *imxdrm = drm-dev_private; + struct device_node *ep, *last_ep = NULL; uint32_t crtc_mask = 0; int i, ret = 0; for (i = 0; !ret; i++) { - struct of_phandle_args args; uint32_t mask; - int id; - ret = of_parse_phandle_with_args(np, crtcs, #crtc-cells, i, -args); - if (ret == -ENOENT) + ep = v4l2_of_get_next_endpoint(np, last_ep); + if (last_ep) + of_node_put(last_ep); + if (!ep) break; - if (ret 0) - return ret; - id = args.args_count 0 ? args.args[0] : 0; - mask = imx_drm_find_crtc_mask(imxdrm, args.np, id); - of_node_put(args.np); + /* CSI */ + mask = imx_drm_find_crtc_mask(imxdrm, ep); /* * If we failed to find the CRTC(s) which this encoder is @@ -463,12 +475,20 @@ int imx_drm_encoder_parse_of(struct drm_device *drm, * not been registered yet. Defer probing, and hope that * the required CRTC is added later. */ - if (mask == 0) + if (mask == 0) { + of_node_put(ep); return -EPROBE_DEFER; + } crtc_mask |= mask; + last_ep = ep; } + if (ep) + of_node_put(ep); + if (i == 0) + return -ENOENT; + encoder-possible_crtcs = crtc_mask; /* FIXME: this is the mask of outputs which can clone this output. */ -- 1.8.5.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC PATCH 3/3] staging: imx-drm-core: associate crtc devices with di port nodes
To connect the graph in the device tree, each display interface needs to have an associated port node in the device tree. We can associate this node with the crtc device and use it to find the crtc corresponding to a given node instead of using a combination of parent device node and id. Explicitly converting the void* cookie to the port device tree node allows to get rid of the ipu_id and di_id fields. The multiplexer setting now can be obtained from the port reg property in the device tree. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/staging/imx-drm/imx-drm-core.c | 62 +++--- drivers/staging/imx-drm/imx-drm.h | 4 +-- drivers/staging/imx-drm/imx-hdmi.c | 2 +- drivers/staging/imx-drm/imx-ldb.c | 4 +-- drivers/staging/imx-drm/ipuv3-crtc.c | 36 ++-- 5 files changed, 74 insertions(+), 34 deletions(-) diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c index 2c20434..da34b2d 100644 --- a/drivers/staging/imx-drm/imx-drm-core.c +++ b/drivers/staging/imx-drm/imx-drm-core.c @@ -42,9 +42,7 @@ struct imx_drm_crtc { struct drm_crtc *crtc; int pipe; struct imx_drm_crtc_helper_funcsimx_drm_helper_funcs; - void*cookie; - int di_id; - int ipu_id; + struct device_node *port; }; static int legacyfb_depth = 16; @@ -339,14 +337,11 @@ err_kms: /* * imx_drm_add_crtc - add a new crtc - * - * The return value if !NULL is a cookie for the caller to pass to - * imx_drm_remove_crtc later. */ int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc, struct imx_drm_crtc **new_crtc, const struct imx_drm_crtc_helper_funcs *imx_drm_helper_funcs, - void *cookie, int ipu_id, int di_id) + struct device_node *port) { struct imx_drm_device *imxdrm = drm-dev_private; struct imx_drm_crtc *imx_drm_crtc; @@ -368,9 +363,7 @@ int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc, imx_drm_crtc-imx_drm_helper_funcs = *imx_drm_helper_funcs; imx_drm_crtc-pipe = imxdrm-pipes++; - imx_drm_crtc-cookie = cookie; - imx_drm_crtc-di_id = di_id; - imx_drm_crtc-ipu_id = ipu_id; + imx_drm_crtc-port = port; imx_drm_crtc-crtc = crtc; imxdrm-crtc[imx_drm_crtc-pipe] = imx_drm_crtc; @@ -414,7 +407,7 @@ int imx_drm_remove_crtc(struct imx_drm_crtc *imx_drm_crtc) EXPORT_SYMBOL_GPL(imx_drm_remove_crtc); /* - * Find the DRM CRTC possible mask for the device node cookie/id. + * Find the DRM CRTC possible mask for the connected endpoint. * * The encoder possible masks are defined by their position in the * mode_config crtc_list. This means that CRTCs must not be added @@ -423,26 +416,17 @@ EXPORT_SYMBOL_GPL(imx_drm_remove_crtc); static uint32_t imx_drm_find_crtc_mask(struct imx_drm_device *imxdrm, struct device_node *endpoint) { - struct device_node *remote_port; - void *cookie; + struct device_node *port; unsigned i; - int id = 0; - remote_port = v4l2_of_get_remote_port(endpoint); - if (remote_port) - of_property_read_u32(remote_port, reg, id); - else + port = v4l2_of_get_remote_port(endpoint); + if (!port) return 0; - cookie = remote_port-parent; - of_node_put(remote_port); - - /* IPU specific: CSI0/1 at 0/1, DI0/1 at 2/3 */ - id -= 2; + of_node_put(port); for (i = 0; i MAX_CRTC; i++) { struct imx_drm_crtc *imx_drm_crtc = imxdrm-crtc[i]; - if (imx_drm_crtc imx_drm_crtc-di_id == id - imx_drm_crtc-cookie == cookie) + if (imx_drm_crtc imx_drm_crtc-port == port) return drm_helper_crtc_possible_mask(imx_drm_crtc-crtc); } @@ -498,11 +482,35 @@ int imx_drm_encoder_parse_of(struct drm_device *drm, } EXPORT_SYMBOL_GPL(imx_drm_encoder_parse_of); -int imx_drm_encoder_get_mux_id(struct drm_encoder *encoder) +int imx_drm_encoder_get_mux_id(struct device *dev, struct drm_encoder *encoder) { struct imx_drm_crtc *imx_crtc = imx_drm_find_crtc(encoder-crtc); + struct device_node *parent = dev-of_node; + struct device_node *ep, *last_ep = NULL; + struct device_node *port; + int id, ret; + + if (!parent || !imx_crtc) + return -EINVAL; + + do { + ep = v4l2_of_get_next_endpoint(parent, last_ep); + if (last_ep) + of_node_put(last_ep); + if (!ep) + break; + + port = v4l2_of_get_remote_port(ep); + if (port == imx_crtc-port) { +
Re: [PATCH RFC 27/46] imx-drm: convert to componentised device support
Hi Eric, Am Freitag, den 03.01.2014, 12:14 -0700 schrieb Eric Nelson: This is an issue we've seen before. The SABRE Lite board has a voltage divider on the HPD pins and some monitors (esp. DVI monitors) either don't drive things high enough to assert HPD or bounce with connect/disconnect. Yes, I used a DVI monitor. We've instrumented our 3.0.35 kernels to use the RX_SENSE bits instead. Reacting to RX_SENSE0 instead of HPD seems to work. thanks Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 27/46] imx-drm: convert to componentised device support
On Mon, Jan 06, 2014 at 06:41:28PM +0100, Philipp Zabel wrote: Hi Eric, Am Freitag, den 03.01.2014, 12:14 -0700 schrieb Eric Nelson: This is an issue we've seen before. The SABRE Lite board has a voltage divider on the HPD pins and some monitors (esp. DVI monitors) either don't drive things high enough to assert HPD or bounce with connect/disconnect. Yes, I used a DVI monitor. We've instrumented our 3.0.35 kernels to use the RX_SENSE bits instead. Reacting to RX_SENSE0 instead of HPD seems to work. However, it's non-compliant, because HPD can be lowered and raised by the sink when it changes its EDID data (eg, because you're connected through a switch and the routing has been changed.) So, reacting to RX_SENSE0 instead of HPD has to be a work-around enabled only for those boards which are broken in this regard. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 24/46] imx-drm: provide common connector mode validation function
On Thu, Jan 02, 2014 at 09:27:48PM +, Russell King wrote: diff --git a/drivers/staging/imx-drm/imx-drm.h b/drivers/staging/imx-drm/imx-drm.h index 5649f180dc44..4eb594ce9cff 100644 --- a/drivers/staging/imx-drm/imx-drm.h +++ b/drivers/staging/imx-drm/imx-drm.h @@ -68,4 +68,7 @@ int imx_drm_encoder_get_mux_id(struct drm_encoder *encoder); int imx_drm_encoder_add_possible_crtcs(struct imx_drm_encoder *imx_drm_encoder, struct device_node *np); +int imx_drm_connector_mode_valid(struct drm_connector *connector, + struct drm_display_mode *mode); + #endif /* _IMX_DRM_H_ */ CC drivers/staging/imx-drm/ipu-v3/ipu-dc.o LD net/ethernet/built-in.o In file included from drivers/staging/imx-drm/ipu-v3/ipu-dc.c:23:0: drivers/staging/imx-drm/ipu-v3/../imx-drm.h:56:9: warning: ‘struct drm_display_mode’ declared inside parameter list [enabled by default] drivers/staging/imx-drm/ipu-v3/../imx-drm.h:56:9: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default] Shawn ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC 30/46] imx-drm: remove separate imx-fbdev
On Thu, Jan 02, 2014 at 09:28:19PM +, Russell King wrote: @@ -449,6 +458,24 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags) } } + /* + * All components are now initialised, so setup the fb helper. + * The fb helper takes copies of key hardware information, so the + * crtcs/connectors/encoders must not change after this point. + */ +#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER) + if (legacyfb_depth != 16 legacyfb_depth != 32) { + dev_warn(drm-dev, Invalid legacyfb_depth. Defaulting to 16bpp\n); + legacyfb_depth = 16; + } + imxdrm-fbhelper = drm_fbdev_cma_init(drm, legacyfb_depth, + drm-mode_config.num_crtc, 4); s/4/MAX_CRTC imx-drm-core.c has the macro. Shawn + if (IS_ERR(imxdrm-fbhelper)) { + ret = PTR_ERR(imxdrm-fbhelper); + imxdrm-fbhelper = NULL; + goto err_unbind; + } +#endif return 0; err_unbind: ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel