[PATCH 1/2] staging: bcm: Remove unneeded set a variable
bClassificationSucceed is initialized with false, do not need to set false again. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/bcm/Qos.c |3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/bcm/Qos.c b/drivers/staging/bcm/Qos.c index 0727599..ead57b4 100644 --- a/drivers/staging/bcm/Qos.c +++ b/drivers/staging/bcm/Qos.c @@ -222,10 +222,7 @@ static USHORT IpVersion4(struct bcm_mini_adapter *Adapter, //Checking classifier validity if (!pstClassifierRule-bUsed || pstClassifierRule-ucDirection == DOWNLINK_DIR) - { - bClassificationSucceed = false; break; - } BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, is IPv6 check!); if (pstClassifierRule-bIpv6Protocol) -- 1.7.9.5 --- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: bcm: fix checkpatch error 'assignment in if condition'
clean up checkpatch errors and bClassificationSucceed is set to TRUE proper location. If protocal is not TCP or UDP, when it checks protocal, bClassificationSucceed must be set to TRUE. Also the end of do-while(0) loop, bClassificationSucceed is set to TRUE. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/bcm/Qos.c | 32 ++-- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/drivers/staging/bcm/Qos.c b/drivers/staging/bcm/Qos.c index ead57b4..4f31583 100644 --- a/drivers/staging/bcm/Qos.c +++ b/drivers/staging/bcm/Qos.c @@ -230,51 +230,47 @@ static USHORT IpVersion4(struct bcm_mini_adapter *Adapter, //**Checking IP header parameter**// BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, Trying to match Source IP Address); - if (false == (bClassificationSucceed = - MatchSrcIpAddress(pstClassifierRule, iphd-saddr))) + if (!MatchSrcIpAddress(pstClassifierRule, iphd-saddr)) break; BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, Source IP Address Matched); - if (false == (bClassificationSucceed = - MatchDestIpAddress(pstClassifierRule, iphd-daddr))) + if (!MatchDestIpAddress(pstClassifierRule, iphd-daddr)) break; BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, Destination IP Address Matched); - if (false == (bClassificationSucceed = - MatchTos(pstClassifierRule, iphd-tos))) - { + if (!MatchTos(pstClassifierRule, iphd-tos)) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, TOS Match failed\n); break; } BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, TOS Matched); - if (false == (bClassificationSucceed = - MatchProtocol(pstClassifierRule, iphd-protocol))) + if (!MatchProtocol(pstClassifierRule, iphd-protocol)) break; BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, Protocol Matched); //if protocol is not TCP or UDP then no need of comparing source port and destination port - if (iphd-protocol != TCP iphd-protocol != UDP) + if (iphd-protocol != TCP iphd-protocol != UDP) { + bClassificationSucceed = TRUE; break; + } //**Checking Transport Layer Header field if present *// BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, Source Port %04x, (iphd-protocol == UDP) ? xprt_hdr-uhdr.source : xprt_hdr-thdr.source); - if (false == (bClassificationSucceed = - MatchSrcPort(pstClassifierRule, - ntohs((iphd-protocol == UDP) ? - xprt_hdr-uhdr.source : xprt_hdr-thdr.source + if (!MatchSrcPort(pstClassifierRule, + ntohs((iphd-protocol == UDP) ? + xprt_hdr-uhdr.source : xprt_hdr-thdr.source))) break; BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, Src Port Matched); BCM_DEBUG_PRINT(Adapter, DBG_TYPE_TX, IPV4_DBG, DBG_LVL_ALL, Destination Port %04x, (iphd-protocol == UDP) ? xprt_hdr-uhdr.dest : xprt_hdr-thdr.dest); - if (false == (bClassificationSucceed = - MatchDestPort(pstClassifierRule, - ntohs((iphd-protocol == UDP) ? - xprt_hdr-uhdr.dest : xprt_hdr-thdr.dest + if (!MatchDestPort(pstClassifierRule, + ntohs((iphd-protocol == UDP) ? + xprt_hdr-uhdr.dest : xprt_hdr-thdr.dest))) break; + bClassificationSucceed = TRUE; } while (0); if (TRUE == bClassificationSucceed) -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings
On 25/02/14 16:23, Philipp Zabel wrote: +Freescale i.MX DRM master device + + +The freescale i.MX DRM master device is a virtual device needed to list all +IPU or other display interface nodes that comprise the graphics subsystem. + +Required properties: +- compatible: Should be fsl,imx-drm +- ports: Should contain a list of phandles pointing to display interface ports + of IPU devices + +example: + +imx-drm { + compatible = fsl,imx-drm; + ports = ipu_di0; +}; I'm not a fan of having non-hardware related things in the DT data. Especially if it makes direct references to our SW, in this case DRM. There's no DRM on the board. I wanted to avoid all that with OMAP display bindings. Is there even need for such a master device? You can find all the connected display devices from any single display device, by just following the endpoint links. display@di0 { compatible = fsl,imx-parallel-display; edid = [edid-data]; - crtc = ipu 0; interface-pix-fmt = rgb24; + + port { + display_in: endpoint { + remote-endpoint = ipu_di0_disp0; + }; + }; }; Shouldn't the pix-fmt be defined in the endpoint node? It is about pixel format for a particular endpoint, isn't it? diff --git a/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt index ed93778..578a1fc 100644 --- a/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt +++ b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt @@ -50,12 +50,14 @@ have a look at Documentation/devicetree/bindings/video/display-timing.txt. Required properties: - reg : should be 0 or 1 - - crtcs : a list of phandles with index pointing to the IPU display interfaces - that can be used as video source for this channel. - fsl,data-mapping : should be spwg or jeida This describes how the color bits are laid out in the serialized LVDS signal. - fsl,data-width : should be 18 or 24 + - port: A port node with endpoint definitions as defined in + Documentation/devicetree/bindings/media/video-interfaces.txt. + On i.MX6, there should be four ports (port@[0-3]) that correspond + to the four LVDS multiplexer inputs. Is the ldb something that's on the imx SoC? Do you have a public branch somewhere? It'd be easier to look at the final result, as I'm not familiar with imx. Tomi signature.asc Description: OpenPGP digital signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: Fix typo in rtl8188eu/core
Fix spelling typo in comments and printk within rtl8188eu/core Signed-off-by: Masanari Iida standby2...@gmail.com --- drivers/staging/rtl8188eu/core/rtw_ap.c| 14 +- drivers/staging/rtl8188eu/core/rtw_br_ext.c| 6 ++--- drivers/staging/rtl8188eu/core/rtw_debug.c | 2 +- drivers/staging/rtl8188eu/core/rtw_ioctl_set.c | 4 +-- drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 36 +- drivers/staging/rtl8188eu/core/rtw_mp.c| 4 +-- drivers/staging/rtl8188eu/core/rtw_pwrctrl.c | 2 +- drivers/staging/rtl8188eu/core/rtw_recv.c | 8 +++--- drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 2 +- drivers/staging/rtl8188eu/core/rtw_xmit.c | 6 ++--- 10 files changed, 42 insertions(+), 42 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c index 62a6147..0f1c1e5 100644 --- a/drivers/staging/rtl8188eu/core/rtw_ap.c +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c @@ -94,7 +94,7 @@ static void update_BCNTIM(struct adapter *padapter) } else { tim_ielen = 0; - /* calucate head_len */ + /* calculate head_len */ offset = _FIXED_IE_LENGTH_; offset += pnetwork_mlmeext-Ssid.SsidLength + 2; @@ -129,7 +129,7 @@ static void update_BCNTIM(struct adapter *padapter) *dst_ie++ = tim_ielen; *dst_ie++ = 0;/* DTIM count */ - *dst_ie++ = 1;/* DTIM peroid */ + *dst_ie++ = 1;/* DTIM period */ if (pstapriv-tim_bitmapBIT(0))/* for bc/mc frames */ *dst_ie++ = BIT(0);/* bitmap ctrl */ @@ -821,7 +821,7 @@ static void start_bss_network(struct adapter *padapter, u8 *pbuf) /* update cur_wireless_mode */ update_wireless_mode(padapter); - /* udpate capability after cur_wireless_mode updated */ + /* update capability after cur_wireless_mode updated */ update_capinfo(padapter, rtw_get_capability((struct wlan_bssid_ex *)pnetwork)); /* let pnetwork_mlmeext == pnetwork_mlme. */ @@ -1415,7 +1415,7 @@ void update_beacon(struct adapter *padapter, u8 ie_id, u8 *oui, u8 tx) /* op_mode -Set to 0 (HT pure) under the followign conditions +Set to 0 (HT pure) under the following conditions - all STAs in the BSS are 20/40 MHz HT in 20/40 MHz BSS or - all STAs in the BSS are 20 MHz HT in 20 MHz BSS Set to 1 (HT non-member protection) if there may be non-HT STAs @@ -1494,7 +1494,7 @@ static int rtw_ht_operation_update(struct adapter *padapter) void associated_clients_update(struct adapter *padapter, u8 updated) { - /* update associcated stations cap. */ + /* update associated stations cap. */ if (updated) { struct list_head *phead, *plist; struct sta_info *psta = NULL; @@ -1647,7 +1647,7 @@ void bss_cap_update_on_sta_join(struct adapter *padapter, struct sta_info *psta) update_beacon(padapter, _HT_ADD_INFO_IE_, NULL, true); } - /* update associcated stations cap. */ + /* update associated stations cap. */ associated_clients_update(padapter, beacon_updated); DBG_88E(%s, updated =%d\n, __func__, beacon_updated); @@ -1711,7 +1711,7 @@ u8 bss_cap_update_on_sta_leave(struct adapter *padapter, struct sta_info *psta) update_beacon(padapter, _HT_ADD_INFO_IE_, NULL, true); } - /* update associcated stations cap. */ + /* update associated stations cap. */ DBG_88E(%s, updated =%d\n, __func__, beacon_updated); diff --git a/drivers/staging/rtl8188eu/core/rtw_br_ext.c b/drivers/staging/rtl8188eu/core/rtw_br_ext.c index 75e38d4..96c8a93 100644 --- a/drivers/staging/rtl8188eu/core/rtw_br_ext.c +++ b/drivers/staging/rtl8188eu/core/rtw_br_ext.c @@ -543,10 +543,10 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method) if (!__nat25_db_network_lookup_and_replace(priv, skb, networkAddr)) { if (*((unsigned char *)iph-daddr + 3) == 0xff) { /* L2 is unicast but L3 is broadcast, make L2 bacome broadcast */ - DEBUG_INFO(NAT25: Set DA as boardcast\n); + DEBUG_INFO(NAT25: Set DA as broadcast\n); memset(skb-data, 0xff, ETH_ALEN); } else { - /* forward unknow IP packet to upper TCP/IP */ + /* forward unknown IP packet to upper TCP/IP */ DEBUG_INFO(NAT25: Replace DA with BR's MAC\n); if ((*(u32 *)priv-br_mac) == 0 (*(u16 *)(priv-br_mac+4)) == 0) {
[PATCH] staging: usbip: Fix format string mismatch in usbip_vhci_attach_device2
Argument type of sockfd is set as int, but format string is set as unsigned int. Fix the mismatch. Signed-off-by: Masanari Iida standby2...@gmail.com --- drivers/staging/usbip/userspace/libsrc/vhci_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c index f4bfefe..d80d37c 100644 --- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c +++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c @@ -545,7 +545,7 @@ int usbip_vhci_attach_device2(uint8_t port, int sockfd, uint32_t devid, return -1; } - snprintf(buff, sizeof(buff), %u %u %u %u, + snprintf(buff, sizeof(buff), %u %d %u %u, port, sockfd, devid, speed); dbg(writing: %s, buff); -- 1.9.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings
Am Donnerstag, den 27.02.2014, 13:06 +0200 schrieb Tomi Valkeinen: On 25/02/14 16:23, Philipp Zabel wrote: +Freescale i.MX DRM master device + + +The freescale i.MX DRM master device is a virtual device needed to list all +IPU or other display interface nodes that comprise the graphics subsystem. + +Required properties: +- compatible: Should be fsl,imx-drm +- ports: Should contain a list of phandles pointing to display interface ports + of IPU devices + +example: + +imx-drm { + compatible = fsl,imx-drm; + ports = ipu_di0; +}; I'm not a fan of having non-hardware related things in the DT data. Especially if it makes direct references to our SW, in this case DRM. There's no DRM on the board. I wanted to avoid all that with OMAP display bindings. Is there even need for such a master device? You can find all the connected display devices from any single display device, by just following the endpoint links. I don't particularly like this either, but it kind of has been decided. For the i.MX6 display subsystem there is no clear single master device, and the physical configuration changes across the SoC family. The i.MX6Q/i.MX6D SoCs have two separate display controller devices IPU1 and IPU2, with two output ports each. The i.MX6DL/i.MX6S SoCs only have one IPU1, but it is accompanied by separate lower-power LCDIF display controller with a single output. These may or may not be connected indirectly across the encoder input multiplexers, so collecting them would require scanning the whole device tree from an always-enabled imx-drm platform device if we didn't have this node. Also, we are free to just ignore this node in the future, if a better way is found. display@di0 { compatible = fsl,imx-parallel-display; edid = [edid-data]; - crtc = ipu 0; interface-pix-fmt = rgb24; + + port { + display_in: endpoint { + remote-endpoint = ipu_di0_disp0; + }; + }; }; Shouldn't the pix-fmt be defined in the endpoint node? It is about pixel format for a particular endpoint, isn't it? diff --git a/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt index ed93778..578a1fc 100644 --- a/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt +++ b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt @@ -50,12 +50,14 @@ have a look at Documentation/devicetree/bindings/video/display-timing.txt. Required properties: - reg : should be 0 or 1 - - crtcs : a list of phandles with index pointing to the IPU display interfaces - that can be used as video source for this channel. - fsl,data-mapping : should be spwg or jeida This describes how the color bits are laid out in the serialized LVDS signal. - fsl,data-width : should be 18 or 24 + - port: A port node with endpoint definitions as defined in + Documentation/devicetree/bindings/media/video-interfaces.txt. + On i.MX6, there should be four ports (port@[0-3]) that correspond + to the four LVDS multiplexer inputs. Is the ldb something that's on the imx SoC? Yes. It consists of two LVDS encoders. On i.MX5 each channel is connected to one display interface of the single IPU. On i.MX6Q its parallel input can be connected to any of the four IPU1/2 display interfaces using a 4-port multiplexer (and on i.MX6DL it can be connected to IPU1 or LCDIF). Do you have a public branch somewhere? It'd be easier to look at the final result, as I'm not familiar with imx. Not yet, I will prepare a branch with the next version. regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings
On Thu, Feb 27, 2014 at 02:06:25PM +0100, Philipp Zabel wrote: For the i.MX6 display subsystem there is no clear single master device, and the physical configuration changes across the SoC family. The i.MX6Q/i.MX6D SoCs have two separate display controller devices IPU1 and IPU2, with two output ports each. Not also forgetting that there's another scenario too: you may wish to drive IPU1 and IPU2 as two completely separate display subsystems in some hardware, but as a combined display subsystem in others. Here's another scenario. You may have these two IPUs on the SoC, but there's only one display output. You want to leave the second IPU disabled, as you wouldn't want it to be probed or even exposed to userland. On the face of it, the top-level super-device node doesn't look very hardware-y, but it actually is - it's about how a board uses the hardware provided. This is entirely in keeping with the spirit of DT, which is to describe what hardware is present and how it's connected together, whether it be at the chip or board level. If this wasn't the case, we wouldn't even attempt to describe what devices we have on which I2C buses - we'd just list the hardware on the board without giving any information about how it's wired together. This is no different - however, it doesn't have (and shouldn't) be subsystem specific... but - and this is the challenge we then face - how do you decide that on one board with a single zImage kernel, with both DRM and fbdev built-in, whether to use the DRM interfaces or the fbdev interfaces? We could have both matching the same compatible string, but we'd also need some way to tell each other that they're not allowed to bind. Before anyone argues against it isn't hardware-y, stop and think. What if I design a board with two Epson LCD controllers on board and put a muxing arrangement on their output. Is that one or two devices? What if I want them to operate as one combined system? What if I have two different LCD controllers on a board. How is this any different from the two independent IPU hardware blocks integrated inside an iMX6 SoC with a muxing arrangement on their output? It's very easy to look at a SoC and make the wrong decision... -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] Memory leak in usbip_exported_device_new
From: Heinrich Schuchardt xypron.g...@gmx.de Memory was leaked and a device not closed. Signed-off-by: Heinrich Schuchardt xypron.g...@gmx.de --- drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c b/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c index 71a449c..6a92f0f 100644 --- a/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c +++ b/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c @@ -102,6 +102,7 @@ static int32_t read_attr_usbip_status(struct usbip_usb_device *udev) static struct usbip_exported_device *usbip_exported_device_new(char *sdevpath) { struct usbip_exported_device *edev = NULL; + struct usbip_exported_device *edev_old; size_t size; int i; @@ -127,8 +128,10 @@ static struct usbip_exported_device *usbip_exported_device_new(char *sdevpath) size = sizeof(*edev) + edev-udev.bNumInterfaces * sizeof(struct usbip_usb_interface); + edev_old = edev; edev = realloc(edev, size); if (!edev) { + edev = edev_old; dbg(realloc failed); goto err; } -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings
On 27/02/14 13:56, Russell King - ARM Linux wrote: Is there even need for such a master device? You can find all the connected display devices from any single display device, by just following the endpoint links. Please read up on what has been discussed over previous years: http://lists.freedesktop.org/archives/dri-devel/2013-July/041159.html Thanks, that was an interesting thread. Too bad I missed it, it was during the holiday season. And seems Laurent missed it also, as he didn't make any replies. The thread seemed to go over the very same things that had already been discussed with CDF. Tomi signature.asc Description: OpenPGP digital signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings
On Thu, Feb 27, 2014 at 03:16:03PM +0200, Tomi Valkeinen wrote: On 27/02/14 13:56, Russell King - ARM Linux wrote: Is there even need for such a master device? You can find all the connected display devices from any single display device, by just following the endpoint links. Please read up on what has been discussed over previous years: http://lists.freedesktop.org/archives/dri-devel/2013-July/041159.html Thanks, that was an interesting thread. Too bad I missed it, it was during the holiday season. And seems Laurent missed it also, as he didn't make any replies. The thread seemed to go over the very same things that had already been discussed with CDF. That may be - but the problem with CDF solving this problem is that it's wrong. It's fixing what is in actual fact a *generic* problem in a much too specific way. To put it another way, it's forcing everyone to fix the same problem in their own separate ways because no one is willing to take a step back and look at the larger picture. We can see that because ASoC has exactly the same problem - it has to wait until all devices (DMA, CPU DAIs, codecs etc) are present before it can initialise, just like DRM. Can you re-use the CDF solution for ASoC? No. Can it be re-used elsewhere in non-display subsystems? No. Therefore, CDF is yet another implementation specific solution to a generic problem which can't be re-used. Yes, I realise that CDF may do other stuff, but because of the above, it's a broken solution. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings
On 27/02/14 15:00, Russell King - ARM Linux wrote: On Thu, Feb 27, 2014 at 02:06:25PM +0100, Philipp Zabel wrote: For the i.MX6 display subsystem there is no clear single master device, and the physical configuration changes across the SoC family. The i.MX6Q/i.MX6D SoCs have two separate display controller devices IPU1 and IPU2, with two output ports each. Not also forgetting that there's another scenario too: you may wish to drive IPU1 and IPU2 as two completely separate display subsystems in some hardware, but as a combined display subsystem in others. Here's another scenario. You may have these two IPUs on the SoC, but there's only one display output. You want to leave the second IPU disabled, as you wouldn't want it to be probed or even exposed to userland. I first want to say I don't see anything wrong with such a super node. As you say, it does describe hardware. But I also want to say that I still don't see a need for it. Or, maybe more exactly, I don't see a need for it in general. Maybe there are certain cases where two devices has to be controlled by a master device. Maybe this one is one of those. In the imx case, why wouldn't this work, without any master node, with the IPU nodes separate in the DT data: - One IPU enabled, one disabled: nothing special here, just set the other IPU to status=disabled in the DT data. The driver for the enabled IPU would register the required DRM entities. - Two IPUs as separate units: almost the same as above, but both would independently register the DRM entities. - Two IPUs in combined mode: Pick one IPU as the master, and one as slave. Link the IPU nodes in DT data with phandles, say: master=ipu1 on the slave IPU and slave=ipu0 on the master. The master one will register the DRM entities, and the slave one will just do what the master says. As for the probe time are we ready yet? problem, the IPU driver can just delay registering the DRM entities until all the nodes in its graph have been probed. The component helpers can probably be used here. On the face of it, the top-level super-device node doesn't look very hardware-y, but it actually is - it's about how a board uses the hardware provided. This is entirely in keeping with the spirit of DT, which is to describe what hardware is present and how it's connected together, whether it be at the chip or board level. No disagreement there. I'm mostly put off by the naming. The binding doc says it's a DRM master device, compatible with fsl,imx-drm. Now, naming may not be the most important thing in the world, but I'd rather use generic terms, not linux driver stack names. If this wasn't the case, we wouldn't even attempt to describe what devices we have on which I2C buses - we'd just list the hardware on the board without giving any information about how it's wired together. This is no different - however, it doesn't have (and shouldn't) be subsystem specific... but - and this is the challenge we then face - how do you decide that on one board with a single zImage kernel, with both DRM and fbdev built-in, whether to use the DRM interfaces or the fbdev interfaces? We could have both matching the same compatible string, but we'd also need some way to tell each other that they're not allowed to bind. Yes, that's an annoying problem, we have that on OMAP. It's a clear sign that our video support is rather messed up. My opinion is that the fbdev and drm drivers for a single hardware should be exclusive at compile time. We don't allow multiple drivers for single device for other subsystems either, do we? Eventually we should have only one driver for one hardware device. If that's not possible, then the drivers in question could have an option to enable or disable themselves, passed via the kernel command line, so that the user can select which subsystem to use. Before anyone argues against it isn't hardware-y, stop and think. What if I design a board with two Epson LCD controllers on board and put a muxing arrangement on their output. Is that one or two devices? What if I want them to operate as one combined system? What if I have two different LCD controllers on a board. How is this any different from the two independent IPU hardware blocks integrated inside an iMX6 SoC with a muxing arrangement on their output? Well, generally speaking, I think one option is to treat the two controllers separately and let the userspace handle it. That may or may not be viable, depending on the hardware, but to me it resembles very much a PC with two video cards. If you want the two controllers to operate together more closely, you always need special code for that particular case. This is what CDF has been trying to accomplish: individual drivers for each display entity, connected together via ports and endpoints. Driver for Epson LCD controller would expose an API, that can be used handle the LCD controller, it wouldn't make any other demands on how it's used, is it
Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings
On 27/02/14 15:43, Russell King - ARM Linux wrote: That may be - but the problem with CDF solving this problem is that it's wrong. It's fixing what is in actual fact a *generic* problem in a much too specific way. To put it another way, it's forcing everyone to fix the same problem in their own separate ways because no one is willing to take a step back and look at the larger picture. We can see that because ASoC has exactly the same problem - it has to wait until all devices (DMA, CPU DAIs, codecs etc) are present before it can initialise, just like DRM. Can you re-use the CDF solution for ASoC? No. Can it be re-used elsewhere in non-display subsystems? No. Therefore, CDF is yet another implementation specific solution to a generic problem which can't be re-used. Yes, I realise that CDF may do other stuff, but because of the above, it's a broken solution. What? Because CDF didn't fix a particular subproblem for everyone, it's broken solution? Or did I miss your point? The main point of CDF is not solving the initialization issue. If that was the point, it would've been Common Initialization Framework. The main point of CDF is to allow us to have encoder and panel drivers that can be used by all platforms, in complex display pipeline setups. It just also has to have some solution for the initialization problem to get things working. In fact, Laurent's CDF version has a solution for init problem which, I my memory serves me right, is very much similar to yours. It just wasn't generic. I don't remember if Laurent had a specific master node defined, but the LCD controller was very much like it. It would be trivial to change it to use the component helpers. My solution is different, because I don't like the idea of requiring all the display components to be up and running to use any of the displays. In fact, it's not a solution at all for me, as it would prevent displays working on boards that do not have all the display components installed, or if the user didn't compile all the drivers. Tomi signature.asc Description: OpenPGP digital signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: speakup: buffers: Fixed a return value coding style issue.
Fixed a coding style issue. Signed-off-by: Reza Snowdon r...@mage.me.uk --- drivers/staging/speakup/buffers.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/speakup/buffers.c b/drivers/staging/speakup/buffers.c index 382973e..8d7caa7 100644 --- a/drivers/staging/speakup/buffers.c +++ b/drivers/staging/speakup/buffers.c @@ -55,7 +55,8 @@ static int synth_buffer_free(void) int synth_buffer_empty(void) { - return (buff_in == buff_out); + int buff_comparison = (buff_in == buff_out); + return buff_comparison; } EXPORT_SYMBOL_GPL(synth_buffer_empty); -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: speakup: buffers: Fixed a return value coding style issue.
On Thu, Feb 27, 2014 at 04:26:42PM +, Reza Snowdon wrote: Fixed a coding style issue. Signed-off-by: Reza Snowdon r...@mage.me.uk --- drivers/staging/speakup/buffers.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/speakup/buffers.c b/drivers/staging/speakup/buffers.c index 382973e..8d7caa7 100644 --- a/drivers/staging/speakup/buffers.c +++ b/drivers/staging/speakup/buffers.c @@ -55,7 +55,8 @@ static int synth_buffer_free(void) int synth_buffer_empty(void) { - return (buff_in == buff_out); + int buff_comparison = (buff_in == buff_out); + return buff_comparison; What's wrong with the original code here? If anything, just drop the (), but really, it's fine, leave it alone, it's obvious what is happening is ok. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: speakup: buffers: Fixed a return value coding style issue.
On Thu, Feb 27, 2014 at 04:26:42PM +, Reza Snowdon wrote: Fixed a coding style issue. Signed-off-by: Reza Snowdon r...@mage.me.uk --- drivers/staging/speakup/buffers.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/speakup/buffers.c b/drivers/staging/speakup/buffers.c index 382973e..8d7caa7 100644 --- a/drivers/staging/speakup/buffers.c +++ b/drivers/staging/speakup/buffers.c @@ -55,7 +55,8 @@ static int synth_buffer_free(void) int synth_buffer_empty(void) { - return (buff_in == buff_out); + int buff_comparison = (buff_in == buff_out); + return buff_comparison; This isn't simpler than the original code. Just do: int synth_buffer_empty(void) { return buff_in == buff_out; } regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: speakup: buffers: Fixed a return value coding style issue.
When checking buffers.c with checkpatch.pl it throws the following error: drivers/staging/speakup/buffers.c:58: ERROR: return is not a function, parentheses are not required I watched one of your talks, it's very informative and concise: http://www.youtube.com/watch?v=LLBrBBImJt4feature=youtu.be Thanks, Reza Snowdon On 27/02/14 16:40, Greg KH wrote: On Thu, Feb 27, 2014 at 04:26:42PM +, Reza Snowdon wrote: Fixed a coding style issue. Signed-off-by: Reza Snowdon r...@mage.me.uk --- drivers/staging/speakup/buffers.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/speakup/buffers.c b/drivers/staging/speakup/buffers.c index 382973e..8d7caa7 100644 --- a/drivers/staging/speakup/buffers.c +++ b/drivers/staging/speakup/buffers.c @@ -55,7 +55,8 @@ static int synth_buffer_free(void) int synth_buffer_empty(void) { - return (buff_in == buff_out); + int buff_comparison = (buff_in == buff_out); + return buff_comparison; What's wrong with the original code here? If anything, just drop the (), but really, it's fine, leave it alone, it's obvious what is happening is ok. thanks, greg k-h -- Regards, Reza Snowdon From a2dda399463c20f63306e31b743e0c9c3be2ca3a Mon Sep 17 00:00:00 2001 From: Reza Snowdon r...@mage.me.uk Date: Thu, 27 Feb 2014 17:57:59 + Subject: [PATCH] Staging: speakup: buffers: Fixed a return coding style issue When checking buffers.c with checkpatch.pl it throws the following error: drivers/staging/speakup/buffers.c:58: ERROR: return is not a function, parentheses are not required Fixed error by removing parenthesis. Signed-off-by: Reza Snowdon r...@mage.me.uk --- drivers/staging/speakup/buffers.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/speakup/buffers.c b/drivers/staging/speakup/buffers.c index 382973e..5e6ec13 100644 --- a/drivers/staging/speakup/buffers.c +++ b/drivers/staging/speakup/buffers.c @@ -55,7 +55,7 @@ static int synth_buffer_free(void) int synth_buffer_empty(void) { - return (buff_in == buff_out); + return buff_in == buff_out; } EXPORT_SYMBOL_GPL(synth_buffer_empty); -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: dgap: fix kernel oops on port open
On 02/26/2014 10:30 AM, Dan Carpenter wrote: On Wed, Feb 26, 2014 at 10:18:26AM -0500, Mark Hounschell wrote: This patch addresses the follow error message followed by a kernel oops: dgap: driver does not set tty-port. This will crash the kernel later. Fix the driver It also renames the main function this patch addresses because its name is misleading. Thanks. regards, dan carpenter I'm in the process of running dgap.c through checkpatch and creating another patch set. Before I get too far into it I wanted to get clarification on a couple of things. 1. Should I wait until I know the status of my last patch before I post new ones? 2. There are MANY over 80 char lines warnings. I'm uncertain of the acceptable way to fix some of them. Here is an example of one: while (tail != head) { if (reason IFTLW) { if (ch-ch_tun.un_flags UN_LOW) { ch-ch_tun.un_flags = ~UN_LOW; // everything in this block is over 80 chars if (ch-ch_tun.un_flags UN_ISOPEN) { if ((ch-ch_tun.un_tty-flags (1 TTY_DO_WRITE_WAKEUP)) ch-ch_tun.un_tty-ldisc-ops-write_wakeup) { // DGAP_UNLOCK(ch-ch_lock, lock_flags2); DGAP_UNLOCK(bd-bd_lock, lock_flags); (ch-ch_tun.un_tty-ldisc-ops-write_wakeup)(ch-ch_tun.un_tty); // DGAP_LOCK(bd-bd_lock, lock_flags); DGAP_LOCK(ch-ch_lock, lock_flags2); } wake_up_interruptible(ch-ch_tun.un_tty-write_wait); wake_up_interruptible(ch-ch_tun.un_flags_wait); // end of nasty block } } I figured I'd leave them for the last patch but there are a few that if I wait will show up in one or more of the patches preceding that last one. This one is actually one of them. While fixing up bracket errors with chechpatch -file, chechpatch doesn't like the patch created. Thanks Mark ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] gpu: ipu-v3: Move i.MX IPUv3 core driver out of staging
On Tue, Feb 25, 2014 at 01:09:57PM +0100, Philipp Zabel wrote: Am Dienstag, den 25.02.2014, 12:43 +0100 schrieb Philipp Zabel: The i.MX Image Processing Unit (IPU) contains a number of image processing blocks that sit right in the middle between DRM and V4L2. Some of the modules, such as Display Controller, Processor, and Interface (DC, DP, DI) or CMOS Sensor Interface (CSI) and their FIFOs could be assigned to either framework, but others, such as the dma controller (IDMAC) and image converter (IC) can be used by both. The IPUv3 core driver provides an internal API to access the modules, to be used by both DRM and V4L2 IPUv3 drivers. [...] This one is missing: diff --git a/drivers/staging/imx-drm/imx-hdmi.c b/drivers/staging/imx-drm/imx-hdmi.c index cb4eb76..9aeb863 100644 --- a/drivers/staging/imx-drm/imx-hdmi.c +++ b/drivers/staging/imx-drm/imx-hdmi.c @@ -28,7 +28,8 @@ #include drm/drm_edid.h #include drm/drm_encoder_slave.h -#include ipu-v3/imx-ipu-v3.h +#include video/imx-ipu-v3.h + What do you mean? I can't apply this patch? Totally confused, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] Move IPUv3 core out of staging
On Tue, Feb 25, 2014 at 12:43:40PM +0100, Philipp Zabel wrote: Hi, this series has two small cleanups for the IPUv3 core driver and then moves it from drivers/staging/imx-drm/ipu-v3 to drivers/gpu. In this directory there is already the host1x driver, which serves a similar purpose. I've applied the first 2, I don't understand your follow-up for patch 3, so I just dropped both copies of it... I need an ack from a DRM maintainer before I can take that one anyway. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] gpu: ipu-v3: Move i.MX IPUv3 core driver out of staging
On Thu, Feb 27, 2014 at 12:41:36PM -0800, Greg Kroah-Hartman wrote: On Tue, Feb 25, 2014 at 01:09:57PM +0100, Philipp Zabel wrote: Am Dienstag, den 25.02.2014, 12:43 +0100 schrieb Philipp Zabel: The i.MX Image Processing Unit (IPU) contains a number of image processing blocks that sit right in the middle between DRM and V4L2. Some of the modules, such as Display Controller, Processor, and Interface (DC, DP, DI) or CMOS Sensor Interface (CSI) and their FIFOs could be assigned to either framework, but others, such as the dma controller (IDMAC) and image converter (IC) can be used by both. The IPUv3 core driver provides an internal API to access the modules, to be used by both DRM and V4L2 IPUv3 drivers. [...] This one is missing: diff --git a/drivers/staging/imx-drm/imx-hdmi.c b/drivers/staging/imx-drm/imx-hdmi.c index cb4eb76..9aeb863 100644 --- a/drivers/staging/imx-drm/imx-hdmi.c +++ b/drivers/staging/imx-drm/imx-hdmi.c @@ -28,7 +28,8 @@ #include drm/drm_edid.h #include drm/drm_encoder_slave.h -#include ipu-v3/imx-ipu-v3.h +#include video/imx-ipu-v3.h + What do you mean? I can't apply this patch? I understand it as: Please squash this hunk changing the #include into patch 3. But note that I don't know anything about the stuff Philipp is working on. Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: dgap: fix kernel oops on port open
On Thu, Feb 27, 2014 at 03:39:08PM -0500, Mark Hounschell wrote: On 02/26/2014 10:30 AM, Dan Carpenter wrote: On Wed, Feb 26, 2014 at 10:18:26AM -0500, Mark Hounschell wrote: This patch addresses the follow error message followed by a kernel oops: dgap: driver does not set tty-port. This will crash the kernel later. Fix the driver It also renames the main function this patch addresses because its name is misleading. Thanks. regards, dan carpenter I'm in the process of running dgap.c through checkpatch and creating another patch set. Before I get too far into it I wanted to get clarification on a couple of things. Please line-wrap your emails :( 1. Should I wait until I know the status of my last patch before I post new ones? I just now applied it :) 2. There are MANY over 80 char lines warnings. I'm uncertain of the acceptable way to fix some of them. Here is an example of one: while (tail != head) { if (reason IFTLW) { if (ch-ch_tun.un_flags UN_LOW) { ch-ch_tun.un_flags = ~UN_LOW; // everything in this block is over 80 chars if (ch-ch_tun.un_flags UN_ISOPEN) { if ((ch-ch_tun.un_tty-flags (1 TTY_DO_WRITE_WAKEUP)) ch-ch_tun.un_tty-ldisc-ops-write_wakeup) { // DGAP_UNLOCK(ch-ch_lock, lock_flags2); DGAP_UNLOCK(bd-bd_lock, lock_flags); (ch-ch_tun.un_tty-ldisc-ops-write_wakeup)(ch-ch_tun.un_tty); // DGAP_LOCK(bd-bd_lock, lock_flags); DGAP_LOCK(ch-ch_lock, lock_flags2); } wake_up_interruptible(ch-ch_tun.un_tty-write_wait); wake_up_interruptible(ch-ch_tun.un_flags_wait); // end of nasty block } } I figured I'd leave them for the last patch but there are a few that if I wait will show up in one or more of the patches preceding that last one. This one is actually one of them. While fixing up bracket errors with chechpatch -file, chechpatch doesn't like the patch created. For major logic chunks like this, I'd recommend just leaving them over 80 columns for now, until they can be refactored into something more readable later. Don't try to just trail characters from the 70-80 character columns to make the tool happy, that's not a good idea. Also, usually some of the if statement logic can be reversed to get the code to be moved to the right easier, but maybe not. good luck! greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: dgap: fix kernel oops on port open
On Thu, Feb 27, 2014 at 03:39:08PM -0500, Mark Hounschell wrote: On 02/26/2014 10:30 AM, Dan Carpenter wrote: On Wed, Feb 26, 2014 at 10:18:26AM -0500, Mark Hounschell wrote: This patch addresses the follow error message followed by a kernel oops: dgap: driver does not set tty-port. This will crash the kernel later. Fix the driver It also renames the main function this patch addresses because its name is misleading. Thanks. regards, dan carpenter I'm in the process of running dgap.c through checkpatch and creating another patch set. Before I get too far into it I wanted to get clarification on a couple of things. 1. Should I wait until I know the status of my last patch before I post new ones? Just assume they will be applied. If not you will have to redo. 2. There are MANY over 80 char lines warnings. I'm uncertain of the acceptable way to fix some of them. Here is an example of one: while (tail != head) { if (reason IFTLW) { if (ch-ch_tun.un_flags UN_LOW) { ch-ch_tun.un_flags = ~UN_LOW; // everything in this block is over 80 chars if (ch-ch_tun.un_flags UN_ISOPEN) { if ((ch-ch_tun.un_tty-flags (1 TTY_DO_WRITE_WAKEUP)) ch-ch_tun.un_tty-ldisc-ops-write_wakeup) { // DGAP_UNLOCK(ch-ch_lock, lock_flags2); DGAP_UNLOCK(bd-bd_lock, lock_flags); (ch-ch_tun.un_tty-ldisc-ops-write_wakeup)(ch-ch_tun.un_tty); // DGAP_LOCK(bd-bd_lock, lock_flags); DGAP_LOCK(ch-ch_lock, lock_flags2); } wake_up_interruptible(ch-ch_tun.un_tty-write_wait); wake_up_interruptible(ch-ch_tun.un_flags_wait); // end of nasty block } } I figured I'd leave them for the last patch but there are a few that if I wait will show up in one or more of the patches preceding that last one. This one is actually one of them. While fixing up bracket errors with chechpatch -file, chechpatch doesn't like the patch created. Break it up into separate functions. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 1/6] imx-drm: ipu-dmfc: Check 'dmfc' pointer first
On Wed, Feb 26, 2014 at 08:53:39PM -0300, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com Fix the following static checker warning: drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c:164 ipu_dmfc_setup_channel() warn: variable dereferenced before check 'dmfc' (see line 157) Reported-by: Dan Carpenter dan.carpen...@oracle.com Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- Changes since v3: - None Changes since v2: - None Changes since v1: - Check 'dmfc' prior to setting the priv pointer drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c b/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c index 98070dd..ce152d9 100644 --- a/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c +++ b/drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c @@ -154,16 +154,18 @@ EXPORT_SYMBOL_GPL(ipu_dmfc_disable_channel); static int ipu_dmfc_setup_channel(struct dmfc_channel *dmfc, int slots, int segment, int burstsize) { - struct ipu_dmfc_priv *priv = dmfc-priv; + struct ipu_dmfc_priv *priv; u32 val, field; + if (!dmfc) + return -EINVAL; How can dmfc ever be NULL? You control what values you pass to it, why check it again? Have you hit this problem in the wild? If so, shouldn't the root cause be fixed and not papered over here? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/6] imx-drm: imx-ldb: Use snprintf()
On Wed, Feb 26, 2014 at 08:53:41PM -0300, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com Use snprintf() in order to fix the following static checker warning: drivers/staging/imx-drm/imx-ldb.c:340 imx_ldb_get_clk() error: format string overflow. buf_size: 16 length: 18 probably 18 is theory and not real life, but 16 is based on theory as well. Reported-by: Dan Carpenter dan.carpen...@oracle.com Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- Changes since v3: - Fix Subject - Also use snprintf in the other ocurrence Changes since v2: - Use snprintf as suggested by Russell Changes since v1: - None drivers/staging/imx-drm/imx-ldb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c index abf8517..daa54df 100644 --- a/drivers/staging/imx-drm/imx-ldb.c +++ b/drivers/staging/imx-drm/imx-ldb.c @@ -334,12 +334,12 @@ static int imx_ldb_get_clk(struct imx_ldb *ldb, int chno) { char clkname[16]; - sprintf(clkname, di%d, chno); + snprintf(clkname, sizeof(clkname), di%d, chno); Are you sure this can not overflow as well? Strings in C are nasty... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/6] imx-drm: imx-ldb: Use snprintf()
On Thu, Feb 27, 2014 at 02:54:43PM -0800, Greg KH wrote: On Wed, Feb 26, 2014 at 08:53:41PM -0300, Fabio Estevam wrote: diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c index abf8517..daa54df 100644 --- a/drivers/staging/imx-drm/imx-ldb.c +++ b/drivers/staging/imx-drm/imx-ldb.c @@ -334,12 +334,12 @@ static int imx_ldb_get_clk(struct imx_ldb *ldb, int chno) { char clkname[16]; - sprintf(clkname, di%d, chno); + snprintf(clkname, sizeof(clkname), di%d, chno); Are you sure this can not overflow as well? Strings in C are nasty... Can you indicate how this would overflow? * snprintf - Format a string and place it in a buffer ... * * The return value is the number of characters which would be * generated for the given input, excluding the trailing null, * as per ISO C99. If the return is greater than or equal to * @size, the resulting string is truncated. Now, there's several layers of protection here. The first obvious one is using snprintf() instead of sprintf() which wouldn't know the buffer size. The second one is something that the static checker can't know, and that is for existing uses, chno is limited to zero or one: ret = of_property_read_u32(child, reg, i); if (ret || i 0 || i 1) return -EINVAL; Of course, that could change in the future, but is unlikely to change significantly - and probably not much beyond two digit decimal. So, the conversion from sprintf() to snprintf() is technically moot, since it can only overflow if checks are removed elsewhere in this code. So really, this is just to shut the static checker up about something that is a non-problem. But there's another problem - and it's one of community process. The reason these patches got raised is because another kernel maintainer requested these errors to get fixed, so we're probably heading to the situation where one maintainer wants them fixed and another doesn't... I have no opinion either way. Personally, I'd have used snprintf() here to start with so at least stack corruption can't happen, and the worst that can happen is the string gets truncated, and the following clk lookup fails, resulting in an error returned during the driver probe. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3] staging/usbip: add new uapi header usbip.h
usbip userspace has duplicated enum definition to report usbip device status maintained by the kernel. Adding an usbip uapi header file will define the kernel - userspace interface for this device status. Shuah Khan (3): staging/usbip: add uapi header to export usbip kernel interfaces staging/usbip: change usbip to include new uapi usbip.h staging/usbip: change usbip userspace to include new uapi usbip.h drivers/staging/usbip/usbip_common.h | 19 ++ .../staging/usbip/userspace/libsrc/usbip_common.h | 20 +++ include/uapi/linux/Kbuild |1 + include/uapi/linux/usbip.h | 26 4 files changed, 32 insertions(+), 34 deletions(-) create mode 100644 include/uapi/linux/usbip.h -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging/usbip: add uapi header to export usbip kernel interfaces
usbip userspace has duplicated enum definition to report usbip device status maintained by the kernel. Adding an usbip uapi header file will define the kernel - userspace interface for this device status. Signed-off-by: Shuah Khan shuah...@samsung.com --- include/uapi/linux/Kbuild |1 + include/uapi/linux/usbip.h | 26 ++ 2 files changed, 27 insertions(+) create mode 100644 include/uapi/linux/usbip.h diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild index 3ce25b5..4fb4800 100644 --- a/include/uapi/linux/Kbuild +++ b/include/uapi/linux/Kbuild @@ -393,6 +393,7 @@ header-y += un.h header-y += unistd.h header-y += unix_diag.h header-y += usbdevice_fs.h +header-y += usbip.h header-y += utime.h header-y += utsname.h header-y += uuid.h diff --git a/include/uapi/linux/usbip.h b/include/uapi/linux/usbip.h new file mode 100644 index 000..fa5db30 --- /dev/null +++ b/include/uapi/linux/usbip.h @@ -0,0 +1,26 @@ +/* + * usbip.h + * + * USBIP uapi defines and function prototypes etc. +*/ + +#ifndef _UAPI_LINUX_USBIP_H +#define _UAPI_LINUX_USBIP_H + +/* usbip device status - exported in usbip device sysfs status */ +enum usbip_device_status { + /* sdev is available. */ + SDEV_ST_AVAILABLE = 0x01, + /* sdev is now used. */ + SDEV_ST_USED, + /* sdev is unusable because of a fatal error. */ + SDEV_ST_ERROR, + + /* vdev does not connect a remote device. */ + VDEV_ST_NULL, + /* vdev is used, but the USB address is not assigned yet */ + VDEV_ST_NOTASSIGNED, + VDEV_ST_USED, + VDEV_ST_ERROR +}; +#endif /* _UAPI_LINUX_USBIP_H */ -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: cxt1e1: remove unused variable
Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/cxt1e1/hwprobe.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/cxt1e1/hwprobe.c b/drivers/staging/cxt1e1/hwprobe.c index 02b4f8f..07de83f 100644 --- a/drivers/staging/cxt1e1/hwprobe.c +++ b/drivers/staging/cxt1e1/hwprobe.c @@ -49,10 +49,9 @@ show_two (hdw_info_t *hi, int brdno) ci_t *ci; struct pci_dev *pdev; char *bid; -char *bp, banner[80]; +char banner[80]; charsn[6]; -bp = banner; memset (banner, 0, 80); /* clear print buffer */ ci = (ci_t *)(netdev_priv(hi-ndev)); -- 1.7.9.5 --- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: sb105x: b_pci_mp.c: fix for non-member access
'struct tty_struct’ has no member named ‘low_latency’ Signed-off-by: Kumar Amit Mehta gmate.a...@gmail.com --- drivers/staging/sb105x/sb_pci_mp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/sb105x/sb_pci_mp.c b/drivers/staging/sb105x/sb_pci_mp.c index c9d6ee3..5687d6c 100644 --- a/drivers/staging/sb105x/sb_pci_mp.c +++ b/drivers/staging/sb105x/sb_pci_mp.c @@ -898,7 +898,7 @@ static int mp_set_info(struct sb_uart_state *state, struct serial_struct *newinf state-closing_wait= closing_wait; port-fifosize = new_serial.xmit_fifo_size; if (state-info-tty) - state-info-tty-low_latency = + state-info-tty-port-low_latency = (port-flags UPF_LOW_LATENCY) ? 1 : 0; check_and_exit: @@ -1571,7 +1571,7 @@ static int mp_open(struct tty_struct *tty, struct file *filp) mtpt = (struct mp_port *)state-port; tty-driver_data = state; - tty-low_latency = (state-port-flags UPF_LOW_LATENCY) ? 1 : 0; + tty-port-low_latency = (state-port-flags UPF_LOW_LATENCY) ? 1 : 0; tty-alt_speed = 0; state-info-tty = tty; -- 1.8.5.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [patch 25/26] x86: hyperv: Cleanup the irq mess
-Original Message- From: Thomas Gleixner [mailto:t...@linutronix.de] Sent: Tuesday, February 25, 2014 11:10 AM To: KY Srinivasan Cc: LKML; Ingo Molnar; Peter Zijlstra; x86; Greg Kroah-Hartman; linuxdrivers Subject: RE: [patch 25/26] x86: hyperv: Cleanup the irq mess On Tue, 25 Feb 2014, KY Srinivasan wrote: -Original Message- From: Thomas Gleixner [mailto:t...@linutronix.de] Sent: Sunday, February 23, 2014 1:40 PM To: LKML Cc: Ingo Molnar; Peter Zijlstra; x86; KY Srinivasan; Greg Kroah-Hartman; linuxdrivers Subject: [patch 25/26] x86: hyperv: Cleanup the irq mess The vmbus/hyperv interrupt handling is another complete trainwreck and probably the worst of all currently in tree. If CONFIG_HYPERV=y then the interrupt delivery to the vmbus happens via the direct HYPERVISOR_CALLBACK_VECTOR. So far so good, but: The driver requests first a normal device interrupt. The only reason to do so is to increment the interrupt stats of that device interrupt. We have proper accounting mechanisms for direct vectors, but of course it's too much effort to add that 5 lines of code. Aside of that the alloc_intr_gate() is not protected against reallocation which makes module reload impossible. If CONFIG_HYPERV=n then the vmbus request a regular device interrupt via request_irq() and installs it's own private flow handler. Of course this lacks any explanation why it can't use the standard flow handler or the existing handle_percpu_irq handler. Solution to the problem is simple to rip out the whole mess and implement it correctly. Thomas, Thank you for cleaning up this code. When CONFIG_HYPERV== n, none of the VMBUS code is active. The special case can go away as you have noted. So, if CONFIG_HYPERV=n then you do not need the request_irq() fallback at all, right? Somehow I missed the HYPERV dependency of the VMBUS stuff That makes stuff even simpler as we can get rid of those extra cases including the extra flow handler. New patch below. Thanks Thomas. Acked-by: K. Y. Srinivasan k...@microsoft.com Thanks, tglx --- arch/x86/include/asm/mshyperv.h |4 +- arch/x86/kernel/cpu/mshyperv.c | 78 - --- drivers/hv/vmbus_drv.c | 39 ++-- 3 files changed, 47 insertions(+), 74 deletions(-) Index: tip/arch/x86/include/asm/mshyperv.h == = --- tip.orig/arch/x86/include/asm/mshyperv.h +++ tip/arch/x86/include/asm/mshyperv.h @@ -2,6 +2,7 @@ #define _ASM_X86_MSHYPER_H #include linux/types.h +#include linux/interrupt.h #include asm/hyperv.h struct ms_hyperv_info { @@ -16,6 +17,7 @@ void hyperv_callback_vector(void); #define trace_hyperv_callback_vector hyperv_callback_vector #endif void hyperv_vector_handler(struct pt_regs *regs); -void hv_register_vmbus_handler(int irq, irq_handler_t handler); +int hv_setup_vmbus_irq(int irq, irq_handler_t handler, void *dev_id); +void hv_remove_vmbus_irq(int irq, void *dev_id); #endif Index: tip/arch/x86/kernel/cpu/mshyperv.c == = --- tip.orig/arch/x86/kernel/cpu/mshyperv.c +++ tip/arch/x86/kernel/cpu/mshyperv.c @@ -17,6 +17,7 @@ #include linux/hardirq.h #include linux/efi.h #include linux/interrupt.h +#include linux/irq.h #include asm/processor.h #include asm/hypervisor.h #include asm/hyperv.h @@ -30,6 +31,45 @@ struct ms_hyperv_info ms_hyperv; EXPORT_SYMBOL_GPL(ms_hyperv); +#ifdef CONFIG_HYPERV +static irq_handler_t *vmbus_handler; + +void hyperv_vector_handler(struct pt_regs *regs) { + struct pt_regs *old_regs = set_irq_regs(regs); + + irq_enter(); + exit_idle(); + + inc_irq_stat(irq_hv_callback_count); + if (vmbus_handler) + vmbus_handler(); + + irq_exit(); + set_irq_regs(old_regs); +} + +int hv_setup_vmbus_irq(int irq, irq_handler_t *handler, void *dev_id) { + vmbus_handler = handler; + /* + * Setup the IDT for hypervisor callback. Prevent reallocation + * at module reload. + */ + if (!test_bit(HYPERVISOR_CALLBACK_VECTOR, used_vectors)) + alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, + hyperv_callback_vector); +} + +void hv_remove_vmbus_irq(unsigned int irq, void *dev_id) { + /* We have no way to deallocate the interrupt gate */ + vmbus_handler = NULL; +} +EXPORT_SYMBOL_GPL(hv_setup_vmbus_irq); +EXPORT_SYMBOL_GPL(hv_remove_vmbus_irq); +#endif + static uint32_t __init ms_hyperv_platform(void) { u32 eax; @@ -113,41 +153,3 @@ const __refconst struct hypervisor_x86 x .init_platform = ms_hyperv_init_platform, }; EXPORT_SYMBOL(x86_hyper_ms_hyperv); - -#if IS_ENABLED(CONFIG_HYPERV) -static int
[PATCH RESEND] x86, hyperv: bypass the timer_irq_works() check
This patch bypass the timer_irq_works() check for hyperv guest since: - It was guaranteed to work. - timer_irq_works() may fail sometime due to the lpj calibration were inaccurate in a hyperv guest or a buggy host. In the future, we should get the tsc frequency from hypervisor and use preset lpj instead. Cc: K. Y. Srinivasan k...@microsoft.com Cc: Haiyang Zhang haiya...@microsoft.com Cc: sta...@vger.kernel.org Acked-by: K. Y. Srinivasan k...@microsoft.com Signed-off-by: Jason Wang jasow...@redhat.com --- arch/x86/kernel/cpu/mshyperv.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c index 9f7ca26..832d05a 100644 --- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c @@ -26,6 +26,7 @@ #include asm/irq_regs.h #include asm/i8259.h #include asm/apic.h +#include asm/timer.h struct ms_hyperv_info ms_hyperv; EXPORT_SYMBOL_GPL(ms_hyperv); @@ -105,6 +106,11 @@ static void __init ms_hyperv_init_platform(void) if (ms_hyperv.features HV_X64_MSR_TIME_REF_COUNT_AVAILABLE) clocksource_register_hz(hyperv_cs, NSEC_PER_SEC/100); + +#ifdef CONFIG_X86_IO_APIC + no_timer_check = 1; +#endif + } const __refconst struct hypervisor_x86 x86_hyper_ms_hyperv = { -- 1.8.3.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/6] imx-drm: imx-ldb: Use snprintf()
On Thu, Feb 27, 2014 at 11:44:38PM +, Russell King - ARM Linux wrote: On Thu, Feb 27, 2014 at 02:54:43PM -0800, Greg KH wrote: On Wed, Feb 26, 2014 at 08:53:41PM -0300, Fabio Estevam wrote: diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c index abf8517..daa54df 100644 --- a/drivers/staging/imx-drm/imx-ldb.c +++ b/drivers/staging/imx-drm/imx-ldb.c @@ -334,12 +334,12 @@ static int imx_ldb_get_clk(struct imx_ldb *ldb, int chno) { char clkname[16]; - sprintf(clkname, di%d, chno); + snprintf(clkname, sizeof(clkname), di%d, chno); Are you sure this can not overflow as well? Strings in C are nasty... Can you indicate how this would overflow? * snprintf - Format a string and place it in a buffer ... * * The return value is the number of characters which would be * generated for the given input, excluding the trailing null, * as per ISO C99. If the return is greater than or equal to * @size, the resulting string is truncated. Ick, I forgot that snprintf() takes into account the trailing \0, you are right, this is safe. Fabio, can you resend this please? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/6] imx-drm: imx-ldb: Use snprintf()
On Thu, Feb 27, 2014 at 11:44:38PM +, Russell King - ARM Linux wrote: On Thu, Feb 27, 2014 at 02:54:43PM -0800, Greg KH wrote: On Wed, Feb 26, 2014 at 08:53:41PM -0300, Fabio Estevam wrote: diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c index abf8517..daa54df 100644 --- a/drivers/staging/imx-drm/imx-ldb.c +++ b/drivers/staging/imx-drm/imx-ldb.c @@ -334,12 +334,12 @@ static int imx_ldb_get_clk(struct imx_ldb *ldb, int chno) { char clkname[16]; - sprintf(clkname, di%d, chno); + snprintf(clkname, sizeof(clkname), di%d, chno); Are you sure this can not overflow as well? Strings in C are nasty... Can you indicate how this would overflow? * snprintf - Format a string and place it in a buffer ... * * The return value is the number of characters which would be * generated for the given input, excluding the trailing null, * as per ISO C99. If the return is greater than or equal to * @size, the resulting string is truncated. Now, there's several layers of protection here. The first obvious one is using snprintf() instead of sprintf() which wouldn't know the buffer size. The second one is something that the static checker can't know, and that is for existing uses, chno is limited to zero or one: ret = of_property_read_u32(child, reg, i); if (ret || i 0 || i 1) return -EINVAL; If you have the cross function database built then Smatch wouldn't have complained. But this driver is outside of my normal build so I didn't have that. Of course, my first impression was that this wasn't a real bug. But these things are easy to solve and people who don't use snprintf() should be more careful about picking buffer sizes so I don't care about harrassing people with false positives. ;) If the code were old and outside of staging then I wouldn't have mentioned the warning. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 1/6] imx-drm: ipu-dmfc: Check 'dmfc' pointer first
On Thu, Feb 27, 2014 at 11:54:23PM +, Russell King - ARM Linux wrote: On Thu, Feb 27, 2014 at 02:52:34PM -0800, Greg KH wrote: How can dmfc ever be NULL? You control what values you pass to it, why check it again? Indeed. This is actually a case where the static checker warning is ambiguous. What's triggered it is the dmfc test for NULL after it's been used. It doesn't necessarily mean that the test should be moved earlier. Btw, this is another case where Smatch wouldn't have generated a warning if I had had the cross function database set up. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/5] staging: cxt1e1: remove space between function name and parenthesis
clean up checkpatch.pl warnings: WARNING: space prohibited between function name and open parenthesis '(' Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/cxt1e1/hwprobe.c | 120 +++--- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/drivers/staging/cxt1e1/hwprobe.c b/drivers/staging/cxt1e1/hwprobe.c index 07de83f..5e93580 100644 --- a/drivers/staging/cxt1e1/hwprobe.c +++ b/drivers/staging/cxt1e1/hwprobe.c @@ -36,15 +36,15 @@ extern int error_flag; extern int drvr_state; /* forward references */ -voidc4_stopwd (ci_t *); -struct net_device * __init c4_add_dev (hdw_info_t *, int, unsigned long, unsigned long, int, int); +voidc4_stopwd(ci_t *); +struct net_device * __init c4_add_dev(hdw_info_t *, int, unsigned long, unsigned long, int, int); struct s_hdw_info hdw_info[MAX_BOARDS]; void__init -show_two (hdw_info_t *hi, int brdno) +show_two(hdw_info_t *hi, int brdno) { ci_t *ci; struct pci_dev *pdev; @@ -52,26 +52,26 @@ show_two (hdw_info_t *hi, int brdno) char banner[80]; charsn[6]; -memset (banner, 0, 80); /* clear print buffer */ +memset(banner, 0, 80); /* clear print buffer */ ci = (ci_t *)(netdev_priv(hi-ndev)); -bid = sbeid_get_bdname (ci); +bid = sbeid_get_bdname(ci); switch (hi-promfmt) { case PROM_FORMAT_TYPE1: -memcpy (sn, (FLD_TYPE1 *) (hi-mfg_info.pft1.Serial), 6); +memcpy(sn, (FLD_TYPE1 *)(hi-mfg_info.pft1.Serial), 6); break; case PROM_FORMAT_TYPE2: -memcpy (sn, (FLD_TYPE2 *) (hi-mfg_info.pft2.Serial), 6); +memcpy(sn, (FLD_TYPE2 *)(hi-mfg_info.pft2.Serial), 6); break; default: -memset (sn, 0, 6); +memset(sn, 0, 6); break; } -sprintf (banner, %s: %s S/N %06X, MUSYCC Rev %02X, - hi-devname, bid, - ((sn[3] 16) 0xff) | +sprintf(banner, %s: %s S/N %06X, MUSYCC Rev %02X, +hi-devname, bid, +((sn[3] 16) 0xff) | ((sn[4] 8) 0x00ff00) | (sn[5] 0xff), (u_int8_t) hi-revid[0]); @@ -82,20 +82,20 @@ show_two (hdw_info_t *hi, int brdno) pr_info(%s: %s at v/p=%lx/%lx (%02x:%02x.%x) irq %d\n, hi-devname, MUSYCC, (unsigned long) hi-addr_mapped[0], hi-addr[0], -hi-pci_busno, (u_int8_t) PCI_SLOT (pdev-devfn), -(u_int8_t) PCI_FUNC (pdev-devfn), pdev-irq); +hi-pci_busno, (u_int8_t)PCI_SLOT(pdev-devfn), +(u_int8_t)PCI_FUNC(pdev-devfn), pdev-irq); pdev = hi-pdev[1]; pr_info(%s: %s at v/p=%lx/%lx (%02x:%02x.%x) irq %d\n, hi-devname, EBUS , (unsigned long) hi-addr_mapped[1], hi-addr[1], -hi-pci_busno, (u_int8_t) PCI_SLOT (pdev-devfn), -(u_int8_t) PCI_FUNC (pdev-devfn), pdev-irq); +hi-pci_busno, (u_int8_t)PCI_SLOT(pdev-devfn), +(u_int8_t)PCI_FUNC(pdev-devfn), pdev-irq); } void__init -hdw_sn_get (hdw_info_t *hi, int brdno) +hdw_sn_get(hdw_info_t *hi, int brdno) { /* obtain hardware EEPROM information */ longaddr; @@ -103,7 +103,7 @@ hdw_sn_get (hdw_info_t *hi, int brdno) addr = (long) hi-addr_mapped[1] + EEPROM_OFFSET; /* read EEPROM with largest known format size... */ -pmc_eeprom_read_buffer (addr, 0, (char *) hi-mfg_info.data, sizeof (FLD_TYPE2)); +pmc_eeprom_read_buffer(addr, 0, (char *)hi-mfg_info.data, sizeof(FLD_TYPE2)); #if 0 { @@ -133,7 +133,7 @@ hdw_sn_get (hdw_info_t *hi, int brdno) hi-mfg_info.Serial[5]); #endif -if ((hi-promfmt = pmc_verify_cksum (hi-mfg_info.data)) == PROM_FORMAT_Unk) +if ((hi-promfmt = pmc_verify_cksum(hi-mfg_info.data)) == PROM_FORMAT_Unk) { /* bad crc, data is suspect */ if (cxt1e1_log_level = LOG_WARN) @@ -145,7 +145,7 @@ hdw_sn_get (hdw_info_t *hi, int brdno) void__init -prep_hdw_info (void) +prep_hdw_info(void) { hdw_info_t *hi; int i; @@ -165,7 +165,7 @@ prep_hdw_info (void) } void -cleanup_ioremap (void) +cleanup_ioremap(void) { hdw_info_t *hi; int i; @@ -176,14 +176,14 @@ cleanup_ioremap (void) break; if (hi-addr_mapped[0]) { -iounmap ((void *) (hi-addr_mapped[0])); -release_mem_region ((long) hi-addr[0], hi-len[0]); +iounmap((void *)(hi-addr_mapped[0])); +release_mem_region((long) hi-addr[0], hi-len[0]); hi-addr_mapped[0] = 0; } if (hi-addr_mapped[1]) { -iounmap ((void *) (hi-addr_mapped[1])); -release_mem_region ((long) hi-addr[1], hi-len[1]); +iounmap((void *)(hi-addr_mapped[1])); +release_mem_region((long) hi-addr[1], hi-len[1]); hi-addr_mapped[1] = 0;
[PATCH 2/5] staging: cxt1e1: Fix no spaces at the start of a line in hwprobe.c
clean up checkpatch.pl warnings: WARNING: please no spaces at the start of a line in Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/cxt1e1/hwprobe.c | 585 +++--- 1 file changed, 293 insertions(+), 292 deletions(-) diff --git a/drivers/staging/cxt1e1/hwprobe.c b/drivers/staging/cxt1e1/hwprobe.c index 5e93580..85040bb 100644 --- a/drivers/staging/cxt1e1/hwprobe.c +++ b/drivers/staging/cxt1e1/hwprobe.c @@ -43,347 +43,348 @@ struct net_device * __init c4_add_dev(hdw_info_t *, int, unsigned long, unsigned struct s_hdw_info hdw_info[MAX_BOARDS]; -void__init +void __init show_two(hdw_info_t *hi, int brdno) { -ci_t *ci; -struct pci_dev *pdev; -char *bid; -char banner[80]; -charsn[6]; - -memset(banner, 0, 80); /* clear print buffer */ - -ci = (ci_t *)(netdev_priv(hi-ndev)); -bid = sbeid_get_bdname(ci); -switch (hi-promfmt) -{ -case PROM_FORMAT_TYPE1: -memcpy(sn, (FLD_TYPE1 *)(hi-mfg_info.pft1.Serial), 6); -break; -case PROM_FORMAT_TYPE2: -memcpy(sn, (FLD_TYPE2 *)(hi-mfg_info.pft2.Serial), 6); -break; -default: -memset(sn, 0, 6); -break; -} - -sprintf(banner, %s: %s S/N %06X, MUSYCC Rev %02X, -hi-devname, bid, -((sn[3] 16) 0xff) | - ((sn[4] 8) 0x00ff00) | - (sn[5] 0xff), - (u_int8_t) hi-revid[0]); - -pr_info(%s\n, banner); - -pdev = hi-pdev[0]; -pr_info(%s: %s at v/p=%lx/%lx (%02x:%02x.%x) irq %d\n, -hi-devname, MUSYCC, -(unsigned long) hi-addr_mapped[0], hi-addr[0], -hi-pci_busno, (u_int8_t)PCI_SLOT(pdev-devfn), -(u_int8_t)PCI_FUNC(pdev-devfn), pdev-irq); - -pdev = hi-pdev[1]; -pr_info(%s: %s at v/p=%lx/%lx (%02x:%02x.%x) irq %d\n, -hi-devname, EBUS , -(unsigned long) hi-addr_mapped[1], hi-addr[1], -hi-pci_busno, (u_int8_t)PCI_SLOT(pdev-devfn), -(u_int8_t)PCI_FUNC(pdev-devfn), pdev-irq); + ci_t *ci; + struct pci_dev *pdev; + char *bid; + char banner[80]; + charsn[6]; + + /* clear print buffer */ + memset(banner, 0, 80); + + ci = (ci_t *)(netdev_priv(hi-ndev)); + bid = sbeid_get_bdname(ci); + switch (hi-promfmt) + { + case PROM_FORMAT_TYPE1: + memcpy(sn, (FLD_TYPE1 *)(hi-mfg_info.pft1.Serial), 6); + break; + case PROM_FORMAT_TYPE2: + memcpy(sn, (FLD_TYPE2 *)(hi-mfg_info.pft2.Serial), 6); + break; + default: + memset(sn, 0, 6); + break; + } + + sprintf(banner, %s: %s S/N %06X, MUSYCC Rev %02X, + hi-devname, bid, + ((sn[3] 16) 0xff) | + ((sn[4] 8) 0x00ff00) | + (sn[5] 0xff), + (u_int8_t) hi-revid[0]); + + pr_info(%s\n, banner); + + pdev = hi-pdev[0]; + pr_info(%s: %s at v/p=%lx/%lx (%02x:%02x.%x) irq %d\n, + hi-devname, MUSYCC, + (unsigned long) hi-addr_mapped[0], hi-addr[0], + hi-pci_busno, (u_int8_t) PCI_SLOT(pdev-devfn), + (u_int8_t) PCI_FUNC(pdev-devfn), pdev-irq); + + pdev = hi-pdev[1]; + pr_info(%s: %s at v/p=%lx/%lx (%02x:%02x.%x) irq %d\n, + hi-devname, EBUS , + (unsigned long) hi-addr_mapped[1], hi-addr[1], + hi-pci_busno, (u_int8_t) PCI_SLOT(pdev-devfn), + (u_int8_t) PCI_FUNC(pdev-devfn), pdev-irq); } -void__init +void __init hdw_sn_get(hdw_info_t *hi, int brdno) { -/* obtain hardware EEPROM information */ -longaddr; + /* obtain hardware EEPROM information */ + longaddr; -addr = (long) hi-addr_mapped[1] + EEPROM_OFFSET; + addr = (long) hi-addr_mapped[1] + EEPROM_OFFSET; -/* read EEPROM with largest known format size... */ -pmc_eeprom_read_buffer(addr, 0, (char *)hi-mfg_info.data, sizeof(FLD_TYPE2)); + /* read EEPROM with largest known format size... */ + pmc_eeprom_read_buffer(addr, 0, (char *)hi-mfg_info.data, sizeof(FLD_TYPE2)); #if 0 -{ -unsigned char *ucp = (unsigned char *) hi-mfg_info.data; - -pr_info(eeprom[00]: %02x %02x %02x %02x %02x %02x %02x %02x\n, -*(ucp + 0), *(ucp + 1), *(ucp + 2), *(ucp + 3), *(ucp + 4), *(ucp + 5), *(ucp + 6), *(ucp + 7)); -pr_info(eeprom[08]: %02x %02x %02x %02x %02x %02x %02x %02x\n, -*(ucp + 8), *(ucp + 9), *(ucp + 10), *(ucp + 11), *(ucp + 12), *(ucp + 13), *(ucp + 14), *(ucp + 15)); -pr_info(eeprom[16]: %02x %02x %02x %02x %02x %02x %02x %02x\n, -*(ucp + 16), *(ucp + 17), *(ucp + 18), *(ucp + 19), *(ucp + 20), *(ucp + 21), *(ucp + 22), *(ucp + 23)); -
[PATCH 3/5] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c
clean up checkpatch.pl warnings: WARNING: Line length over 80 characters Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/cxt1e1/hwprobe.c | 45 -- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/drivers/staging/cxt1e1/hwprobe.c b/drivers/staging/cxt1e1/hwprobe.c index 85040bb..d6ccbd9 100644 --- a/drivers/staging/cxt1e1/hwprobe.c +++ b/drivers/staging/cxt1e1/hwprobe.c @@ -37,7 +37,8 @@ extern int drvr_state; /* forward references */ voidc4_stopwd(ci_t *); -struct net_device * __init c4_add_dev(hdw_info_t *, int, unsigned long, unsigned long, int, int); +struct net_device * __init c4_add_dev(hdw_info_t *, int, unsigned long, + unsigned long, int, int); struct s_hdw_info hdw_info[MAX_BOARDS]; @@ -104,24 +105,31 @@ hdw_sn_get(hdw_info_t *hi, int brdno) addr = (long) hi-addr_mapped[1] + EEPROM_OFFSET; /* read EEPROM with largest known format size... */ - pmc_eeprom_read_buffer(addr, 0, (char *)hi-mfg_info.data, sizeof(FLD_TYPE2)); + pmc_eeprom_read_buffer(addr, 0, (char *)hi-mfg_info.data, + sizeof(FLD_TYPE2)); #if 0 { unsigned char *ucp = (unsigned char *) hi-mfg_info.data; pr_info(eeprom[00]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 0), *(ucp + 1), *(ucp + 2), *(ucp + 3), *(ucp + 4), *(ucp + 5), *(ucp + 6), *(ucp + 7)); + *(ucp + 0), *(ucp + 1), *(ucp + 2), *(ucp + 3), + *(ucp + 4), *(ucp + 5), *(ucp + 6), *(ucp + 7)); pr_info(eeprom[08]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 8), *(ucp + 9), *(ucp + 10), *(ucp + 11), *(ucp + 12), *(ucp + 13), *(ucp + 14), *(ucp + 15)); + *(ucp + 8), *(ucp + 9), *(ucp + 10), *(ucp + 11), + *(ucp + 12), *(ucp + 13), *(ucp + 14), *(ucp + 15)); pr_info(eeprom[16]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 16), *(ucp + 17), *(ucp + 18), *(ucp + 19), *(ucp + 20), *(ucp + 21), *(ucp + 22), *(ucp + 23)); + *(ucp + 16), *(ucp + 17), *(ucp + 18), *(ucp + 19), + *(ucp + 20), *(ucp + 21), *(ucp + 22), *(ucp + 23)); pr_info(eeprom[24]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 24), *(ucp + 25), *(ucp + 26), *(ucp + 27), *(ucp + 28), *(ucp + 29), *(ucp + 30), *(ucp + 31)); + *(ucp + 24), *(ucp + 25), *(ucp + 26), *(ucp + 27), + *(ucp + 28), *(ucp + 29), *(ucp + 30), *(ucp + 31)); pr_info(eeprom[32]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 32), *(ucp + 33), *(ucp + 34), *(ucp + 35), *(ucp + 36), *(ucp + 37), *(ucp + 38), *(ucp + 39)); + *(ucp + 32), *(ucp + 33), *(ucp + 34), *(ucp + 35), + *(ucp + 36), *(ucp + 37), *(ucp + 38), *(ucp + 39)); pr_info(eeprom[40]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 40), *(ucp + 41), *(ucp + 42), *(ucp + 43), *(ucp + 44), *(ucp + 45), *(ucp + 46), *(ucp + 47)); + *(ucp + 40), *(ucp + 41), *(ucp + 42), *(ucp + 43), + *(ucp + 44), *(ucp + 45), *(ucp + 46), *(ucp + 47)); } #endif #if 0 @@ -230,10 +238,11 @@ c4_hdw_init(struct pci_dev *pdev, int found) return 0; } - if (pdev-bus) /* obtain bus number */ + /* obtain bus number */ + if (pdev-bus) busno = pdev-bus-number; else - busno = 0; /* default for system PCI inconsistency */ + busno = 0; /* default for system PCI inconsistency */ slot = pdev-devfn ~0x07; /* @@ -246,8 +255,8 @@ c4_hdw_init(struct pci_dev *pdev, int found) for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) { /* -* match with board's first found interface, otherwise this is first -* found +* match with board's first found interface, otherwise this is +* fisrt found */ if ((hi-pci_slot == 0xff) || /* new board */ ((hi-pci_slot == slot) (hi-bus == pdev-bus))) @@ -256,13 +265,14 @@ c4_hdw_init(struct pci_dev *pdev, int found) if (i == MAX_BOARDS)/* no match in above loop means MAX * exceeded */ { - pr_warning(exceeded number of allowed devices (%d)?\n, MAX_BOARDS); + pr_warning(exceeded number of allowed devices (%d)?\n, + MAX_BOARDS); return
[PATCH 4/5] staging: cxt1e1: fix checkpatch error 'assignment in if condition'
checkpatch.pl error: ERROR: do not use assignment in if condition Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/cxt1e1/hwprobe.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/staging/cxt1e1/hwprobe.c b/drivers/staging/cxt1e1/hwprobe.c index d6ccbd9..5f0e05d 100644 --- a/drivers/staging/cxt1e1/hwprobe.c +++ b/drivers/staging/cxt1e1/hwprobe.c @@ -142,8 +142,8 @@ hdw_sn_get(hdw_info_t *hi, int brdno) hi-mfg_info.Serial[5]); #endif - if ((hi-promfmt = pmc_verify_cksum(hi-mfg_info.data)) == PROM_FORMAT_Unk) - { + hi-promfmt = pmc_verify_cksum(hi-mfg_info.data); + if (hi-promfmt == PROM_FORMAT_Unk) { /* bad crc, data is suspect */ if (cxt1e1_log_level = LOG_WARN) pr_info(%s: EEPROM cksum error\n, hi-devname); @@ -232,8 +232,8 @@ c4_hdw_init(struct pci_dev *pdev, int found) unsigned char busno = 0xff; /* our MUSYCC chip supports two functions, 0 1 */ - if ((fun = PCI_FUNC(pdev-devfn)) 1) - { + fun = PCI_FUNC(pdev-devfn); + if (fun 1) { pr_warning(unexpected devfun: 0x%x\n, pdev-devfn); return 0; } @@ -380,11 +380,11 @@ c4hw_attach_all(void) } pci_set_master(hi-pdev[0]); pci_set_master(hi-pdev[1]); - if (!(hi-ndev = c4_add_dev(hi, i, (long) hi-addr_mapped[0], - (long) hi-addr_mapped[1], - hi-pdev[0]-irq, - hi-pdev[1]-irq))) - { + hi-ndev = c4_add_dev(hi, i, (long) hi-addr_mapped[0], + (long) hi-addr_mapped[1], + hi-pdev[0]-irq, + hi-pdev[1]-irq); + if (!hi-ndev) { drvr_state = SBE_DRVR_DOWN; cleanup_ioremap(); /* NOTE: c4_add_dev() does its own device cleanup */ -- 1.7.9.5 --- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/5] staging: cxt1e1: fix checkpatch errors with open brace '{'
clean up checkpatch.pl error: ERROR: that open brace { should be on the previous line Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/cxt1e1/hwprobe.c | 62 +++--- 1 file changed, 25 insertions(+), 37 deletions(-) diff --git a/drivers/staging/cxt1e1/hwprobe.c b/drivers/staging/cxt1e1/hwprobe.c index 5f0e05d..d87a011 100644 --- a/drivers/staging/cxt1e1/hwprobe.c +++ b/drivers/staging/cxt1e1/hwprobe.c @@ -58,8 +58,7 @@ show_two(hdw_info_t *hi, int brdno) ci = (ci_t *)(netdev_priv(hi-ndev)); bid = sbeid_get_bdname(ci); - switch (hi-promfmt) - { + switch (hi-promfmt) { case PROM_FORMAT_TYPE1: memcpy(sn, (FLD_TYPE1 *)(hi-mfg_info.pft1.Serial), 6); break; @@ -159,8 +158,7 @@ prep_hdw_info(void) hdw_info_t *hi; int i; - for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) - { + for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) { hi-pci_busno = 0xff; hi-pci_slot = 0xff; hi-pci_pin[0] = 0; @@ -179,18 +177,15 @@ cleanup_ioremap(void) hdw_info_t *hi; int i; - for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) - { + for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) { if (hi-pci_slot == 0xff) break; - if (hi-addr_mapped[0]) - { + if (hi-addr_mapped[0]) { iounmap((void *)(hi-addr_mapped[0])); release_mem_region((long) hi-addr[0], hi-len[0]); hi-addr_mapped[0] = 0; } - if (hi-addr_mapped[1]) - { + if (hi-addr_mapped[1]) { iounmap((void *)(hi-addr_mapped[1])); release_mem_region((long) hi-addr[1], hi-len[1]); hi-addr_mapped[1] = 0; @@ -205,8 +200,7 @@ cleanup_devs(void) hdw_info_t *hi; int i; - for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) - { + for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) { if (hi-pci_slot == 0xff || !hi-ndev) break; c4_stopwd(netdev_priv(hi-ndev)); @@ -252,8 +246,7 @@ c4_hdw_init(struct pci_dev *pdev, int found) * element, identified by slot==(0xff). The second part of a board's * functionality will match the previously loaded slot/busno. */ - for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) - { + for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) { /* * match with board's first found interface, otherwise this is * fisrt found @@ -262,17 +255,19 @@ c4_hdw_init(struct pci_dev *pdev, int found) ((hi-pci_slot == slot) (hi-bus == pdev-bus))) break; /* found for-loop exit */ } - if (i == MAX_BOARDS)/* no match in above loop means MAX -* exceeded */ - { + + /* no match in above loop means MAX exceeded */ + if (i == MAX_BOARDS) { pr_warning(exceeded number of allowed devices (%d)?\n, MAX_BOARDS); return 0; } + if (pdev-bus) hi-pci_busno = pdev-bus-number; else hi-pci_busno = 0; /* default for system PCI inconsistency */ + hi-pci_slot = slot; pci_read_config_byte(pdev, PCI_INTERRUPT_PIN, hi-pci_pin[fun]); pci_read_config_byte(pdev, PCI_REVISION_ID, hi-revid[fun]); @@ -311,43 +306,38 @@ c4hw_attach_all(void) /*** scan PCI bus for all possible boards */ while ((pdev = pci_get_device(PCI_VENDOR_ID_CONEXANT, PCI_DEVICE_ID_CN8474, - pdev))) - { + pdev))) { if (c4_hdw_init(pdev, found)) found++; } - if (!found) - { + + if (!found) { pr_warning(No boards found\n); return -ENODEV; } + /* sanity check for consistent hardware found */ - for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) - { - if (hi-pci_slot != 0xff (!hi-addr[0] || !hi-addr[1])) - { + for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) { + if (hi-pci_slot != 0xff (!hi-addr[0] || !hi-addr[1])) { pr_warning(%s: something very wrong with pci_get_device\n, hi-devname); return -EIO; } } /* bring board's memory regions on/line */ - for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) -
Re: [PATCH 3/5] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c
On Fri, Feb 28, 2014 at 04:12:22PM +0900, Daeseok Youn wrote: clean up checkpatch.pl warnings: WARNING: Line length over 80 characters Patch is white space dammaged and doesn't apply. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: comedi: addi-data: remove unnecessary variable initializations in hwdrv_apci035.c
Nearly every variable in hwdrv_apci035.c is initialized to 0 when it is declared, and then set to some other value before ever being used. As such, we can remove all of these initializations. They are accomplishing nothing. Signed-off-by: Chase Southwood chase.southw...@yahoo.com --- .../comedi/drivers/addi-data/hwdrv_apci035.c | 26 +++--- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c index 9041fdf..80cca95 100644 --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c @@ -169,9 +169,9 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev, unsigned int *data) { struct addi_private *devpriv = dev-private; - unsigned int ui_Status = 0; - unsigned int ui_Command = 0; - unsigned int ui_Mode = 0; + unsigned int ui_Status; + unsigned int ui_Command; + unsigned int ui_Mode; i_Temp = 0; devpriv-tsk_Current = current; @@ -318,8 +318,8 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev, unsigned int *data) { struct addi_private *devpriv = dev-private; - unsigned int ui_Command = 0; - int i_Count = 0; + unsigned int ui_Command; + int i_Count; if (data[0] == 1) { ui_Command = @@ -433,7 +433,7 @@ static int i_APCI035_ReadTimerWatchdog(struct comedi_device *dev, unsigned int *data) { struct addi_private *devpriv = dev-private; - unsigned int ui_Status = 0; /* Status register */ + unsigned int ui_Status; /* Status register */ i_WatchdogNbr = insn-unused[0]; @@ -526,7 +526,7 @@ static int i_APCI035_ReadAnalogInput(struct comedi_device *dev, unsigned int *data) { struct addi_private *devpriv = dev-private; - unsigned int ui_CommandRegister = 0; + unsigned int ui_CommandRegister; /* Set the start */ ui_CommandRegister = 0x8; @@ -557,7 +557,7 @@ static int i_APCI035_ReadAnalogInput(struct comedi_device *dev, static int i_APCI035_Reset(struct comedi_device *dev) { struct addi_private *devpriv = dev-private; - int i_Count = 0; + int i_Count; for (i_Count = 1; i_Count = 4; i_Count++) { i_WatchdogNbr = i_Count; @@ -591,11 +591,11 @@ static void v_APCI035_Interrupt(int irq, void *d) { struct comedi_device *dev = d; struct addi_private *devpriv = dev-private; - unsigned int ui_StatusRegister1 = 0; - unsigned int ui_StatusRegister2 = 0; - unsigned int ui_ReadCommand = 0; - unsigned int ui_ChannelNumber = 0; - unsigned int ui_DigitalTemperature = 0; + unsigned int ui_StatusRegister1; + unsigned int ui_StatusRegister2; + unsigned int ui_ReadCommand; + unsigned int ui_ChannelNumber; + unsigned int ui_DigitalTemperature; if (i_Temp == 1) { i_WatchdogNbr = i_Flag; -- 1.8.5.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] Staging: comedi: addi-data: fix lines that are over 80 characters
hwdrv_apci1564.c had numerous lines over the column limit. This patch splits all such lines to bring them in compliance with coding style. Signed-off-by: Chase Southwood chase.south...@yahoo.com --- .../comedi/drivers/addi-data/hwdrv_apci1564.c | 50 -- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c index 2b47fa1..77030c5 100644 --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c @@ -324,11 +324,15 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev, inl(devpriv-i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); ul_Command1 = ul_Command1 0xF9FEUL; - outl(ul_Command1, devpriv-i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* Stop The Timer */ + /* Stop The Timer */ + outl(ul_Command1, devpriv-i_IobaseAmcc + APCI1564_TIMER + + APCI1564_TCW_PROG); devpriv-b_TimerSelectMode = ADDIDATA_TIMER; if (data[1] == 1) { - outl(0x02, devpriv-i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* Enable TIMER int DISABLE ALL THE OTHER int SOURCES */ + /* Enable TIMER int DISABLE ALL THE OTHER int SOURCES */ + outl(0x02, devpriv-i_IobaseAmcc + APCI1564_TIMER + + APCI1564_TCW_PROG); outl(0x0, devpriv-i_IobaseAmcc + APCI1564_DIGITAL_IP + APCI1564_DIGITAL_IP_IRQ); @@ -352,7 +356,9 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev, devpriv-iobase + APCI1564_COUNTER4 + APCI1564_TCW_IRQ); } else { - outl(0x0, devpriv-i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* disable Timer interrupt */ + /* disable Timer interrupt */ + outl(0x0, devpriv-i_IobaseAmcc + APCI1564_TIMER + + APCI1564_TCW_PROG); } /* Loading Timebase */ @@ -370,7 +376,9 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev, APCI1564_TCW_PROG); ul_Command1 = (ul_Command1 0xFFF719E2UL) | 2UL 13UL | 0x10UL; - outl(ul_Command1, devpriv-i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* mode 2 */ + /* mode 2 */ + outl(ul_Command1, devpriv-i_IobaseAmcc + APCI1564_TIMER + + APCI1564_TCW_PROG); } else if (data[0] == ADDIDATA_COUNTER) { devpriv-b_TimerSelectMode = ADDIDATA_COUNTER; devpriv-b_ModeSelectRegister = data[5]; @@ -380,7 +388,9 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev, inl(devpriv-iobase + ((data[5] - 1) * 0x20) + APCI1564_TCW_PROG); ul_Command1 = ul_Command1 0xF9FEUL; - outl(ul_Command1, devpriv-iobase + ((data[5] - 1) * 0x20) + APCI1564_TCW_PROG);/* Stop The Timer */ + /* Stop The Timer */ + outl(ul_Command1, devpriv-iobase + ((data[5] - 1) * 0x20) + + APCI1564_TCW_PROG); /* Set the reload value */ outl(data[3], @@ -457,7 +467,9 @@ static int i_APCI1564_StartStopWriteTimerCounterWatchdog(struct comedi_device *d if (devpriv-b_TimerSelectMode == ADDIDATA_WATCHDOG) { switch (data[1]) { case 0: /* stop the watchdog */ - outl(0x0, devpriv-i_IobaseAmcc + APCI1564_DIGITAL_OP_WATCHDOG + APCI1564_TCW_PROG);/* disable the watchdog */ + /* disable the watchdog */ + outl(0x0, devpriv-i_IobaseAmcc + APCI1564_DIGITAL_OP_WATCHDOG + + APCI1564_TCW_PROG); break; case 1: /* start the watchdog */ outl(0x0001, @@ -678,13 +690,18 @@ static void v_APCI1564_Interrupt(int irq, void *d) inl(devpriv-i_IobaseAmcc + APCI1564_DIGITAL_IP + APCI1564_DIGITAL_IP_INTERRUPT_STATUS); ui_InterruptStatus_1564 = ui_InterruptStatus_1564 0X0000; - send_sig(SIGIO, devpriv-tsk_Current, 0); /* send signal to the sample */ - outl(ui_DI, devpriv-i_IobaseAmcc + APCI1564_DIGITAL_IP + APCI1564_DIGITAL_IP_IRQ); /* enable the interrupt */ + /* send signal to the sample */ + send_sig(SIGIO,
[PATCH 2/2] Staging: comedi: addi-data: remove unnecessary variable initializations in hwdrv_apci1564.c
A handful of variables here were being initialized to 0 upon declaration, however they are always then set to another value before their first use, so initialization here is useless and we can remove it. Signed-off-by: Chase Southwood chase.southw...@yahoo.com --- drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c index 968e26c..83e4a41 100644 --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c @@ -304,7 +304,7 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev, unsigned int *data) { struct addi_private *devpriv = dev-private; - unsigned int ul_Command1 = 0; + unsigned int ul_Command1; devpriv-tsk_Current = current; if (data[0] == ADDIDATA_WATCHDOG) { @@ -462,7 +462,7 @@ static int i_APCI1564_StartStopWriteTimerCounterWatchdog(struct comedi_device *d unsigned int *data) { struct addi_private *devpriv = dev-private; - unsigned int ul_Command1 = 0; + unsigned int ul_Command1; if (devpriv-b_TimerSelectMode == ADDIDATA_WATCHDOG) { switch (data[1]) { @@ -560,7 +560,7 @@ static int i_APCI1564_ReadTimerCounterWatchdog(struct comedi_device *dev, unsigned int *data) { struct addi_private *devpriv = dev-private; - unsigned int ul_Command1 = 0; + unsigned int ul_Command1; if (devpriv-b_TimerSelectMode == ADDIDATA_WATCHDOG) { /* Stores the status of the Watchdog */ @@ -658,7 +658,7 @@ static void v_APCI1564_Interrupt(int irq, void *d) unsigned int ui_DO, ui_DI; unsigned int ui_Timer; unsigned int ui_C1, ui_C2, ui_C3, ui_C4; - unsigned int ul_Command2 = 0; + unsigned int ul_Command2; ui_DI = inl(devpriv-i_IobaseAmcc + APCI1564_DIGITAL_IP + APCI1564_DIGITAL_IP_IRQ) 0x01; -- 1.8.5.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/5] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c
OK. sorry. I will send again. Thanks. Daeseok Youn 2014-02-28 16:28 GMT+09:00 Dan Carpenter dan.carpen...@oracle.com: On Fri, Feb 28, 2014 at 04:12:22PM +0900, Daeseok Youn wrote: clean up checkpatch.pl warnings: WARNING: Line length over 80 characters Patch is white space dammaged and doesn't apply. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: comedi: add timeouts to while loops in s626.c
Smatch located a handful of while loops testing readl calls in s626.c. Since these while loops depend on readl succeeding, it's safer to make sure they time out eventually. Signed-off-by: Chase Southwood chase.southw...@yahoo.com --- Ian and/or Hartley, I'd love your comments on this. It seems to me that we want these kinds of while loops properly timed out, but I want to make sure I'm doing everything properly. First off, s626_debi_transfer() says directly that it is called from within critical sections, so I assume that means that the new comedi_timeout() function is no good here, and s626_send_dac() looked equally suspicious, so I opted for iterative timeouts. Is this correct? Also, for these timeouts, I used a very conservative 1 iterations, would it be better to decrease that? Also, do my error strings appear acceptable? And finally, are timeouts here even necessary or helpful, or are there any better ways to do it? Thanks, Chase drivers/staging/comedi/drivers/s626.c | 49 --- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c index 5ba4b4a..282636b 100644 --- a/drivers/staging/comedi/drivers/s626.c +++ b/drivers/staging/comedi/drivers/s626.c @@ -209,6 +209,8 @@ static const struct comedi_lrange s626_range_table = { static void s626_debi_transfer(struct comedi_device *dev) { struct s626_private *devpriv = dev-private; + static const int timeout = 1; + int i; /* Initiate upload of shadow RAM to DEBI control register */ s626_mc_enable(dev, S626_MC2_UPLD_DEBI, S626_P_MC2); @@ -221,8 +223,13 @@ static void s626_debi_transfer(struct comedi_device *dev) ; /* Wait until DEBI transfer is done */ - while (readl(devpriv-mmio + S626_P_PSR) S626_PSR_DEBI_S) - ; + for (i = 0; i timeout; i++) { + if (!(readl(devpriv-mmio + S626_P_PSR) S626_PSR_DEBI_S)) + break; + udelay(1); + } + if (i == timeout) + comedi_error(dev, DEBI transfer timeout.); } /* @@ -359,6 +366,8 @@ static const uint8_t s626_trimadrs[] = { static void s626_send_dac(struct comedi_device *dev, uint32_t val) { struct s626_private *devpriv = dev-private; + static const int timeout = 1; + int i; /* START THE SERIAL CLOCK RUNNING - */ @@ -404,8 +413,13 @@ static void s626_send_dac(struct comedi_device *dev, uint32_t val) * Done by polling the DMAC enable flag; this flag is automatically * cleared when the transfer has finished. */ - while (readl(devpriv-mmio + S626_P_MC1) S626_MC1_A2OUT) - ; + for (i = 0; i timeout; i++) { + if (!(readl(devpriv-mmio + S626_P_MC1) S626_MC1_A2OUT)) + break; + udelay(1); + } + if (i == timeout) + comedi_error(dev, DMA transfer timeout.); /* START THE OUTPUT STREAM TO THE TARGET DAC */ @@ -425,8 +439,13 @@ static void s626_send_dac(struct comedi_device *dev, uint32_t val) * finished transferring the DAC's data DWORD from the output FIFO * to the output buffer register. */ - while (!(readl(devpriv-mmio + S626_P_SSR) S626_SSR_AF2_OUT)) - ; + for (i = 0; i timeout; i++) { + if (readl(devpriv-mmio + S626_P_SSR) S626_SSR_AF2_OUT) + break; + udelay(1); + } + if (i == timeout) + comedi_error(dev, TSL timeout waiting for slot 1 to execute.); /* * Set up to trap execution at slot 0 when the TSL sequencer cycles @@ -466,8 +485,13 @@ static void s626_send_dac(struct comedi_device *dev, uint32_t val) * from 0xFF to 0x00, which slot 0 causes to happen by shifting * out/in on SD2 the 0x00 that is always referenced by slot 5. */ - while (readl(devpriv-mmio + S626_P_FB_BUFFER2) 0xff00) - ; + for (i = 0; i timeout; i++) { + if (!(readl(devpriv-mmio + S626_P_FB_BUFFER2) 0xff00)) + break; + udelay(1); + } + if (i == timeout) + comedi_error(dev, TSL timeout waiting for slot 0 to execute.); } /* * Either (1) we were too late setting the slot 0 trap; the TSL @@ -486,8 +510,13 @@ static void s626_send_dac(struct comedi_device *dev, uint32_t val) * the next DAC write. This is detected when FB_BUFFER2 MSB changes * from 0x00 to 0xFF. */ - while (!(readl(devpriv-mmio + S626_P_FB_BUFFER2) 0xff00)) - ; + for (i = 0; i timeout; i++) { + if
[PATCH 3/5 v2] Staging: cxt1e1: Fix line length over 80 characters in hwprobe.c
clean up checkpatch.pl warnings: WARNING: Line length over 80 characters Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- drivers/staging/cxt1e1/hwprobe.c | 45 -- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/drivers/staging/cxt1e1/hwprobe.c b/drivers/staging/cxt1e1/hwprobe.c index 85040bb..d6ccbd9 100644 --- a/drivers/staging/cxt1e1/hwprobe.c +++ b/drivers/staging/cxt1e1/hwprobe.c @@ -37,7 +37,8 @@ extern int drvr_state; /* forward references */ voidc4_stopwd(ci_t *); -struct net_device * __init c4_add_dev(hdw_info_t *, int, unsigned long, unsigned long, int, int); +struct net_device * __init c4_add_dev(hdw_info_t *, int, unsigned long, + unsigned long, int, int); struct s_hdw_info hdw_info[MAX_BOARDS]; @@ -104,24 +105,31 @@ hdw_sn_get(hdw_info_t *hi, int brdno) addr = (long) hi-addr_mapped[1] + EEPROM_OFFSET; /* read EEPROM with largest known format size... */ - pmc_eeprom_read_buffer(addr, 0, (char *)hi-mfg_info.data, sizeof(FLD_TYPE2)); + pmc_eeprom_read_buffer(addr, 0, (char *)hi-mfg_info.data, + sizeof(FLD_TYPE2)); #if 0 { unsigned char *ucp = (unsigned char *) hi-mfg_info.data; pr_info(eeprom[00]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 0), *(ucp + 1), *(ucp + 2), *(ucp + 3), *(ucp + 4), *(ucp + 5), *(ucp + 6), *(ucp + 7)); + *(ucp + 0), *(ucp + 1), *(ucp + 2), *(ucp + 3), + *(ucp + 4), *(ucp + 5), *(ucp + 6), *(ucp + 7)); pr_info(eeprom[08]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 8), *(ucp + 9), *(ucp + 10), *(ucp + 11), *(ucp + 12), *(ucp + 13), *(ucp + 14), *(ucp + 15)); + *(ucp + 8), *(ucp + 9), *(ucp + 10), *(ucp + 11), + *(ucp + 12), *(ucp + 13), *(ucp + 14), *(ucp + 15)); pr_info(eeprom[16]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 16), *(ucp + 17), *(ucp + 18), *(ucp + 19), *(ucp + 20), *(ucp + 21), *(ucp + 22), *(ucp + 23)); + *(ucp + 16), *(ucp + 17), *(ucp + 18), *(ucp + 19), + *(ucp + 20), *(ucp + 21), *(ucp + 22), *(ucp + 23)); pr_info(eeprom[24]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 24), *(ucp + 25), *(ucp + 26), *(ucp + 27), *(ucp + 28), *(ucp + 29), *(ucp + 30), *(ucp + 31)); + *(ucp + 24), *(ucp + 25), *(ucp + 26), *(ucp + 27), + *(ucp + 28), *(ucp + 29), *(ucp + 30), *(ucp + 31)); pr_info(eeprom[32]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 32), *(ucp + 33), *(ucp + 34), *(ucp + 35), *(ucp + 36), *(ucp + 37), *(ucp + 38), *(ucp + 39)); + *(ucp + 32), *(ucp + 33), *(ucp + 34), *(ucp + 35), + *(ucp + 36), *(ucp + 37), *(ucp + 38), *(ucp + 39)); pr_info(eeprom[40]: %02x %02x %02x %02x %02x %02x %02x %02x\n, - *(ucp + 40), *(ucp + 41), *(ucp + 42), *(ucp + 43), *(ucp + 44), *(ucp + 45), *(ucp + 46), *(ucp + 47)); + *(ucp + 40), *(ucp + 41), *(ucp + 42), *(ucp + 43), + *(ucp + 44), *(ucp + 45), *(ucp + 46), *(ucp + 47)); } #endif #if 0 @@ -230,10 +238,11 @@ c4_hdw_init(struct pci_dev *pdev, int found) return 0; } - if (pdev-bus) /* obtain bus number */ + /* obtain bus number */ + if (pdev-bus) busno = pdev-bus-number; else - busno = 0; /* default for system PCI inconsistency */ + busno = 0; /* default for system PCI inconsistency */ slot = pdev-devfn ~0x07; /* @@ -246,8 +255,8 @@ c4_hdw_init(struct pci_dev *pdev, int found) for (i = 0, hi = hdw_info; i MAX_BOARDS; i++, hi++) { /* -* match with board's first found interface, otherwise this is first -* found +* match with board's first found interface, otherwise this is +* fisrt found */ if ((hi-pci_slot == 0xff) || /* new board */ ((hi-pci_slot == slot) (hi-bus == pdev-bus))) @@ -256,13 +265,14 @@ c4_hdw_init(struct pci_dev *pdev, int found) if (i == MAX_BOARDS)/* no match in above loop means MAX * exceeded */ { - pr_warning(exceeded number of allowed devices (%d)?\n, MAX_BOARDS); + pr_warning(exceeded number of allowed devices (%d)?\n, + MAX_BOARDS); return 0;
Re: [RFC PATCH v4 3/8] staging: imx-drm: Document updated imx-drm device tree bindings
On 27/02/14 18:54, Philipp Zabel wrote: - One IPU enabled, one disabled: nothing special here, just set the other IPU to status=disabled in the DT data. The driver for the enabled IPU would register the required DRM entities. that should work. Let the enabled IPU create the imx-drm platform device on probe, parse the device tree and ignore everything only hanging off of the disabled IPU. I think you misunderstood me a bit. What I meant is that there's no need for imx-drm device at all, neither in the DT data or in the kernel side. There'd just be the DT nodes for the IPUs, which would cause the IPU platform devices to be created, and a driver for the IPU. So just like for any other normal platform device. In the simplest cases, where only one IPU is enabled, or the IPUs want to be considered as totally independent, there'd be nothing special. The IPU driver would just register the drm entities. [Reordering a bit...] - Two IPUs in combined mode: Pick one IPU as the master, and one as slave. Link the IPU nodes in DT data with phandles, say: master=ipu1 on the slave IPU and slave=ipu0 on the master. The master one will register the DRM entities, and the slave one will just do what the master says. That might work, too. Just let the each IPU scan the graph and try to find the imx-drm master before creating the imx-drm platform device. The first IPU fill find no preexisting master and create the imx-drm platform device as above, adding the other IPU as well as the other components with component_master_add_child. It just has to make sure that the other IPU is added to the list before the encoders are. The second IPU will scan the graph, find a preexisting master for the other IPU node, register its component and just wait to be bound by the master. Here the slave IPU doesn't need to scan the graph at all. It just needs to make itself available somehow to the master. Maybe just by exported functions, or registering itself somewhere. Only the master IPU will scan the graph, and as all the entities are connected to the same graph, including the slave IPU, the master can find all the relevant nodes. - Two IPUs as separate units: almost the same as above, but both would independently register the DRM entities. Here the second IPU would not be connected to the first IPU via the graph - it would not find a preexisting imx-drm device when scanning its graph and create its own imx-drm device just like the first IPU did. As a result there are two completely separate DRM devices. I understood that that would be the idea, two separate, independent DRM devices. Like two graphics cards on a PC. That being said, this change could be made at any time in the future, in a backwards compatible fashion, by just declaring the imx-drm node optional and ignoring it if it exists. Yes, I agree. And I don't even know if the master-slave method I described is valid, although I don't see why it would not work. The master display-subsystem DT node does make sense to me in cases like this, where the IPUs need to be driven as a single unit. Did anybody propose such a generic term? How about: -imx-drm { - compatible = fsl,imx-drm; - ports = ipu1_di0, ipu1_di1; -}; +display-subsystem { + compatible = fsl,imx-display-subsystem; + ports = ipu1_di0, ipu1_di1; +}; That sounds fine to me. I wonder how it works if, say, there are 4 IPUs, and you want to run them in two pairs. In that case you need two of those display-subsystem nodes. But I guess it's just a matter of assigning a number for them with 'regs' property, and making sure the driver has nothing that prevents multiple instances of it. If this wasn't the case, we wouldn't even attempt to describe what devices we have on which I2C buses - we'd just list the hardware on the board without giving any information about how it's wired together. This is no different - however, it doesn't have (and shouldn't) be subsystem specific... but - and this is the challenge we then face - how do you decide that on one board with a single zImage kernel, with both DRM and fbdev built-in, whether to use the DRM interfaces or the fbdev interfaces? We could have both matching the same compatible string, but we'd also need some way to tell each other that they're not allowed to bind. Yes, that's an annoying problem, we have that on OMAP. It's a clear sign that our video support is rather messed up. My opinion is that the fbdev and drm drivers for a single hardware should be exclusive at compile time. We don't allow multiple drivers for single device for other subsystems either, do we? Eventually we should have only one driver for one hardware device. If that's not possible, then the drivers in question could have an option to enable or disable themselves, passed via the kernel command line, so that the user can select which subsystem to use. That is the exact same problem as having multiple drivers
Re: [PATCH 1/2] Staging: comedi: addi-data: fix lines that are over 80 characters
On Fri, Feb 28, 2014 at 01:31:20AM -0600, Chase Southwood wrote: hwdrv_apci1564.c had numerous lines over the column limit. This patch splits all such lines to bring them in compliance with coding style. Signed-off-by: Chase Southwood chase.south...@yahoo.com --- .../comedi/drivers/addi-data/hwdrv_apci1564.c | 50 -- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c index 2b47fa1..77030c5 100644 --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c @@ -324,11 +324,15 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev, inl(devpriv-i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); ul_Command1 = ul_Command1 0xF9FEUL; - outl(ul_Command1, devpriv-i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* Stop The Timer */ + /* Stop The Timer */ + outl(ul_Command1, devpriv-i_IobaseAmcc + APCI1564_TIMER + + APCI1564_TCW_PROG); Just make a helper function so that you can call it like this: static void outl_1564_timer(struct addi_private *devpriv, unsigned int cmd, unsigned int reg) { outl(cmd, devpriv-i_IobaseAmcc + APCI1564_TIMER, reg); } Then the caller becomes: outl_1564_timer(devpriv, cmd, APCI1564_TCW_PROG); regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] Staging: comedi: addi-data: fix lines that are over 80 characters
On Fri, Feb 28, 2014 at 10:52:32AM +0300, Dan Carpenter wrote: On Fri, Feb 28, 2014 at 01:31:20AM -0600, Chase Southwood wrote: hwdrv_apci1564.c had numerous lines over the column limit. This patch splits all such lines to bring them in compliance with coding style. Signed-off-by: Chase Southwood chase.south...@yahoo.com --- .../comedi/drivers/addi-data/hwdrv_apci1564.c | 50 -- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c index 2b47fa1..77030c5 100644 --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c @@ -324,11 +324,15 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev, inl(devpriv-i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); ul_Command1 = ul_Command1 0xF9FEUL; - outl(ul_Command1, devpriv-i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* Stop The Timer */ + /* Stop The Timer */ + outl(ul_Command1, devpriv-i_IobaseAmcc + APCI1564_TIMER + + APCI1564_TCW_PROG); Just make a helper function so that you can call it like this: static void outl_1564_timer(struct addi_private *devpriv, unsigned int cmd, unsigned int reg) { outl(cmd, devpriv-i_IobaseAmcc + APCI1564_TIMER, reg); ^^^ Should be devpriv-i_IobaseAmcc + APCI1564_TIMER + reg obviously. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel