[PATCH 1/2] staging: bcm: Remove unneeded set a variable

2014-02-27 Thread Daeseok Youn

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'

2014-02-27 Thread Daeseok Youn

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

2014-02-27 Thread 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.

  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

2014-02-27 Thread Masanari Iida
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

2014-02-27 Thread Masanari Iida
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

2014-02-27 Thread Philipp Zabel
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

2014-02-27 Thread Russell King - ARM Linux
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

2014-02-27 Thread xypron . glpk
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

2014-02-27 Thread Tomi Valkeinen
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

2014-02-27 Thread Russell King - ARM Linux
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

2014-02-27 Thread Tomi Valkeinen
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

2014-02-27 Thread Tomi Valkeinen
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.

2014-02-27 Thread Reza Snowdon
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.

2014-02-27 Thread Greg KH
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.

2014-02-27 Thread Dan Carpenter
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.

2014-02-27 Thread r...@mage.me.uk
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

2014-02-27 Thread Mark Hounschell
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

2014-02-27 Thread Greg Kroah-Hartman
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

2014-02-27 Thread Greg Kroah-Hartman
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

2014-02-27 Thread Uwe Kleine-König
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

2014-02-27 Thread Greg Kroah-Hartman
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

2014-02-27 Thread Dan Carpenter
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

2014-02-27 Thread Greg KH
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()

2014-02-27 Thread Greg KH
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()

2014-02-27 Thread Russell King - ARM Linux
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

2014-02-27 Thread Shuah Khan
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

2014-02-27 Thread Shuah Khan
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

2014-02-27 Thread Daeseok Youn

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

2014-02-27 Thread Kumar Amit Mehta
'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

2014-02-27 Thread KY Srinivasan


 -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

2014-02-27 Thread Jason Wang
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()

2014-02-27 Thread Greg KH
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()

2014-02-27 Thread Dan Carpenter
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

2014-02-27 Thread Dan Carpenter
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

2014-02-27 Thread Daeseok Youn

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

2014-02-27 Thread Daeseok Youn

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

2014-02-27 Thread Daeseok Youn

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'

2014-02-27 Thread Daeseok Youn

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 '{'

2014-02-27 Thread Daeseok Youn

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

2014-02-27 Thread Dan Carpenter
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

2014-02-27 Thread Chase Southwood
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

2014-02-27 Thread Chase Southwood
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

2014-02-27 Thread Chase Southwood
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

2014-02-27 Thread DaeSeok Youn
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

2014-02-27 Thread Chase Southwood
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

2014-02-27 Thread Daeseok Youn

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

2014-02-27 Thread Tomi Valkeinen
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

2014-02-27 Thread Dan Carpenter
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

2014-02-27 Thread Dan Carpenter
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