Re: [PATCH v4 7/8] netdev: octeon-ethernet: Add Cavium Octeon III support.
Hi David, Dan,
On Thu, Nov 30, 2017 at 12:50 AM, David Daney wrote:
> On 11/29/2017 08:07 AM, Souptick Joarder wrote:
>>
>> On Wed, Nov 29, 2017 at 4:00 PM, Souptick Joarder
>> wrote:
>>>
>>> On Wed, Nov 29, 2017 at 6:25 AM, David Daney
>>> wrote:
From: Carlos Munoz
The Cavium OCTEON cn78xx and cn73xx SoCs have network packet I/O
hardware that is significantly different from previous generations of
the family.
>>
>>
diff --git a/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
b/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
new file mode 100644
index ..4dad35fa4270
--- /dev/null
+++ b/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
@@ -0,0 +1,2033 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2017 Cavium, Inc.
+ *
+ * This file is subject to the terms and conditions of the GNU General
Public
+ * License. See the file "COPYING" in the main directory of this
archive
+ * for more details.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
>>
>>
+static void bgx_port_sgmii_set_link_down(struct bgx_port_priv *priv)
+{
+ u64 data;
>>
>>
+ data = oct_csr_read(BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx,
priv->index));
+ data |= BIT(11);
+ oct_csr_write(data, BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx,
priv->index));
+ data = oct_csr_read(BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx,
priv->index));
>>>
>>>
>>> Any particular reason to read immediately after write ?
>>
>>
>
> Yes, to ensure the write is committed to hardware before the next step.
>
>>
>>
+static int bgx_port_sgmii_set_link_speed(struct bgx_port_priv *priv,
struct port_status status)
+{
+ u64 data;
+ u64 prtx;
+ u64 miscx;
+ int timeout;
+
>>
>>
+
+ switch (status.speed) {
+ case 10:
>>>
>>>
>>> In my opinion, instead of hard coding the value, is it fine to use ENUM ?
>>
>> Similar comments applicable in other places where hard coded values
>> are used.
>>
>
> There is nothing to be gained by interposing an extra layer of abstraction
> in this case. The code is more clear with the raw numbers in this
> particular case.
As mentioned by Andrew, macros defined in uapi/linux/ethtool.h may
be useful here.
Otherwise it's fine to me :)
>
>
>>
>>
+static int bgx_port_gser_27882(struct bgx_port_priv *priv)
+{
+ u64 data;
+ u64 addr;
>>>
>>>
+ int timeout = 200;
+
+ //timeout = 200;
>>
>> Better to initialize the timeout value
>
>
> What are you talking about? It is properly initialized using valid C code.
I mean, instead of writing
int timeout;
timeout = 200;
write,
int timeout = 200;
Anyway both are correct and there is nothing wrong in your code.
Please ignore my comment here.
>
>
>>
>>
+static int bgx_port_qlm_rx_equalization(struct bgx_port_priv *priv, int
qlm, int lane)
+{
+ lmode = oct_csr_read(GSER_LANE_MODE(priv->node, qlm));
+ lmode &= 0xf;
+ addr = GSER_LANE_P_MODE_1(priv->node, qlm, lmode);
+ data = oct_csr_read(addr);
+ /* Don't complete rx equalization if in VMA manual mode */
+ if (data & BIT(14))
+ return 0;
+
+ /* Apply rx equalization for speed > 6250 */
+ if (bgx_port_get_qlm_speed(priv, qlm) < 6250)
+ return 0;
+
+ /* Wait until rx data is valid (CDRLOCK) */
+ timeout = 500;
>>>
>>>
>>> 500 us is the min required value or it can be further reduced ?
>>
>>
>
>
> 500 uS works well and is shorter than the 2000 uS from the hardware manual.
>
> If you would like to verify shorter timeout values, we could consider
> merging such a patch. But really, this doesn't matter as it is a very short
> one-off action when the link is brought up.
Ok.
>
>>
+static int bgx_port_init_xaui_link(struct bgx_port_priv *priv)
+{
>>
>>
+
+ if (use_ber) {
+ timeout = 1;
+ do {
+ data =
+
oct_csr_read(BGX_SPU_BR_STATUS1(priv->node, priv->bgx, priv->index));
+ if (data & BIT(0))
+ break;
+ timeout--;
+ udelay(1);
+ } while (timeout);
>>>
>>>
>>> In my opinion, it's better to implement similar kind of loops inside
>>> macros.
>
>
> Ok, duly noted. I think we are in disagreement with respect to this po
Re: [PATCH v3] staging: lustre: Replace 'uint32_t' with 'u32' and 'uint64_t' with 'u64'
On Wed, Nov 29, 2017 at 07:46:21PM +0300, Roman Storozhenko wrote: > There are two reasons for that: In my email client the subject line and body are not next to each other. It looks like this: https://marc.info/?l=linux-arm-kernel&m=151187366315885&w=2 So it took me a while to realize what you were talking about. Please assume I'm either reading the subject or the body but not both. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 0/4] hv_balloon: fixes for num_pages_onlined accounting and misc improvements
> -Original Message-
> From: Haiyang Zhang
> Sent: Wednesday, November 29, 2017 16:55
> > -Original Message-
> > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> > Vitaly Kuznetsov writes:
> >
> > > While doing routing code review I noticed that commit 6df8d9aaf3af
> > > ("Drivers: hv: balloon: Correctly update onlined page count")
> > > introduced an issue with num_pages_onlined accounting on memory
> > > offlining. Deeper look showed that the accounting was always buggy. This
> > > is
> > fixed in PATCH3.
> > > PATCHes 1 and 2 are preparatory cleanups, PATCH4 adds a tracepoint to
> > > post_status so it's now possible to see what's being sent to the host
> > > and where the data comes from.
> > >
> >
> > K. Y., Alex,
> >
> > did you have a chance to take a look?
> >
> > Thanks,
> > --
> > Vitaly
>
> Dexuan,
>
> Maybe, you can take a look? Thanks.
Sure. I'll take a look.
Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: xgifb: remove unused macro XGIPART3
On Wed, Nov 29, 2017 at 09:53:48PM -0500, Joshua Abraham wrote: > Signed-off-by: Joshua Abraham > > This patch removes the unused macro XGIPART3. > The Signed-off-by line goes after the changelog. > --- > drivers/staging/xgifb/XGI_main.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/staging/xgifb/XGI_main.h > b/drivers/staging/xgifb/XGI_main.h > index a3af1cbbf8ee..5f55d0a39bc1 100644 > --- a/drivers/staging/xgifb/XGI_main.h > +++ b/drivers/staging/xgifb/XGI_main.h > @@ -25,7 +25,6 @@ MODULE_DEVICE_TABLE(pci, xgifb_pci_table); > #define XGIDACD(xgifb_info->dev_info.P3c9) > #define XGIPART1 (xgifb_info->dev_info.Part1Port) > #define XGIPART2 (xgifb_info->dev_info.Part2Port) > -#define XGIPART3 (xgifb_info->dev_info.Part3Port) That define isn't hurting anyone. > #define XGIPART4 (xgifb_info->dev_info.Part4Port) > #define XGIPART5 (xgifb_info->dev_info.Part5Port) Actually these should all be deleted because they mean you have to have a xgifb_info variable and they hurt readability by hiding stuff behind a define. It would be better to remove them all than to just remove one from the middle. That's a more complicated patch, but it's a useful patch. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: xgifb: remove unused macro XGIPART3
Signed-off-by: Joshua Abraham This patch removes the unused macro XGIPART3. --- drivers/staging/xgifb/XGI_main.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/xgifb/XGI_main.h b/drivers/staging/xgifb/XGI_main.h index a3af1cbbf8ee..5f55d0a39bc1 100644 --- a/drivers/staging/xgifb/XGI_main.h +++ b/drivers/staging/xgifb/XGI_main.h @@ -25,7 +25,6 @@ MODULE_DEVICE_TABLE(pci, xgifb_pci_table); #define XGIDACD (xgifb_info->dev_info.P3c9) #define XGIPART1 (xgifb_info->dev_info.Part1Port) #define XGIPART2 (xgifb_info->dev_info.Part2Port) -#define XGIPART3 (xgifb_info->dev_info.Part3Port) #define XGIPART4 (xgifb_info->dev_info.Part4Port) #define XGIPART5 (xgifb_info->dev_info.Part5Port) #define XGIDAC2A XGIPART5 -- 2.15.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 0/4] hv_balloon: fixes for num_pages_onlined accounting and misc improvements
> -Original Message-
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> Sent: Tuesday, November 28, 2017 8:30 AM
> To: Alex Ng ; KY Srinivasan
> Cc: linux-ker...@vger.kernel.org; Haiyang Zhang ;
> Stephen Hemminger ; Dexuan Cui
> ; de...@linuxdriverproject.org
> Subject: Re: [PATCH 0/4] hv_balloon: fixes for num_pages_onlined
> accounting and misc improvements
>
> Vitaly Kuznetsov writes:
>
> > While doing routing code review I noticed that commit 6df8d9aaf3af
> > ("Drivers: hv: balloon: Correctly update onlined page count")
> > introduced an issue with num_pages_onlined accounting on memory
> > offlining. Deeper look showed that the accounting was always buggy. This is
> fixed in PATCH3.
> > PATCHes 1 and 2 are preparatory cleanups, PATCH4 adds a tracepoint to
> > post_status so it's now possible to see what's being sent to the host
> > and where the data comes from.
> >
>
> K. Y., Alex,
>
> did you have a chance to take a look?
>
> Thanks,
>
> --
> Vitaly
Dexuan,
Maybe, you can take a look? Thanks.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 7/8] netdev: octeon-ethernet: Add Cavium Octeon III support.
On 11/29/2017 02:56 PM, Andrew Lunn wrote:
On Tue, Nov 28, 2017 at 04:55:39PM -0800, David Daney wrote:
+static int bgx_probe(struct platform_device *pdev)
+{
+ struct mac_platform_data platform_data;
+ const __be32 *reg;
+ u32 port;
+ u64 addr;
+ struct device_node *child;
+ struct platform_device *new_dev;
+ struct platform_device *pki_dev;
+ int numa_node, interface;
+ int i;
+ int r = 0;
+ char id[64];
+ u64 data;
+
+ reg = of_get_property(pdev->dev.of_node, "reg", NULL);
+ addr = of_translate_address(pdev->dev.of_node, reg);
+ interface = (addr >> 24) & 0xf;
+ numa_node = (addr >> 36) & 0x7;
Hi David
You have these two a few times in the code. Maybe add a helper to do
it? The NUMA one i assume could go somewhere in the SoC code?
Thanks for looking at it, I will try with helpers.
The rest of your comments below raise valid points, I will fix those too.
+static int bgx_mix_init_from_fdt(void)
+{
+ struct device_node *node;
+ struct device_node *parent = NULL;
+ int mix = 0;
+ /* Get the lmac index */
+ reg = of_get_property(lmac_fdt_node, "reg", NULL);
+ if (!reg)
+ goto err;
+
+ mix_port_lmacs[mix].lmac = *reg;
I don't think of_get_property() deals with endianness. Is there any
danger of this driver being used on hardware with the other endianness
to what you have tested?
+/**
+ * bgx_pki_init_from_param - Initialize the list of lmacs that connect to the
+ * pki from information in the "pki_port" parameter.
+ *
+ * The pki_port parameter format is as follows:
+ * pki_port=nbl
+ * where:
+ * n = node
+ * b = bgx
+ * l = lmac
+ *
+ * Commas must be used to separate multiple lmacs:
+ * pki_port=000,100,110
+ *
+ * Asterisks (*) specify all possible characters in
+ * the subset:
+ * pki_port=00* (all lmacs of node0 bgx0).
+ *
+ * Missing lmacs identifiers default to all
+ * possible characters in the subset:
+ * pki_port=00 (all lmacs on node0 bgx0)
+ *
+ * Brackets ('[' and ']') specify the valid
+ * characters in the subset:
+ * pki_port=00[01] (lmac0 and lmac1 of node0 bgx0).
+ *
+ * Returns 0 if successful.
+ * Returns <0 for error codes.
I've not used kerneldoc much, but i suspect this is wrongly formated:
https://www.kernel.org/doc/html/v4.9/kernel-documentation.html#function-documentation
+int bgx_port_ethtool_set_settings(struct net_device*netdev,
+ struct ethtool_cmd*cmd)
+{
+ struct bgx_port_priv *p = bgx_port_netdev2priv(netdev);
+
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
Not required. The enforces this. See dev_ethtool()
+
+ if (p->phydev)
+ return phy_ethtool_sset(p->phydev, cmd);
+
+ return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(bgx_port_ethtool_set_settings);
+
+int bgx_port_ethtool_nway_reset(struct net_device *netdev)
+{
+ struct bgx_port_priv *p = bgx_port_netdev2priv(netdev);
+
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
Also not needed.
+static void bgx_port_adjust_link(struct net_device *netdev)
+{
+ struct bgx_port_priv*priv = bgx_port_netdev2priv(netdev);
+ boollink_changed = false;
+ unsigned intlink;
+ unsigned intspeed;
+ unsigned intduplex;
+
+ mutex_lock(&priv->lock);
+
+ if (!priv->phydev->link && priv->last_status.link)
+ link_changed = true;
+
+ if (priv->phydev->link &&
+ (priv->last_status.link != priv->phydev->link ||
+priv->last_status.duplex != priv->phydev->duplex ||
+priv->last_status.speed != priv->phydev->speed))
+ link_changed = true;
+
+ link = priv->phydev->link;
+ priv->last_status.link = priv->phydev->link;
+
+ speed = priv->phydev->speed;
+ priv->last_status.speed = priv->phydev->speed;
+
+ duplex = priv->phydev->duplex;
+ priv->last_status.duplex = priv->phydev->duplex;
+
+ mutex_unlock(&priv->lock);
+
+ if (link_changed) {
+ struct port_status status;
+
+ phy_print_status(priv->phydev);
+
+ status.link = link ? 1 : 0;
+ status.duplex = duplex;
+ status.speed = speed;
+ if (!link) {
+ netif_carrier_off(netdev);
+
Re: [PATCH v4 7/8] netdev: octeon-ethernet: Add Cavium Octeon III support.
On Tue, Nov 28, 2017 at 04:55:39PM -0800, David Daney wrote:
> +static int bgx_probe(struct platform_device *pdev)
> +{
> + struct mac_platform_data platform_data;
> + const __be32 *reg;
> + u32 port;
> + u64 addr;
> + struct device_node *child;
> + struct platform_device *new_dev;
> + struct platform_device *pki_dev;
> + int numa_node, interface;
> + int i;
> + int r = 0;
> + char id[64];
> + u64 data;
> +
> + reg = of_get_property(pdev->dev.of_node, "reg", NULL);
> + addr = of_translate_address(pdev->dev.of_node, reg);
> + interface = (addr >> 24) & 0xf;
> + numa_node = (addr >> 36) & 0x7;
Hi David
You have these two a few times in the code. Maybe add a helper to do
it? The NUMA one i assume could go somewhere in the SoC code?
> +static int bgx_mix_init_from_fdt(void)
> +{
> + struct device_node *node;
> + struct device_node *parent = NULL;
> + int mix = 0;
> + /* Get the lmac index */
> + reg = of_get_property(lmac_fdt_node, "reg", NULL);
> + if (!reg)
> + goto err;
> +
> + mix_port_lmacs[mix].lmac = *reg;
I don't think of_get_property() deals with endianness. Is there any
danger of this driver being used on hardware with the other endianness
to what you have tested?
> +/**
> + * bgx_pki_init_from_param - Initialize the list of lmacs that connect to the
> + *pki from information in the "pki_port" parameter.
> + *
> + *The pki_port parameter format is as follows:
> + *pki_port=nbl
> + *where:
> + * n = node
> + * b = bgx
> + * l = lmac
> + *
> + *Commas must be used to separate multiple lmacs:
> + *pki_port=000,100,110
> + *
> + *Asterisks (*) specify all possible characters in
> + *the subset:
> + *pki_port=00* (all lmacs of node0 bgx0).
> + *
> + *Missing lmacs identifiers default to all
> + *possible characters in the subset:
> + *pki_port=00 (all lmacs on node0 bgx0)
> + *
> + *Brackets ('[' and ']') specify the valid
> + *characters in the subset:
> + *pki_port=00[01] (lmac0 and lmac1 of node0 bgx0).
> + *
> + * Returns 0 if successful.
> + * Returns <0 for error codes.
I've not used kerneldoc much, but i suspect this is wrongly formated:
https://www.kernel.org/doc/html/v4.9/kernel-documentation.html#function-documentation
> +int bgx_port_ethtool_set_settings(struct net_device *netdev,
> + struct ethtool_cmd*cmd)
> +{
> + struct bgx_port_priv *p = bgx_port_netdev2priv(netdev);
> +
> + if (!capable(CAP_NET_ADMIN))
> + return -EPERM;
Not required. The enforces this. See dev_ethtool()
> +
> + if (p->phydev)
> + return phy_ethtool_sset(p->phydev, cmd);
> +
> + return -EOPNOTSUPP;
> +}
> +EXPORT_SYMBOL(bgx_port_ethtool_set_settings);
> +
> +int bgx_port_ethtool_nway_reset(struct net_device *netdev)
> +{
> + struct bgx_port_priv *p = bgx_port_netdev2priv(netdev);
> +
> + if (!capable(CAP_NET_ADMIN))
> + return -EPERM;
Also not needed.
> +static void bgx_port_adjust_link(struct net_device *netdev)
> +{
> + struct bgx_port_priv*priv = bgx_port_netdev2priv(netdev);
> + boollink_changed = false;
> + unsigned intlink;
> + unsigned intspeed;
> + unsigned intduplex;
> +
> + mutex_lock(&priv->lock);
> +
> + if (!priv->phydev->link && priv->last_status.link)
> + link_changed = true;
> +
> + if (priv->phydev->link &&
> + (priv->last_status.link != priv->phydev->link ||
> + priv->last_status.duplex != priv->phydev->duplex ||
> + priv->last_status.speed != priv->phydev->speed))
> + link_changed = true;
> +
> + link = priv->phydev->link;
> + priv->last_status.link = priv->phydev->link;
> +
> + speed = priv->phydev->speed;
> + priv->last_status.speed = priv->phydev->speed;
> +
> + duplex = priv->phydev->duplex;
> + priv->last_status.duplex = priv->phydev->duplex;
> +
> + mutex_unlock(&priv->lock);
> +
> + if (link_changed) {
> + struct port_status status;
> +
> + phy_print_status(priv->phydev);
> +
> + status.link = link ? 1 : 0;
> + status.duplex = duplex;
> + status.speed = speed;
> + if (!link) {
> + netif_carrier_off(netdev);
> + /* Let TX drain. FIXME check that it is drained. */
> + mdelay(50);
> +
Re: [PATCH v4 7/8] netdev: octeon-ethernet: Add Cavium Octeon III support.
On Wed, Nov 29, 2017 at 10:11:38PM +0300, Dan Carpenter wrote:
> On Wed, Nov 29, 2017 at 09:37:15PM +0530, Souptick Joarder wrote:
> > >> +static int bgx_port_sgmii_set_link_speed(struct bgx_port_priv *priv,
> > >> struct port_status status)
> > >> +{
> > >> + u64 data;
> > >> + u64 prtx;
> > >> + u64 miscx;
> > >> + int timeout;
> > >> +
> >
> > >> +
> > >> + switch (status.speed) {
> > >> + case 10:
> > >
> > > In my opinion, instead of hard coding the value, is it fine to use ENUM ?
> >Similar comments applicable in other places where hard coded values are
> > used.
> >
>
> 10 means 10M right? That's not really a magic number. It's fine.
There are also :
uapi/linux/ethtool.h:#define SPEED_10 10
uapi/linux/ethtool.h:#define SPEED_100 100
uapi/linux/ethtool.h:#define SPEED_1000 1000
uapi/linux/ethtool.h:#define SPEED_11
uapi/linux/ethtool.h:#define SPEED_10 10
Andrew
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 00/10] posix_clocks: Prepare syscalls for 64 bit time_t conversion
On Wed, Nov 29, 2017 at 12:17 AM, Deepa Dinamani wrote: > On Tue, Nov 28, 2017 at 6:17 AM, Arnd Bergmann wrote: >> On Mon, Nov 27, 2017 at 11:29 PM, Deepa Dinamani >> wrote: > Right. There are three options: > > 1. Use two configs to identify which syscalls need not be supported by > new architectures. > In this case it makes sense to say LEGACY_TIME_SYSCALLS and > COMPAT_32BIT_TIME both need to be disabled for new architectures. And, > I can reword the config to what you mention below. > > 2. Make the LEGACY_TIME_SYSCALLS eliminate non y2038 safe syscalls > mentioned below only. > In this case only the native and compat functions of the below > mentioned syscalls need to be identified by the config. I like this > option as this clearly identifies which syscalls are deprecated and do > not have a 64 bit counterpart. Not all architectures need to support > turning this off. > > 3. If we don't need either 1 or 2, then we could stick with what we > have today in the series as CONFIG_64BIT_TIME will be deleted and they > only need #ifdef CONFIG_64BIT. > > Let me know if anyone prefers something else. I think I prefer to have both LEGACY_TIME_SYSCALLS to guard the native deprecated syscalls (disabled on 32-bit architectures after the conversion, and enabled on 64-bit architectures until we merge the next one), and COMPAT_32BIT_TIME to guard the compat versions of both the deprecated and the non-deprecated syscalls (enabled on all existing 32-bit architectures after the conversion, and on 64-bit architectures if they provide a compat mode for the former). Those two are not symmetric, but I think those are the most common combinations, and the Kconfig symbol helps document what they are. There is one more category for things like io_getevents() and rt_sigtimedwait that also need two separate compat versions, one for 32-bit time_t and one for 64-bit time_t, but it seems better to deal with those case-by-case rather than introducing another Kconfig symbol. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 7/8] netdev: octeon-ethernet: Add Cavium Octeon III support.
On 11/29/2017 08:07 AM, Souptick Joarder wrote:
On Wed, Nov 29, 2017 at 4:00 PM, Souptick Joarder wrote:
On Wed, Nov 29, 2017 at 6:25 AM, David Daney wrote:
From: Carlos Munoz
The Cavium OCTEON cn78xx and cn73xx SoCs have network packet I/O
hardware that is significantly different from previous generations of
the family.
diff --git a/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
b/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
new file mode 100644
index ..4dad35fa4270
--- /dev/null
+++ b/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
@@ -0,0 +1,2033 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2017 Cavium, Inc.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static void bgx_port_sgmii_set_link_down(struct bgx_port_priv *priv)
+{
+ u64 data;
+ data = oct_csr_read(BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx,
priv->index));
+ data |= BIT(11);
+ oct_csr_write(data, BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx,
priv->index));
+ data = oct_csr_read(BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx,
priv->index));
Any particular reason to read immediately after write ?
Yes, to ensure the write is committed to hardware before the next step.
+static int bgx_port_sgmii_set_link_speed(struct bgx_port_priv *priv, struct
port_status status)
+{
+ u64 data;
+ u64 prtx;
+ u64 miscx;
+ int timeout;
+
+
+ switch (status.speed) {
+ case 10:
In my opinion, instead of hard coding the value, is it fine to use ENUM ?
Similar comments applicable in other places where hard coded values are
used.
There is nothing to be gained by interposing an extra layer of
abstraction in this case. The code is more clear with the raw numbers
in this particular case.
+static int bgx_port_gser_27882(struct bgx_port_priv *priv)
+{
+ u64 data;
+ u64 addr;
+ int timeout = 200;
+
+ //timeout = 200;
Better to initialize the timeout value
What are you talking about? It is properly initialized using valid C code.
+static int bgx_port_qlm_rx_equalization(struct bgx_port_priv *priv, int qlm,
int lane)
+{
+ lmode = oct_csr_read(GSER_LANE_MODE(priv->node, qlm));
+ lmode &= 0xf;
+ addr = GSER_LANE_P_MODE_1(priv->node, qlm, lmode);
+ data = oct_csr_read(addr);
+ /* Don't complete rx equalization if in VMA manual mode */
+ if (data & BIT(14))
+ return 0;
+
+ /* Apply rx equalization for speed > 6250 */
+ if (bgx_port_get_qlm_speed(priv, qlm) < 6250)
+ return 0;
+
+ /* Wait until rx data is valid (CDRLOCK) */
+ timeout = 500;
500 us is the min required value or it can be further reduced ?
500 uS works well and is shorter than the 2000 uS from the hardware manual.
If you would like to verify shorter timeout values, we could consider
merging such a patch. But really, this doesn't matter as it is a very
short one-off action when the link is brought up.
+static int bgx_port_init_xaui_link(struct bgx_port_priv *priv)
+{
+
+ if (use_ber) {
+ timeout = 1;
+ do {
+ data =
+ oct_csr_read(BGX_SPU_BR_STATUS1(priv->node,
priv->bgx, priv->index));
+ if (data & BIT(0))
+ break;
+ timeout--;
+ udelay(1);
+ } while (timeout);
In my opinion, it's better to implement similar kind of loops inside macros.
Ok, duly noted. I think we are in disagreement with respect to this point.
+ if (!timeout) {
+ pr_debug("BGX%d:%d:%d: BLK_LOCK timeout\n",
+priv->bgx, priv->index, priv->node);
+ return -1;
+ }
+ } else {
+ timeout = 1;
+ do {
+ data =
+ oct_csr_read(BGX_SPU_BX_STATUS(priv->node,
priv->bgx, priv->index));
+ if (data & BIT(12))
+ break;
+ timeout--;
+ udelay(1);
+ } while (timeout);
same here
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 7/8] netdev: octeon-ethernet: Add Cavium Octeon III support.
On Wed, Nov 29, 2017 at 09:37:15PM +0530, Souptick Joarder wrote:
> >> +static int bgx_port_sgmii_set_link_speed(struct bgx_port_priv *priv,
> >> struct port_status status)
> >> +{
> >> + u64 data;
> >> + u64 prtx;
> >> + u64 miscx;
> >> + int timeout;
> >> +
>
> >> +
> >> + switch (status.speed) {
> >> + case 10:
> >
> > In my opinion, instead of hard coding the value, is it fine to use ENUM ?
>Similar comments applicable in other places where hard coded values are
> used.
>
10 means 10M right? That's not really a magic number. It's fine.
> >> +static int bgx_port_init_xaui_link(struct bgx_port_priv *priv)
> >> +{
>
> >> +
> >> + if (use_ber) {
> >> + timeout = 1;
> >> + do {
> >> + data =
> >> +
> >> oct_csr_read(BGX_SPU_BR_STATUS1(priv->node, priv->bgx, priv->index));
> >> + if (data & BIT(0))
> >> + break;
> >> + timeout--;
> >> + udelay(1);
> >> + } while (timeout);
> >
> > In my opinion, it's better to implement similar kind of loops inside macros.
I don't understand what you mean here. For what it's worth this code
seems clear enough to me (except for the bad indenting of oct_csr_read().
It should be something like:
data =
oct_csr_read(BGX_SPU_BR_STATUS1(priv->node,
priv->bgx, priv->index));
That's over the 80 char limit but so is the original code.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Check for register_shrinker() failure.
On Wed, Nov 29, 2017 at 10:33:39PM +0900, Tetsuo Handa wrote: > register_shrinker() might return -ENOMEM error since Linux 3.12. > But since callers of ion_device_add_heap() are not ready to receive an > error and it is not simple enough to fix within this patch, this patch > just prints a warning line when register_shrinker() failed. > > Signed-off-by: Tetsuo Handa > Cc: John Stultz > Cc: Greg Kroah-Hartman I think you need to update your address book, that old email hasn't been valid for over 5 years now :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: nvec: Fix usleep_range is preferred over udelay
On 11/29/2017 06:00 PM, Joshua Abraham wrote:
Signed-off-by: Joshua Abraham
This patch fixes the issue:
CHECK: usleep_range is preferred over udelay; see
Documentation/timers/timers-howto.txt
---
drivers/staging/nvec/nvec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 4ff8f47385da..2a01ef4b54ff 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -631,7 +631,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
break;
case 2: /* first byte after command */
if (status == (I2C_SL_IRQ | RNW | RCVD)) {
- udelay(33);
+ usleep_range(30, 35);
if (nvec->rx->data[0] != 0x01) {
dev_err(nvec->dev,
"Read without prior read command\n");
This is incorrect, as this function is an interrupt handler and we
cannot sleep in interrupt context.
Cheers,
Mikko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: pi433: pi433_if.c codestyle brace on previous line
This patch fixes the following checkpatch.pl error:
ERROR: that open brace { should be on the previous line
in pi433_if.c as requested by TODO file
Signed-off-by: Oliver Graute
---
drivers/staging/pi433/pi433_if.c | 38 +-
1 file changed, 13 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 0d18412..1515198 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -133,19 +133,14 @@ static irqreturn_t DIO0_irq_handler(int irq, void *dev_id)
{
struct pi433_device *device = dev_id;
- if (device->irq_state[DIO0] == DIO_PacketSent)
- {
+ if (device->irq_state[DIO0] == DIO_PacketSent) {
device->free_in_fifo = FIFO_SIZE;
dev_dbg(device->dev, "DIO0 irq: Packet sent\n");
wake_up_interruptible(&device->fifo_wait_queue);
- }
- else if (device->irq_state[DIO0] == DIO_Rssi_DIO0)
- {
+ } else if (device->irq_state[DIO0] == DIO_Rssi_DIO0) {
dev_dbg(device->dev, "DIO0 irq: RSSI level over threshold\n");
wake_up_interruptible(&device->rx_wait_queue);
- }
- else if (device->irq_state[DIO0] == DIO_PayloadReady)
- {
+ } else if (device->irq_state[DIO0] == DIO_PayloadReady) {
dev_dbg(device->dev, "DIO0 irq: PayloadReady\n");
device->free_in_fifo = 0;
wake_up_interruptible(&device->fifo_wait_queue);
@@ -158,14 +153,13 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
{
struct pi433_device *device = dev_id;
- if (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1)
- {
+ if (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1) {
device->free_in_fifo = FIFO_SIZE;
- }
- else if (device->irq_state[DIO1] == DIO_FifoLevel)
- {
- if (device->rx_active) device->free_in_fifo = FIFO_THRESHOLD -
1;
- elsedevice->free_in_fifo = FIFO_SIZE -
FIFO_THRESHOLD - 1;
+ } else if (device->irq_state[DIO1] == DIO_FifoLevel) {
+ if (device->rx_active)
+ device->free_in_fifo = FIFO_THRESHOLD - 1;
+ else
+ device->free_in_fifo = FIFO_SIZE - FIFO_THRESHOLD - 1;
}
dev_dbg(device->dev,
"DIO1 irq: %d bytes free in fifo\n", device->free_in_fifo);
@@ -198,12 +192,9 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
/* packet config */
/* enable */
SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
- if (rx_cfg->enable_sync == optionOn)
- {
+ if (rx_cfg->enable_sync == optionOn) {
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
afterSyncInterrupt));
- }
- else
- {
+ } else {
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
}
if (rx_cfg->enable_length_byte == optionOn) {
@@ -220,12 +211,9 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
/* lengths */
SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
- if (rx_cfg->enable_length_byte == optionOn)
- {
+ if (rx_cfg->enable_length_byte == optionOn) {
SET_CHECKED(rf69_set_payload_length(dev->spi, 0xff));
- }
- else if (rx_cfg->fixed_message_length != 0)
- {
+ } else if (rx_cfg->fixed_message_length != 0) {
payload_length = rx_cfg->fixed_message_length;
if (rx_cfg->enable_length_byte == optionOn) payload_length++;
if (rx_cfg->enable_address_filtering != filteringOff)
payload_length++;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: pi433: pi433_if.c codestyle fix space around kthread
This patch fixes the following checkpatch.pl errors:
ERROR: space prohibited after that open parenthesis '('
#499: FILE: pi433_if.c:499:
+ if ( kthread_should_stop() )
ERROR: space prohibited before that close parenthesis ')'
#499: FILE: pi433_if.c:499:
+ if ( kthread_should_stop() )
Signed-off-by: Oliver Graute
---
drivers/staging/pi433/pi433_if.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 4b8980c..28bb77a 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -496,7 +496,7 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
wait_event_interruptible(device->tx_wait_queue,
( !kfifo_is_empty(&device->tx_fifo) ||
kthread_should_stop() ));
- if ( kthread_should_stop() )
+ if (kthread_should_stop())
return 0;
/* get data from fifo in the following order:
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: lustre: Replace 'uint32_t' with 'u32' and 'uint64_t' with 'u64'
There are two reasons for that:
1) As Linus Torvalds said we should use kernel types:
http://lkml.iu.edu/hypermail//linux/kernel/1506.0/00160.html
2) There are only few places in the lustre codebase that use such types.
In the most cases it uses 'u32' and 'u64'.
Signed-off-by: Roman Storozhenko
---
In the second version of this patch I forgot to add subsystem and
driver. As Greg K-H mentioned:
"Please fix up the subject to have the subsystem and driver name in it:
Subject: [PATCH] staging: lustre: ..."
In the first version of this patch I replaced 'uint32_t' with '__u32' and
'uint64_t' with '__u64'. I was suggested to fix that by Greg K-H:
"The __ types are only needed for when you cross the user/kernel boundry.
Otherwise just use the "normal" types of u32 and u64.
Do the changes you made here all cross that boundry? If not, please fix
this up."
I asked lustre community whether those code used only in the kernel
space and Andreas Dilger said:
"These headers are for kernel code only, so should use the "u32" and
similar
types, rather than the "__u32" that are used for user-kernel
structures."
So I have replaced my first patch version with this one.
drivers/staging/lustre/lustre/include/lustre_sec.h | 4 ++--
drivers/staging/lustre/lustre/llite/vvp_dev.c | 2 +-
drivers/staging/lustre/lustre/lov/lov_internal.h | 12 ++--
drivers/staging/lustre/lustre/osc/osc_internal.h | 6 +++---
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/lustre/lustre/include/lustre_sec.h
b/drivers/staging/lustre/lustre/include/lustre_sec.h
index a40f706..64b6fd4 100644
--- a/drivers/staging/lustre/lustre/include/lustre_sec.h
+++ b/drivers/staging/lustre/lustre/include/lustre_sec.h
@@ -341,8 +341,8 @@ void sptlrpc_conf_client_adapt(struct obd_device *obd);
#define SPTLRPC_MAX_PAYLOAD (1024)
struct vfs_cred {
- uint32_tvc_uid;
- uint32_tvc_gid;
+ u32 vc_uid;
+ u32 vc_gid;
};
struct ptlrpc_ctx_ops {
diff --git a/drivers/staging/lustre/lustre/llite/vvp_dev.c
b/drivers/staging/lustre/lustre/llite/vvp_dev.c
index 8ccc8b7..987c03b 100644
--- a/drivers/staging/lustre/lustre/llite/vvp_dev.c
+++ b/drivers/staging/lustre/lustre/llite/vvp_dev.c
@@ -384,7 +384,7 @@ int cl_sb_fini(struct super_block *sb)
struct vvp_pgcache_id {
unsigned int vpi_bucket;
unsigned int vpi_depth;
- uint32_t vpi_index;
+ u32 vpi_index;
unsigned int vpi_curdep;
struct lu_object_header *vpi_obj;
diff --git a/drivers/staging/lustre/lustre/lov/lov_internal.h
b/drivers/staging/lustre/lustre/lov/lov_internal.h
index ae28ddf..a56d71c 100644
--- a/drivers/staging/lustre/lustre/lov/lov_internal.h
+++ b/drivers/staging/lustre/lustre/lov/lov_internal.h
@@ -115,19 +115,19 @@ static inline const struct lsm_operations
*lsm_op_find(int magic)
*/
#if BITS_PER_LONG == 64
# define lov_do_div64(n, base) ({ \
- uint64_t __base = (base); \
- uint64_t __rem; \
- __rem = ((uint64_t)(n)) % __base; \
- (n) = ((uint64_t)(n)) / __base; \
+ u64 __base = (base);\
+ u64 __rem; \
+ __rem = ((u64)(n)) % __base;\
+ (n) = ((u64)(n)) / __base; \
__rem; \
})
#elif BITS_PER_LONG == 32
# define lov_do_div64(n, base) ({ \
- uint64_t __rem; \
+ u64 __rem; \
if ((sizeof(base) > 4) && (((base) & 0xULL) != 0)) { \
int __remainder; \
LASSERTF(!((base) & (LOV_MIN_STRIPE_SIZE - 1)), "64 bit lov " \
-"division %llu / %llu\n", (n), (uint64_t)(base));\
+"division %llu / %llu\n", (n), (u64)(base));\
__remainder = (n) & (LOV_MIN_STRIPE_SIZE - 1); \
(n) >>= LOV_MIN_STRIPE_BITS;\
__rem = do_div(n, (base) >> LOV_MIN_STRIPE_BITS); \
diff --git a/drivers/staging/lustre/lustre/osc/osc_internal.h
b/drivers/staging/lustre/lustre/osc/osc_internal.h
index feda61b..32db150 100644
--- a/drivers/staging/lustre/lustre/osc/osc_internal.h
+++ b/drivers/staging/lustre/lustre/osc/osc_internal.h
@@ -168,9 +168,9 @@ struct osc_device {
/* Write stats is actually protected by client_obd's lock. */
struct osc_stats {
-
Re: [PATCH] staging: nvec: Fix usleep_range is preferred over udelay
On Wed, Nov 29, 2017 at 06:07:53PM +0200, Mikko Perttunen wrote:
> On 11/29/2017 06:00 PM, Joshua Abraham wrote:
> > Signed-off-by: Joshua Abraham
> >
> > This patch fixes the issue:
> >
> > CHECK: usleep_range is preferred over udelay; see
> > Documentation/timers/timers-howto.txt
> >
> > ---
> > drivers/staging/nvec/nvec.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> > index 4ff8f47385da..2a01ef4b54ff 100644
> > --- a/drivers/staging/nvec/nvec.c
> > +++ b/drivers/staging/nvec/nvec.c
> > @@ -631,7 +631,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
> > break;
> > case 2: /* first byte after command */
> > if (status == (I2C_SL_IRQ | RNW | RCVD)) {
> > - udelay(33);
> > + usleep_range(30, 35);
> > if (nvec->rx->data[0] != 0x01) {
> > dev_err(nvec->dev,
> > "Read without prior read command\n");
> >
>
> This is incorrect, as this function is an interrupt handler and we cannot
> sleep in interrupt context.
>
> Cheers,
> Mikko
My mistake. Thank you for the feedback!
-Josh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 7/8] netdev: octeon-ethernet: Add Cavium Octeon III support.
On Wed, Nov 29, 2017 at 4:00 PM, Souptick Joarder wrote:
> On Wed, Nov 29, 2017 at 6:25 AM, David Daney wrote:
>> From: Carlos Munoz
>>
>> The Cavium OCTEON cn78xx and cn73xx SoCs have network packet I/O
>> hardware that is significantly different from previous generations of
>> the family.
>> diff --git a/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
>> b/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
>> new file mode 100644
>> index ..4dad35fa4270
>> --- /dev/null
>> +++ b/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
>> @@ -0,0 +1,2033 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2017 Cavium, Inc.
>> + *
>> + * This file is subject to the terms and conditions of the GNU General
>> Public
>> + * License. See the file "COPYING" in the main directory of this archive
>> + * for more details.
>> + */
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +static void bgx_port_sgmii_set_link_down(struct bgx_port_priv *priv)
>> +{
>> + u64 data;
>> + data = oct_csr_read(BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx,
>> priv->index));
>> + data |= BIT(11);
>> + oct_csr_write(data, BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx,
>> priv->index));
>> + data = oct_csr_read(BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx,
>> priv->index));
>
> Any particular reason to read immediately after write ?
>> +static int bgx_port_sgmii_set_link_speed(struct bgx_port_priv *priv, struct
>> port_status status)
>> +{
>> + u64 data;
>> + u64 prtx;
>> + u64 miscx;
>> + int timeout;
>> +
>> +
>> + switch (status.speed) {
>> + case 10:
>
> In my opinion, instead of hard coding the value, is it fine to use ENUM ?
Similar comments applicable in other places where hard coded values are used.
>> +static int bgx_port_gser_27882(struct bgx_port_priv *priv)
>> +{
>> + u64 data;
>> + u64 addr;
>
>> + int timeout = 200;
>> +
>> + //timeout = 200;
Better to initialize the timeout value
>> +static int bgx_port_qlm_rx_equalization(struct bgx_port_priv *priv, int
>> qlm, int lane)
>> +{
>> + lmode = oct_csr_read(GSER_LANE_MODE(priv->node, qlm));
>> + lmode &= 0xf;
>> + addr = GSER_LANE_P_MODE_1(priv->node, qlm, lmode);
>> + data = oct_csr_read(addr);
>> + /* Don't complete rx equalization if in VMA manual mode */
>> + if (data & BIT(14))
>> + return 0;
>> +
>> + /* Apply rx equalization for speed > 6250 */
>> + if (bgx_port_get_qlm_speed(priv, qlm) < 6250)
>> + return 0;
>> +
>> + /* Wait until rx data is valid (CDRLOCK) */
>> + timeout = 500;
>
> 500 us is the min required value or it can be further reduced ?
>> +static int bgx_port_init_xaui_link(struct bgx_port_priv *priv)
>> +{
>> +
>> + if (use_ber) {
>> + timeout = 1;
>> + do {
>> + data =
>> + oct_csr_read(BGX_SPU_BR_STATUS1(priv->node,
>> priv->bgx, priv->index));
>> + if (data & BIT(0))
>> + break;
>> + timeout--;
>> + udelay(1);
>> + } while (timeout);
>
> In my opinion, it's better to implement similar kind of loops inside macros.
>
>> + if (!timeout) {
>> + pr_debug("BGX%d:%d:%d: BLK_LOCK timeout\n",
>> +priv->bgx, priv->index, priv->node);
>> + return -1;
>> + }
>> + } else {
>> + timeout = 1;
>> + do {
>> + data =
>> + oct_csr_read(BGX_SPU_BX_STATUS(priv->node,
>> priv->bgx, priv->index));
>> + if (data & BIT(12))
>> + break;
>> + timeout--;
>> + udelay(1);
>> + } while (timeout);
> same here
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: nvec: Fix usleep_range is preferred over udelay
Signed-off-by: Joshua Abraham
This patch fixes the issue:
CHECK: usleep_range is preferred over udelay; see
Documentation/timers/timers-howto.txt
---
drivers/staging/nvec/nvec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 4ff8f47385da..2a01ef4b54ff 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -631,7 +631,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
break;
case 2: /* first byte after command */
if (status == (I2C_SL_IRQ | RNW | RCVD)) {
- udelay(33);
+ usleep_range(30, 35);
if (nvec->rx->data[0] != 0x01) {
dev_err(nvec->dev,
"Read without prior read command\n");
--
2.15.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH Resend] staging: media: cxd2099: style fix - replace hard-coded function names
This patch fixes the remaining coding style warnings in the cxd2099
module. Instead of hard coding the function name the __func__ variable
should be used.
It fixes the following checkpatch.pl warning:
WARNING: Prefer using '"%s...", __func__' to using 'i2c_read_reg', this
function's name, in a string
Signed-off-by: Martin Homuth
---
drivers/staging/media/cxd2099/cxd2099.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/cxd2099/cxd2099.c
b/drivers/staging/media/cxd2099/cxd2099.c
index 3e30f48..6641dd2 100644
--- a/drivers/staging/media/cxd2099/cxd2099.c
+++ b/drivers/staging/media/cxd2099/cxd2099.c
@@ -101,7 +101,7 @@ static int i2c_read_reg(struct i2c_adapter *adapter,
u8 adr,
.buf = val, .len = 1} };
if (i2c_transfer(adapter, msgs, 2) != 2) {
- dev_err(&adapter->dev, "error in i2c_read_reg\n");
+ dev_err(&adapter->dev, "error in %s\n", __func__);
return -1;
}
return 0;
@@ -116,7 +116,7 @@ static int i2c_read(struct i2c_adapter *adapter, u8 adr,
.buf = data, .len = n} };
if (i2c_transfer(adapter, msgs, 2) != 2) {
- dev_err(&adapter->dev, "error in i2c_read\n");
+ dev_err(&adapter->dev, "error in %s\n", __func__);
return -1;
}
return 0;
--
2.10.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH Resend] staging: media: cxd2099: style fix - replace hard-coded function names
This patch fixes the remaining coding style warnings in the cxd2099
module. Instead of hard coding the function name the __func__ variable
should be used.
It fixes the following checkpatch.pl warning:
WARNING: Prefer using '"%s...", __func__' to using 'i2c_read_reg', this
function's name, in a string
Signed-off-by: Martin Homuth
---
drivers/staging/media/cxd2099/cxd2099.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/cxd2099/cxd2099.c
b/drivers/staging/media/cxd2099/cxd2099.c
index 3e30f48..6641dd2 100644
--- a/drivers/staging/media/cxd2099/cxd2099.c
+++ b/drivers/staging/media/cxd2099/cxd2099.c
@@ -101,7 +101,7 @@ static int i2c_read_reg(struct i2c_adapter *adapter,
u8 adr,
.buf = val, .len = 1} };
if (i2c_transfer(adapter, msgs, 2) != 2) {
- dev_err(&adapter->dev, "error in i2c_read_reg\n");
+ dev_err(&adapter->dev, "error in %s\n", __func__);
return -1;
}
return 0;
@@ -116,7 +116,7 @@ static int i2c_read(struct i2c_adapter *adapter, u8 adr,
.buf = data, .len = n} };
if (i2c_transfer(adapter, msgs, 2) != 2) {
- dev_err(&adapter->dev, "error in i2c_read\n");
+ dev_err(&adapter->dev, "error in %s\n", __func__);
return -1;
}
return 0;
--
2.10.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: media: cxd2099: style fix - replace hard-coded function names
This patch fixes the remaining coding style warnings in the lirc module.
Instead of hard coding the function name the __func__ variable
should be used.
It fixes the following checkpatch.pl warning:
WARNING: Prefer using '"%s...", __func__' to using 'read', this
function's name, in a string
Signed-off-by: Martin Homuth
---
drivers/staging/media/cxd2099/cxd2099.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/cxd2099/cxd2099.c
b/drivers/staging/media/cxd2099/cxd2099.c
index 3e30f48..6641dd2 100644
--- a/drivers/staging/media/cxd2099/cxd2099.c
+++ b/drivers/staging/media/cxd2099/cxd2099.c
@@ -101,7 +101,7 @@ static int i2c_read_reg(struct i2c_adapter *adapter,
u8 adr,
.buf = val, .len = 1} };
if (i2c_transfer(adapter, msgs, 2) != 2) {
- dev_err(&adapter->dev, "error in i2c_read_reg\n");
+ dev_err(&adapter->dev, "error in %s\n", __func__);
return -1;
}
return 0;
@@ -116,7 +116,7 @@ static int i2c_read(struct i2c_adapter *adapter, u8 adr,
.buf = data, .len = n} };
if (i2c_transfer(adapter, msgs, 2) != 2) {
- dev_err(&adapter->dev, "error in i2c_read\n");
+ dev_err(&adapter->dev, "error in %s\n", __func__);
return -1;
}
return 0;
--
2.10.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ion: Check for register_shrinker() failure.
register_shrinker() might return -ENOMEM error since Linux 3.12.
But since callers of ion_device_add_heap() are not ready to receive an
error and it is not simple enough to fix within this patch, this patch
just prints a warning line when register_shrinker() failed.
Signed-off-by: Tetsuo Handa
Cc: John Stultz
Cc: Greg Kroah-Hartman
Cc: Michal Hocko
---
drivers/staging/android/ion/ion_heap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion_heap.c
b/drivers/staging/android/ion/ion_heap.c
index 91faa7f..56dcbd8 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -312,5 +312,6 @@ void ion_heap_init_shrinker(struct ion_heap *heap)
heap->shrinker.scan_objects = ion_heap_shrink_scan;
heap->shrinker.seeks = DEFAULT_SEEKS;
heap->shrinker.batch = 0;
- register_shrinker(&heap->shrinker);
+ if (register_shrinker(&heap->shrinker))
+ pr_err("Failed to create heap shrinker for %s\n", heap->name);
}
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 0/8] Cavium OCTEON-III network driver.
The net-next tree is closed, please resubmit this when the net-next tree opens again. Thank you. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] android: binder: Check for errors in binder_alloc_shrinker_init().
Both list_lru_init() and register_shrinker() might return an error.
Signed-off-by: Tetsuo Handa
Cc: Sherry Yang
Cc: Greg Kroah-Hartman
Cc: Michal Hocko
---
drivers/android/binder.c | 4 +++-
drivers/android/binder_alloc.c | 12 +---
drivers/android/binder_alloc.h | 2 +-
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 85b0bb4..a54a0f1 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5569,7 +5569,9 @@ static int __init binder_init(void)
struct binder_device *device;
struct hlist_node *tmp;
- binder_alloc_shrinker_init();
+ ret = binder_alloc_shrinker_init();
+ if (ret)
+ return ret;
atomic_set(&binder_transaction_log.cur, ~0U);
atomic_set(&binder_transaction_log_failed.cur, ~0U);
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 0dba2308..fdf9d9f 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -1006,8 +1006,14 @@ void binder_alloc_init(struct binder_alloc *alloc)
INIT_LIST_HEAD(&alloc->buffers);
}
-void binder_alloc_shrinker_init(void)
+int binder_alloc_shrinker_init(void)
{
- list_lru_init(&binder_alloc_lru);
- register_shrinker(&binder_shrinker);
+ int ret = list_lru_init(&binder_alloc_lru);
+
+ if (ret == 0) {
+ ret = register_shrinker(&binder_shrinker);
+ if (ret)
+ list_lru_destroy(&binder_alloc_lru);
+ }
+ return ret;
}
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index 0b14530..9ef64e5 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -130,7 +130,7 @@ extern struct binder_buffer *binder_alloc_new_buf(struct
binder_alloc *alloc,
size_t extra_buffers_size,
int is_async);
extern void binder_alloc_init(struct binder_alloc *alloc);
-void binder_alloc_shrinker_init(void);
+extern int binder_alloc_shrinker_init(void);
extern void binder_alloc_vma_close(struct binder_alloc *alloc);
extern struct binder_buffer *
binder_alloc_prepare_to_free(struct binder_alloc *alloc,
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ashmem: Check for register_shrinker() failure.
register_shrinker() might return -ENOMEM error since Linux 3.12.
Signed-off-by: Tetsuo Handa
Cc: Robert Love
Cc: Marco Nelissen
Cc: John Stultz
Cc: Greg Kroah-Hartman
Cc: Michal Hocko
---
drivers/staging/android/ashmem.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 0f695df..ab56f81 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -862,12 +862,18 @@ static int __init ashmem_init(void)
goto out_free2;
}
- register_shrinker(&ashmem_shrinker);
+ ret = register_shrinker(&ashmem_shrinker);
+ if (unlikely(ret)) {
+ pr_err("failed to register shrinker\n");
+ goto out_free3;
+ }
pr_info("initialized\n");
return 0;
+out_free3:
+ misc_deregister(&ashmem_misc);
out_free2:
kmem_cache_destroy(ashmem_range_cachep);
out_free1:
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 2/2] staging: ion: create one device entry per heap
2017-11-28 14:32 GMT+01:00 Greg KH :
> On Mon, Nov 06, 2017 at 04:59:45PM +0100, Benjamin Gaignard wrote:
>> Instead a getting only one common device "/dev/ion" for
>> all the heaps this patch allow to create one device
>> entry ("/dev/ionX") per heap.
>> Getting an entry per heap could allow to set security rules
>> per heap and global ones for all heaps.
>>
>> Allocation requests will be only allowed if the mask_id
>> match with device minor.
>> Query request could be done on any of the devices.
>>
>> Signed-off-by: Benjamin Gaignard
>> ---
>> drivers/staging/android/TODO| 1 -
>> drivers/staging/android/ion/Kconfig | 7
>> drivers/staging/android/ion/ion-ioctl.c | 18 --
>> drivers/staging/android/ion/ion.c | 62
>> +
>> drivers/staging/android/ion/ion.h | 15 ++--
>> 5 files changed, 91 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
>> index 687e0ea..8a11931 100644
>> --- a/drivers/staging/android/TODO
>> +++ b/drivers/staging/android/TODO
>> @@ -8,7 +8,6 @@ TODO:
>> ion/
>> - Add dt-bindings for remaining heaps (chunk and carveout heaps). This
>> would
>> involve putting appropriate bindings in a memory node for Ion to find.
>> - - Split /dev/ion up into multiple nodes (e.g. /dev/ion/heap0)
>> - Better test framework (integration with VGEM was suggested)
>>
>> Please send patches to Greg Kroah-Hartman and Cc:
>> diff --git a/drivers/staging/android/ion/Kconfig
>> b/drivers/staging/android/ion/Kconfig
>> index a517b2d..cb4666e 100644
>> --- a/drivers/staging/android/ion/Kconfig
>> +++ b/drivers/staging/android/ion/Kconfig
>> @@ -10,6 +10,13 @@ menuconfig ION
>> If you're not using Android its probably safe to
>> say N here.
>>
>> +config ION_LEGACY_DEVICE_API
>> + bool "Keep using Ion legacy misc device API"
>> + depends on ION
>
> You want to default to Y here, so when you do 'make oldconfig' nothing
> breaks, right?
>
I will add it.
>> + help
>> + Choose this option to keep using Ion legacy misc device API
>> + i.e. /dev/ion
>
> You need more text here to describe the trade offs. Why would I not
> want to keep doing this? What does turning this off get me? What does
> keeping it on keep me from doing?
>
Does describe it like that sound better ?
"Choose this option to keep using ION legacy misc device API
i.e. /dev/ion. If this option isn't selected you will only
have per heap device node (i.e /dev/ionX) and allocating buffer
from an unique device node won't be possible."
>> +
>> config ION_SYSTEM_HEAP
>> bool "Ion system heap"
>> depends on ION
>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>> b/drivers/staging/android/ion/ion-ioctl.c
>> index e26b786..bb5c77b 100644
>> --- a/drivers/staging/android/ion/ion-ioctl.c
>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>> @@ -25,7 +25,8 @@ union ion_ioctl_arg {
>> struct ion_heap_query query;
>> };
>>
>> -static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
>> +static int validate_ioctl_arg(struct file *filp,
>> + unsigned int cmd, union ion_ioctl_arg *arg)
>> {
>> switch (cmd) {
>> case ION_IOC_HEAP_QUERY:
>> @@ -34,6 +35,19 @@ static int validate_ioctl_arg(unsigned int cmd, union
>> ion_ioctl_arg *arg)
>> arg->query.reserved2 )
>> return -EINVAL;
>> break;
>> +
>> + case ION_IOC_ALLOC:
>> + {
>> + int mask = 1 << iminor(filp->f_inode);
>> +
>> +#ifdef CONFIG_ION_LEGACY_DEVICE_API
>> + if (imajor(filp->f_inode) == MISC_MAJOR)
>> + return 0;
>
> Why return 0? Because it is already allocated?
No it is because in legacy mode all mask are valids so to keep legacy behavoir
it will not test if the requested heap match with the device.
I will add a comment about that.
>
> Some comments here as to exactly what you are doing would be nice.
>
>> +#endif
>> + if (!(arg->allocation.heap_id_mask & mask))
>
> This doesn't allocate anthing, just check to see if it is?
>
No this function only check if ioctl args are correct.
>> + return -EINVAL;
>> + break;
>> + }
>> default:
>> break;
>> }
>> @@ -69,7 +83,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
>> unsigned long arg)
>> if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
>> return -EFAULT;
>>
>> - ret = validate_ioctl_arg(cmd, &data);
>> + ret = validate_ioctl_arg(filp, cmd, &data);
>> if (WARN_ON_ONCE(ret))
>> return ret;
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index fda9756..2c2568b 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -40,6 +40,9 @@
>>
>> #includ
Re: [PATCH v2 1/3] media: staging: atomisp: fix for sparse "using plain integer as NULL pointer" warnings.
> There are 35 defaults defined by macros like this, most of them much > more complicated that IA_CSS_DEFAULT_ISP_MEM_PARAMS, and a few members > are initialized to non-zero values. My plan, therefore, is to convert > everything to use designated initializers, and then start removing the > zeroes afterwards. Where they are only used once in the tree it might be even cleaner to just do static struct foo = FOO_DEFAULT_BAR; foo.x = 12; foo.bar = &foo; etc in the code. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 7/8] netdev: octeon-ethernet: Add Cavium Octeon III support.
On Wed, Nov 29, 2017 at 04:00:01PM +0530, Souptick Joarder wrote: Hi Souptick Please trim the code when giving reviews. We don't want to have to page through 8K lines of code it find a few comments mixed in. Just keep the beginning of the function you are commented on to make the context clear. Cut the rest. Thanks Andrew ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/7] media: atomisp: stop producing hundreds of kernel-doc warnings
On Wed, Nov 29, 2017 at 10:28:26AM -0200, Mauro Carvalho Chehab wrote: > Em Wed, 29 Nov 2017 10:24:57 -0200 > Mauro Carvalho Chehab escreveu: > > > Em Wed, 29 Nov 2017 14:14:54 +0200 > > Sakari Ailus escreveu: > > > > > Hi Mauro, > > > > > > Thanks for the patch. > > > > > > On Wed, Nov 29, 2017 at 07:08:04AM -0500, Mauro Carvalho Chehab wrote: > > > > A recent change on Kernel 4.15-rc1 causes all tags with > > > > /** to be handled as kernel-doc markups. Well, several > > > > atomisp modules, it doesn't use kernel-doc, but some other > > > > documentation markup (doxygen?). > > > > > > > > So, suppress all those warns by replacing /** by /*. > > > > > > > > Signed-off-by: Mauro Carvalho Chehab > > > > > > I presume you haven't written the patch manually. There are other changes > > > that described by the comment, too, such as removing lesser > > > than-characaters. > > > > > > It'd be good to mention how it's been generated. > > > > Yeah, I used a simple script, and manually fixed a few minor things > > that were still causing warnings. > > > > The core changes were done via: > > > > for i in $(find drivers/staging/media/atomisp -type f); do sed 's,/\*\* > > ,/\*, ' -i $i; done > > for i in $(find drivers/staging/media/atomisp -type f); do sed > > 's,/\*\*<,/\**,' -i $i; done > > for i in > > drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c > > drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_sp.c > > drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/rmgr/src/rmgr_vbuf.c; > > do perl -ne 's,\/\*\*$,/*,g; print $_' $i > a && mv a $i; done; > > > > I'll add it at the patch description. > > > > Thanks, > > Mauro > > Changed patch description to the one enclosed. > > > commit 699a8caa3de69b2bdaa54b9347c29644bca6 > Author: Mauro Carvalho Chehab > Date: Wed Nov 29 03:16:17 2017 -0500 > > media: atomisp: stop producing hundreds of kernel-doc warnings > > A recent change on Kernel 4.15-rc1 causes all tags with > /** to be handled as kernel-doc markups. Well, several > atomisp modules, it doesn't use kernel-doc, but some other > documentation markup (doxygen?). > > So, suppress all those warns by: > - replacing /**< by /**. > - replacing /** by /*. > > The core changes were done with: > > for i in $(find drivers/staging/media/atomisp -type f); do sed > 's,/\*\* ,/\*, ' -i $i; done > for i in $(find drivers/staging/media/atomisp -type f); do sed > 's,/\*\*<,/\**,' -i $i; done > for i in > drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c > drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_sp.c > drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/rmgr/src/rmgr_vbuf.c; > do perl -ne 's,\/\*\*$,/*,g; print $_' $i > a && mv a $i; done; > > A few manual adjustments were made, where needed. > > Signed-off-by: Mauro Carvalho Chehab Thanks. The patch doesn't conflict with the other atomisp patches I have already so feel free to apply it directly. Acked-by: Sakari Ailus -- Sakari Ailus sakari.ai...@linux.intel.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/7] media: atomisp: stop producing hundreds of kernel-doc warnings
Em Wed, 29 Nov 2017 10:24:57 -0200 Mauro Carvalho Chehab escreveu: > Em Wed, 29 Nov 2017 14:14:54 +0200 > Sakari Ailus escreveu: > > > Hi Mauro, > > > > Thanks for the patch. > > > > On Wed, Nov 29, 2017 at 07:08:04AM -0500, Mauro Carvalho Chehab wrote: > > > A recent change on Kernel 4.15-rc1 causes all tags with > > > /** to be handled as kernel-doc markups. Well, several > > > atomisp modules, it doesn't use kernel-doc, but some other > > > documentation markup (doxygen?). > > > > > > So, suppress all those warns by replacing /** by /*. > > > > > > Signed-off-by: Mauro Carvalho Chehab > > > > I presume you haven't written the patch manually. There are other changes > > that described by the comment, too, such as removing lesser > > than-characaters. > > > > It'd be good to mention how it's been generated. > > Yeah, I used a simple script, and manually fixed a few minor things > that were still causing warnings. > > The core changes were done via: > > for i in $(find drivers/staging/media/atomisp -type f); do sed 's,/\*\* > ,/\*, ' -i $i; done > for i in $(find drivers/staging/media/atomisp -type f); do sed > 's,/\*\*<,/\**,' -i $i; done > for i in > drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c > drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_sp.c > drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/rmgr/src/rmgr_vbuf.c; > do perl -ne 's,\/\*\*$,/*,g; print $_' $i > a && mv a $i; done; > > I'll add it at the patch description. > > Thanks, > Mauro Changed patch description to the one enclosed. commit 699a8caa3de69b2bdaa54b9347c29644bca6 Author: Mauro Carvalho Chehab Date: Wed Nov 29 03:16:17 2017 -0500 media: atomisp: stop producing hundreds of kernel-doc warnings A recent change on Kernel 4.15-rc1 causes all tags with /** to be handled as kernel-doc markups. Well, several atomisp modules, it doesn't use kernel-doc, but some other documentation markup (doxygen?). So, suppress all those warns by: - replacing /**< by /**. - replacing /** by /*. The core changes were done with: for i in $(find drivers/staging/media/atomisp -type f); do sed 's,/\*\* ,/\*, ' -i $i; done for i in $(find drivers/staging/media/atomisp -type f); do sed 's,/\*\*<,/\**,' -i $i; done for i in drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_sp.c drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/rmgr/src/rmgr_vbuf.c; do perl -ne 's,\/\*\*$,/*,g; print $_' $i > a && mv a $i; done; A few manual adjustments were made, where needed. Signed-off-by: Mauro Carvalho Chehab Thanks, Mauro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/7] media: atomisp: stop producing hundreds of kernel-doc warnings
Em Wed, 29 Nov 2017 14:14:54 +0200 Sakari Ailus escreveu: > Hi Mauro, > > Thanks for the patch. > > On Wed, Nov 29, 2017 at 07:08:04AM -0500, Mauro Carvalho Chehab wrote: > > A recent change on Kernel 4.15-rc1 causes all tags with > > /** to be handled as kernel-doc markups. Well, several > > atomisp modules, it doesn't use kernel-doc, but some other > > documentation markup (doxygen?). > > > > So, suppress all those warns by replacing /** by /*. > > > > Signed-off-by: Mauro Carvalho Chehab > > I presume you haven't written the patch manually. There are other changes > that described by the comment, too, such as removing lesser > than-characaters. > > It'd be good to mention how it's been generated. Yeah, I used a simple script, and manually fixed a few minor things that were still causing warnings. The core changes were done via: for i in $(find drivers/staging/media/atomisp -type f); do sed 's,/\*\* ,/\*, ' -i $i; done for i in $(find drivers/staging/media/atomisp -type f); do sed 's,/\*\*<,/\**,' -i $i; done for i in drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_sp.c drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/rmgr/src/rmgr_vbuf.c; do perl -ne 's,\/\*\*$,/*,g; print $_' $i > a && mv a $i; done; I'll add it at the patch description. Thanks, Mauro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/7] media: atomisp: stop producing hundreds of kernel-doc warnings
Hi Mauro, Thanks for the patch. On Wed, Nov 29, 2017 at 07:08:04AM -0500, Mauro Carvalho Chehab wrote: > A recent change on Kernel 4.15-rc1 causes all tags with > /** to be handled as kernel-doc markups. Well, several > atomisp modules, it doesn't use kernel-doc, but some other > documentation markup (doxygen?). > > So, suppress all those warns by replacing /** by /*. > > Signed-off-by: Mauro Carvalho Chehab I presume you haven't written the patch manually. There are other changes that described by the comment, too, such as removing lesser than-characaters. It'd be good to mention how it's been generated. -- Kind regards, Sakari Ailus sakari.ai...@linux.intel.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH Resend] staging: media: lirc: style fix - replace hard-coded function names
On Tue, Nov 28, 2017 at 06:47:08PM +0100, Martin Homuth wrote:
> This patch fixes the remaining coding style warnings in the lirc module.
> Instead of hard coding the function name the __func__ variable
> should be used.
>
> It fixes the following checkpatch.pl warning:
>
> WARNING: Prefer using '"%s...", __func__' to using 'read', this
> function's name, in a string
>
> Signed-off-by: Martin Homuth
> ---
> drivers/staging/media/lirc/lirc_zilog.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/lirc/lirc_zilog.c
> b/drivers/staging/media/lirc/lirc_zilog.c
> index 6bd0717bf76e..be68ee652071 100644
> --- a/drivers/staging/media/lirc/lirc_zilog.c
> +++ b/drivers/staging/media/lirc/lirc_zilog.c
I'm afraid that lirc_zilog has been re-written.
https://patchwork.linuxtv.org/patch/45189/
It hasn't been merged yet, but I suspect that is imminent.
Sean
> @@ -888,9 +888,9 @@ static ssize_t read(struct file *filep, char __user
> *outbuf, size_t n,
> unsigned int m;
> DECLARE_WAITQUEUE(wait, current);
>
> - dev_dbg(ir->dev, "read called\n");
> + dev_dbg(ir->dev, "%s called\n", __func__);
> if (n % rbuf->chunk_size) {
> - dev_dbg(ir->dev, "read result = -EINVAL\n");
> + dev_dbg(ir->dev, "%s result = -EINVAL\n", __func__);
> return -EINVAL;
> }
>
> @@ -949,7 +949,7 @@ static ssize_t read(struct file *filep, char __user
> *outbuf, size_t n,
> retries++;
> }
> if (retries >= 5) {
> - dev_err(ir->dev, "Buffer read failed!\n");
> + dev_err(ir->dev, "%s failed!\n", __func__);
> ret = -EIO;
> }
> }
> @@ -959,7 +959,7 @@ static ssize_t read(struct file *filep, char __user
> *outbuf, size_t n,
> put_ir_rx(rx, false);
> set_current_state(TASK_RUNNING);
>
> - dev_dbg(ir->dev, "read result = %d (%s)\n", ret,
> + dev_dbg(ir->dev, "%s result = %d (%s)\n", __func__, ret,
> ret ? "Error" : "OK");
>
> return ret ? ret : written;
> --
> 2.13.6
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] Replace 'uint32_t' with 'u32' and 'uint64_t' with 'u64'
On Fri, Nov 17, 2017 at 02:46:18PM +0300, Roman Storozhenko wrote: > There are two reasons for that: > 1) As Linus Torvalds said we should use kernel types: > http://lkml.iu.edu/hypermail//linux/kernel/1506.0/00160.html > > 2) There are only few places in the lustre codebase that use such types. > In the most cases it uses 'u32' and 'u64'. > > Signed-off-by: Roman Storozhenko > > --- > In the first version of this patch I replaced 'uint32_t' with '__u32' and > 'uint64_t' with '__u64'. I was suggested to fix that by Greg K-H: > > "The __ types are only needed for when you cross the user/kernel boundry. > Otherwise just use the "normal" types of u32 and u64. > > Do the changes you made here all cross that boundry? If not, please fix > this up." > > I asked lustre community whether those code used only in the kernel > space and Andreas Dilger said: > > "These headers are for kernel code only, so should use the "u32" and > similar > types, rather than the "__u32" that are used for user-kernel > structures." > > So I have replaced my first patch version with this one. Please fix up the subject to have the subsystem and driver name in it: Subject: [PATCH] staging: lustre: ... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] media: staging: atomisp: defined as static some const arrays which don't need external linkage.
On Wed, Nov 29, 2017 at 10:15:06AM +0100, Greg KH wrote: > On Wed, Nov 29, 2017 at 11:08:17AM +0200, Sakari Ailus wrote: > > Hi Greg, > > > > On Mon, Nov 27, 2017 at 01:21:25PM +0100, Greg KH wrote: > > > On Mon, Nov 27, 2017 at 11:30:53AM +, Jeremy Sowden wrote: > > > > Signed-off-by: Jeremy Sowden > > > > --- > > > > .../isp/kernels/eed1_8/ia_css_eed1_8.host.c| 24 > > > > +++--- > > > > 1 file changed, 12 insertions(+), 12 deletions(-) > > > > > > I can never take patches without any changelog text, and so no one > > > should write them that way :) > > > > I've been applying the atomisp patches to my tree for some time now, > > further on passing them to Mauro to be merged via the media tree. To avoid > > conflicts, I suggest to avoid merging them via the staging tree. > > I'm not touching any of these patches, just commenting that patches > without changelogs should not be written :) Ack, just wanted to avoid potential mege conflicts. Thanks. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] media: staging: atomisp: defined as static some const arrays which don't need external linkage.
On Wed, Nov 29, 2017 at 11:08:17AM +0200, Sakari Ailus wrote: > Hi Greg, > > On Mon, Nov 27, 2017 at 01:21:25PM +0100, Greg KH wrote: > > On Mon, Nov 27, 2017 at 11:30:53AM +, Jeremy Sowden wrote: > > > Signed-off-by: Jeremy Sowden > > > --- > > > .../isp/kernels/eed1_8/ia_css_eed1_8.host.c| 24 > > > +++--- > > > 1 file changed, 12 insertions(+), 12 deletions(-) > > > > I can never take patches without any changelog text, and so no one > > should write them that way :) > > I've been applying the atomisp patches to my tree for some time now, > further on passing them to Mauro to be merged via the media tree. To avoid > conflicts, I suggest to avoid merging them via the staging tree. I'm not touching any of these patches, just commenting that patches without changelogs should not be written :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] media: staging: atomisp: defined as static some const arrays which don't need external linkage.
Hi Greg, On Mon, Nov 27, 2017 at 01:21:25PM +0100, Greg KH wrote: > On Mon, Nov 27, 2017 at 11:30:53AM +, Jeremy Sowden wrote: > > Signed-off-by: Jeremy Sowden > > --- > > .../isp/kernels/eed1_8/ia_css_eed1_8.host.c| 24 > > +++--- > > 1 file changed, 12 insertions(+), 12 deletions(-) > > I can never take patches without any changelog text, and so no one > should write them that way :) I've been applying the atomisp patches to my tree for some time now, further on passing them to Mauro to be merged via the media tree. To avoid conflicts, I suggest to avoid merging them via the staging tree. Thanks. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
I need your cooperation
Dear Friend, I know that this mail will come to you as a surprise as we have never met before, but need not to worry as I am contacting you independently of my investigation and no one is informed of this communication. I need your urgent assistance in transferring the sum of $11.3million immediately to your private account.The money has been here in our Bank lying dormant for years now without anybody coming for the claim of it. I want to release the money to you as the relative to our deceased customer (the account owner) who died a long with his supposed NEXT OF KIN since 16th October 2005. The Banking laws here does not allow such money to stay more than 12 years, because the money will be recalled to the Bank treasury account as unclaimed fund. By indicating your interest I will send you the full details on how the business will be executed. Please respond urgently and delete if you are not interested. Best Regards, Mr. Ahmed Hassan. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/3] Sparse fixes for the Atom ISP Staging Driver
On Tue, Nov 28, 2017 at 10:27:24AM +, Jeremy Sowden wrote: > Fixed some sparse warnings in the Atom ISP staging driver. > > This time with longer commit messages. :) > > I've chosen to ignore checkpatch.pl's suggestion to change the types of > the arrays in the second patch from int16_t to s16. > > Jeremy Sowden (2): > media: staging: atomisp: fix for sparse "using plain integer as NULL > pointer" warnings. > media: staging: atomisp: fixes for "symbol was not declared. Should it > be static?" sparse warnings. > > .../isp/kernels/eed1_8/ia_css_eed1_8.host.c| 24 > +++--- > .../isp_param/interface/ia_css_isp_param_types.h | 2 +- > 2 files changed, 13 insertions(+), 13 deletions(-) Thanks, applied! -- Sakari Ailus e-mail: sakari.ai...@iki.fi ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] media: staging: atomisp: prefer s16 to int16_t.
On Mon, Nov 27, 2017 at 11:30:54AM +, Jeremy Sowden wrote: > Signed-off-by: Jeremy Sowden I'd just leave it as-is, int16_t is a standard type. The commit message would be needed, too. -- Sakari Ailus e-mail: sakari.ai...@iki.fi ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: fsl-mc: move bus driver out of staging
Hi Andrew, On 11/28/2017 06:04 PM, Andrew Lunn wrote: > On Tue, Nov 28, 2017 at 05:27:57PM +0200, laurentiu.tu...@nxp.com wrote: >> diff --git a/drivers/staging/fsl-mc/bus/dpmcp.h b/drivers/bus/fsl-mc/dpmcp.h >> similarity index 100% >> rename from drivers/staging/fsl-mc/bus/dpmcp.h >> rename to drivers/bus/fsl-mc/dpmcp.h >> diff --git a/drivers/staging/fsl-mc/bus/dpmng-cmd.h >> b/drivers/bus/fsl-mc/dpmng-cmd.h >> similarity index 100% >> rename from drivers/staging/fsl-mc/bus/dpmng-cmd.h >> rename to drivers/bus/fsl-mc/dpmng-cmd.h >> diff --git a/drivers/staging/fsl-mc/bus/dprc-cmd.h >> b/drivers/bus/fsl-mc/dprc-cmd.h >> similarity index 100% >> rename from drivers/staging/fsl-mc/bus/dprc-cmd.h >> rename to drivers/bus/fsl-mc/dprc-cmd.h >> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c >> b/drivers/bus/fsl-mc/dprc-driver.c >> similarity index 99% >> rename from drivers/staging/fsl-mc/bus/dprc-driver.c >> rename to drivers/bus/fsl-mc/dprc-driver.c > > Hi Laurentiu > > It is hard to review code when we don't see it. I'll reformat the patch with rename detection disabled, but please note that it will be pretty big (~370KiB). --- Best Regards, Laurentiu ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] media: staging: atomisp: fix for sparse "using plain integer as NULL pointer" warnings.
On 2017-11-29, at 03:04:53 +0300, Dan Carpenter wrote:
> On Tue, Nov 28, 2017 at 11:33:37PM +, Jeremy Sowden wrote:
> > On 2017-11-28, at 17:15:24 +0300, Dan Carpenter wrote:
> > > On Mon, Nov 27, 2017 at 12:44:48PM +, Jeremy Sowden wrote:
> > > > The "address" member of struct ia_css_host_data is a
> > > > pointer-to-char, so define default as NULL.
> > > >
> > > > ---
> > > > a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
> > > > +++
> > > > b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isp_param/interface/ia_css_isp_param_types.h
> > > > @@ -95,7 +95,7 @@ union ia_css_all_memory_offsets {
> > > > };
> > > >
> > > > #define IA_CSS_DEFAULT_ISP_MEM_PARAMS \
> > > > - { { { { 0, 0 } } } }
> > > > + { { { { NULL, 0 } } } }
> > >
> > > This define is way ugly and instead of making superficial changes, you
> > > should try to eliminate it.
> > >
> > > People look at warnings as a bad thing but they are actually a
> > > valuable resource which call attention to bad code. By making this
> > > change you're kind of wasting the warning. The bad code is still
> > > there, it's just swept under the rug but like a dead mouse carcass
> > > it's still stinking up the living room. We should leave the warning
> > > there until it irritates someone enough to fix it properly.
> >
> > Tracking down the offending initializer was definitely a pain.
> >
> > Compound literals with designated initializers would make this macro
> > (and a number of others) easier to understand and more type-safe:
> >
> >#define IA_CSS_DEFAULT_ISP_MEM_PARAMS \
> > - { { { { 0, 0 } } } }
> > + (struct ia_css_isp_param_host_segments) { \
> > + .params = { { \
> > + (struct ia_css_host_data) { \
> > + .address = NULL, \
> > + .size = 0 \
> > + } \
> > + } } \
> > + }
>
> Using designated initializers is good, yes. Can't we just use an
> empty initializer since this is all zeroed memory anyway?
>
> (struct ia_css_isp_param_host_segments) { }
>
> I haven't tried it.
There are 35 defaults defined by macros like this, most of them much
more complicated that IA_CSS_DEFAULT_ISP_MEM_PARAMS, and a few members
are initialized to non-zero values. My plan, therefore, is to convert
everything to use designated initializers, and then start removing the
zeroes afterwards.
> >
> > Unfortunately this default value is one end of a chain of default values
>
> Yeah. A really long chain...
>
> > used to initialize members of default values of enclosing structs where
> > the outermost values are used to initialize some static variables:
> >
> > static enum ia_css_err
> > init_pipe_defaults(enum ia_css_pipe_mode mode,
> > struct ia_css_pipe *pipe,
> > bool copy_pipe)
> > {
> > static struct ia_css_pipe default_pipe = IA_CSS_DEFAULT_PIPE;
> > static struct ia_css_preview_settings prev =
> > IA_CSS_DEFAULT_PREVIEW_SETTINGS;
> > static struct ia_css_capture_settings capt =
> > IA_CSS_DEFAULT_CAPTURE_SETTINGS;
> > static struct ia_css_video_settings video =
> > IA_CSS_DEFAULT_VIDEO_SETTINGS;
> > static struct ia_css_yuvpp_settings yuvpp =
> > IA_CSS_DEFAULT_YUVPP_SETTINGS;
> >
> > if (pipe == NULL) {
> > IA_CSS_ERROR("NULL pipe parameter");
> > return IA_CSS_ERR_INVALID_ARGUMENTS;
> > }
> >
> > /* Initialize pipe to pre-defined defaults */
> > *pipe = default_pipe;
> >
> > [...]
> >
> > I'm not convinced, however, that those variables actually achieve very
> > much. If I change the code to assign the defaults directly, the problem
> > goes away:
> >
> > [...]
> >
> > Does this seem reasonable or am I barking up the wrong tree?
>
> Yes. Chopping the chain down and deleting as much of this code as
> possible seems a good thing.
I'll get chopping.
J.
signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V3 22/29] [media] atomisp: deprecate pci_get_bus_and_slot()
Hi Sinan,
On Mon, Nov 27, 2017 at 11:57:59AM -0500, Sinan Kaya wrote:
> diff --git
> a/drivers/staging/media/atomisp/platform/intel-mid/intel_mid_pcihelpers.c
> b/drivers/staging/media/atomisp/platform/intel-mid/intel_mid_pcihelpers.c
> index 4631b1d..51dcef57 100644
> --- a/drivers/staging/media/atomisp/platform/intel-mid/intel_mid_pcihelpers.c
> +++ b/drivers/staging/media/atomisp/platform/intel-mid/intel_mid_pcihelpers.c
> @@ -39,7 +39,7 @@ static inline int platform_is(u8 model)
>
> static int intel_mid_msgbus_init(void)
> {
> - pci_root = pci_get_bus_and_slot(0, PCI_DEVFN(0, 0));
> + pci_root = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
> if (!pci_root) {
> pr_err("%s: Error: msgbus PCI handle NULL\n", __func__);
> return -ENODEV;
This file has been removed, I'm applying the rest of the patch.
Please use the media tree as the base in the future. Thanks.
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
