Re: [PATCH v4 18/36] media: Add i.MX media core driver
On Thu, 2017-02-16 at 17:33 -0800, Steve Longerbeam wrote: > > On 02/16/2017 05:02 AM, Philipp Zabel wrote: > > On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote: > > >> + > >> +- Clean up and move the ov5642 subdev driver to drivers/media/i2c, and > >> + create the binding docs for it. > > > > This is done already, right? > > > I cleaned up ov5640 and moved it to drivers/media/i2c with binding docs, > but not the ov5642 yet. Ok, thanks. > >> +- The Frame Interval Monitor could be exported to v4l2-core for > >> + general use. > >> + > >> +- The subdev that is the original source of video data (referred to as > >> + the "sensor" in the code), is called from various subdevs in the > >> + pipeline in order to set/query the video standard ({g|s|enum}_std) > >> + and to get/set the original frame interval from the capture interface > >> + ([gs]_parm). Instead, the entities that need this info should call its > >> + direct neighbor, and the neighbor should propagate the call to its > >> + neighbor in turn if necessary. > > > > Especially the [gs]_parm fix is necessary to present userspace with the > > correct frame interval in case of frame skipping in the CSI. > > > Right, understood. I've added this to list of fixes for version 5. > > What a pain though! It means propagating every call to g_frame_interval > upstream until a subdev "that cares" returns ret == 0 or > ret != -ENOIOCTLCMD. And that goes for any other chained subdev call > as well. Not at all. Since the frame interval is a property of the pad, that had to be propagated downstream by media-ctl along with media bus format, frame size, and colorimetry earlier. regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: BCM2835-Audio: bcm2835-pcm: single line block statement braces fix
A single line statement under an if clause doesn't require braces {} Signed-off-by: Daniel Perez de Andres --- drivers/staging/bcm2835-audio/bcm2835-pcm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/bcm2835-audio/bcm2835-pcm.c b/drivers/staging/bcm2835-audio/bcm2835-pcm.c index 014bf7a..a2a8694 100644 --- a/drivers/staging/bcm2835-audio/bcm2835-pcm.c +++ b/drivers/staging/bcm2835-audio/bcm2835-pcm.c @@ -317,9 +317,9 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream *substream) err = bcm2835_audio_set_params(alsa_stream, channels, alsa_stream->params_rate, alsa_stream->pcm_format_width); - if (err < 0) { + if (err < 0) audio_error(" error setting hw params\n"); - } + bcm2835_audio_setup(alsa_stream); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/6] staging: ks7010: fixed warning of avoiding line over 80 characters
This is patch 01 to ks_wlan.h file in order to fix warning of line over 80 characters, as issued by checkpatch.pl Signed-off-by: Chetan Sethi --- v2: - split multiple changes across different patches v3: - mentioned patch revision in subject drivers/staging/ks7010/ks_wlan.h | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h index 9ab80e1..668202d 100644 --- a/drivers/staging/ks7010/ks_wlan.h +++ b/drivers/staging/ks7010/ks_wlan.h @@ -18,10 +18,10 @@ #include #include -#include /* spinlock_t */ -#include/* wait_queue_head_t */ -#include/* pid_t */ -#include/* struct net_device_stats, struct sk_buff */ +#include /* spinlock_t */ +#include/* wait_queue_head_t */ +#include/* pid_t */ +#include/* struct net_device_stats, struct sk_buff */ #include #include #include /* struct atomic_t */ @@ -36,7 +36,8 @@ #ifdef KS_WLAN_DEBUG #define DPRINTK(n, fmt, args...) \ - if (KS_WLAN_DEBUG > (n)) printk(KERN_NOTICE "%s: "fmt, __FUNCTION__, ## args) + if (KS_WLAN_DEBUG > (n)) \ + printk(KERN_NOTICE "%s: "fmt, __FUNCTION__, ## args) #else #define DPRINTK(n, fmt, args...) #endif -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/6] staging: ks7010: fix coding style issue of enclosing complex macro value in parentheses
This is 02nd patch to file ks_wlan.h file fixing error of enclosing complex macro value in parentheses Signed-off-by: Chetan Sethi --- v2: - split multiple changes across different patches v3: - mentioned patch revision in subject - incorporated review comment of correct indentation for do statement drivers/staging/ks7010/ks_wlan.h | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h index 668202d..33d6b28 100644 --- a/drivers/staging/ks7010/ks_wlan.h +++ b/drivers/staging/ks7010/ks_wlan.h @@ -36,8 +36,10 @@ #ifdef KS_WLAN_DEBUG #define DPRINTK(n, fmt, args...) \ - if (KS_WLAN_DEBUG > (n)) \ - printk(KERN_NOTICE "%s: "fmt, __FUNCTION__, ## args) +do { \ + if (KS_WLAN_DEBUG > (n)) \ + printk(KERN_NOTICE "%s: "fmt, __FUNCTION__, ## args); \ + } while (0) #else #define DPRINTK(n, fmt, args...) #endif -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/6] staging: ks7010: fix coding style issue of using tabs instead of spaces
This is 03rd patch to file ks_wlan.h fixing coding style issue of using tabs instead of spaces at start of line, error as issued by checkpatch.pl Signed-off-by: Chetan Sethi --- v2: - split multiple changes across different patches v3: - mentioned patch revision in subject drivers/staging/ks7010/ks_wlan.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h index 33d6b28..94b648b 100644 --- a/drivers/staging/ks7010/ks_wlan.h +++ b/drivers/staging/ks7010/ks_wlan.h @@ -36,7 +36,7 @@ #ifdef KS_WLAN_DEBUG #define DPRINTK(n, fmt, args...) \ -do { \ + do { \ if (KS_WLAN_DEBUG > (n)) \ printk(KERN_NOTICE "%s: "fmt, __FUNCTION__, ## args); \ } while (0) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 4/6] drivers: staging: fix coding style issue of using pr_notice instead of printk
This is 04th patch to ks_wlan.h fixing coding style issue of using pr_notice instead of printk, warning as issued by checkpatch.pl Signed-off-by: Chetan Sethi --- v2: - split multiple changes across different patches v3: - mentioned patch revision in subject drivers/staging/ks7010/ks_wlan.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h index 94b648b..78beca7 100644 --- a/drivers/staging/ks7010/ks_wlan.h +++ b/drivers/staging/ks7010/ks_wlan.h @@ -38,7 +38,7 @@ #define DPRINTK(n, fmt, args...) \ do { \ if (KS_WLAN_DEBUG > (n)) \ - printk(KERN_NOTICE "%s: "fmt, __FUNCTION__, ## args); \ + pr_notice("%s: "fmt, __FUNCTION__, ## args); \ } while (0) #else #define DPRINTK(n, fmt, args...) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 5/6] drivers: staging: fix coding style issue of using __func__ instead of __FUNCTION__
This is 05th patch to file ks_wlan.h which fixes coding style issue of using __func__ instead of gcc specific __FUNCTION__, warning as issued by checkpatch.pl Signed-off-by: Chetan Sethi --- v2: - split multiple changes across different patches v3: - mentioned patch revision in subject drivers/staging/ks7010/ks_wlan.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h index 78beca7..79737bf 100644 --- a/drivers/staging/ks7010/ks_wlan.h +++ b/drivers/staging/ks7010/ks_wlan.h @@ -38,7 +38,7 @@ #define DPRINTK(n, fmt, args...) \ do { \ if (KS_WLAN_DEBUG > (n)) \ - pr_notice("%s: "fmt, __FUNCTION__, ## args); \ + pr_notice("%s: "fmt, __func__, ## args); \ } while (0) #else #define DPRINTK(n, fmt, args...) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 6/6] drivers: staging: fix coding style issue of aligning comments properly
This is 06th patch to file ks_wlan.h in order to fix coding style issue of having block comments using a trailing */ on a separate line, warning as issued by checkpatch.pl Signed-off-by: Chetan Sethi --- v2: - split multiple changes across different patches v3: - mentioned patch revision in subject drivers/staging/ks7010/ks_wlan.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h index 79737bf..1a63704 100644 --- a/drivers/staging/ks7010/ks_wlan.h +++ b/drivers/staging/ks7010/ks_wlan.h @@ -359,7 +359,8 @@ struct wpa_key_t { u8 rx_seq[IW_ENCODE_SEQ_MAX_SIZE]; /* LSB first */ struct sockaddr addr; /* ff:ff:ff:ff:ff:ff for broadcast/multicast * (group) keys or unicast address for -* individual keys */ +* individual keys +*/ u16 alg; u16 key_len;/* WEP: 5 or 13, TKIP: 32, CCMP: 16 */ u8 key_val[IW_ENCODING_TOKEN_MAX]; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/3] x86/vdso: Add Hyper-V TSC page clocksource support
Thomas Gleixner writes: > On Wed, 15 Feb 2017, Vitaly Kuznetsov wrote: >> Actually, we already have an implementation of TSC page update in KVM >> (see arch/x86/kvm/hyperv.c, kvm_hv_setup_tsc_page()) and the update does >> the following: >> >> 0) stash seq into seq_prev >> 1) seq = 0 making all reads from the page invalid >> 2) smp_wmb() >> 3) update tsc_scale, tsc_offset >> 4) smp_wmb() >> 5) set seq = seq_prev + 1 > > I hope they handle the case where seq_prev overflows and becomes 0 :) > >> As far as I understand this helps with situations you described above as >> guest will notice either invalid value of 0 or seq change. In case the >> implementation in real Hyper-V is the same we're safe with compile >> barriers only. > > On x86 that's correct. smp_rmb() resolves to barrier(), but you certainly > need the smp_wmb() on the writer side. > > Now looking at the above your reader side code is bogus: > > + while (1) { > + sequence = tsc_pg->tsc_sequence; > + if (!sequence) > + break; > > Why would you break out of the loop when seq is 0? The 0 is just telling > you that there is an update in progress. Not only. As far as I understand (and I *think* K. Y. pointed this out) when VM is migrating to another host TSC page clocksource is disabled for extended period of time so we're better off reading from MSR than looping here. With regards to VDSO this means reverting to doing normal syscall. > > The Linux seqcount writer side is: > > seq++; > smp_wmb(); > > update... > > smp_wmb(); > seq++; > > and it's defined that an odd sequence count, i.e. bit 0 set means update in > progress. Which is nice, because you don't have to treat 0 special on the > writer side and you don't need extra storage to stash seq away :) > > So the reader side does: > > do { > while (1) { >s = READ_ONCE(seq); >if (!(s & 0x01)) > break; >cpu_relax(); > } >smp_rmb(); > >read data ... > >smp_rmb(); >} while (s != seq) > > So for that hyperv thing you want: > > do { > while (1) { >s = READ_ONCE(seq); >if (s) > break; >cpu_relax(); > } >smp_rmb(); > >read data ... > >smp_rmb(); >} while (s != seq) > > Thanks, > > tglx -- Vitaly ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/3] x86/vdso: Add Hyper-V TSC page clocksource support
On Fri, 17 Feb 2017, Vitaly Kuznetsov wrote: > Thomas Gleixner writes: > > On Wed, 15 Feb 2017, Vitaly Kuznetsov wrote: > >> Actually, we already have an implementation of TSC page update in KVM > >> (see arch/x86/kvm/hyperv.c, kvm_hv_setup_tsc_page()) and the update does > >> the following: > >> > >> 0) stash seq into seq_prev > >> 1) seq = 0 making all reads from the page invalid > >> 2) smp_wmb() > >> 3) update tsc_scale, tsc_offset > >> 4) smp_wmb() > >> 5) set seq = seq_prev + 1 > > > > I hope they handle the case where seq_prev overflows and becomes 0 :) > > > >> As far as I understand this helps with situations you described above as > >> guest will notice either invalid value of 0 or seq change. In case the > >> implementation in real Hyper-V is the same we're safe with compile > >> barriers only. > > > > On x86 that's correct. smp_rmb() resolves to barrier(), but you certainly > > need the smp_wmb() on the writer side. > > > > Now looking at the above your reader side code is bogus: > > > > + while (1) { > > + sequence = tsc_pg->tsc_sequence; > > + if (!sequence) > > + break; > > > > Why would you break out of the loop when seq is 0? The 0 is just telling > > you that there is an update in progress. > > Not only. As far as I understand (and I *think* K. Y. pointed this out) > when VM is migrating to another host TSC page clocksource is disabled for > extended period of time so we're better off reading from MSR than > looping here. With regards to VDSO this means reverting to doing normal > syscall. If you migrate to another host and the VM is using the TSC page, then the TSC page on the new host _must_ be available and accessible _before_ the VM resumes there. So that extended period of time does not make any sense at all. Voodoo programming is the only explanation which come to my mind. Thanks, tglx ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 00/36] i.MX Media Driver
On Thu, 2017-02-16 at 22:57 +, Russell King - ARM Linux wrote: > On Thu, Feb 16, 2017 at 02:27:41PM -0800, Steve Longerbeam wrote: > > > > > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote: > > >On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote: > > >>In version 4: > > > > > >With this version, I get: > > > > > >[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x > > >[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110 > > > > > > > Right, in the imx219, on exit from s_power(), the clock and data lanes > > must be placed in the LP-11 state. This has been done in the ov5640 and > > tc358743 subdevs. > > The only way to do that is to enable streaming from the sensor, wait > an initialisation time, and then disable streaming, and wait for the > current line to finish. There is _no_ other way to get the sensor to > place its clock and data lines into LP-11 state. I thought going through LP-11 is part of the D-PHY transmitter initialization, during the LP->HS wakeup sequence. But then I have no access to MIPI specs. It is unfortunate that the i.MX6 MIPI CSI-2 core needs software assistance here, but could it be possible to trigger that sequence in the sensor and then without waiting switching to polling for LP-11 state in the i.MX6 MIPI CSI-2 receiver? > For that to happen, we need to program the sensor a bit more than we > currently do at power on (to a minimal resolution, and setting up the > PLLs), and introduce another 4ms on top of the 8ms or so that the > runtime resume function already takes. > > Looking at the SMIA driver, things are worse, and I suspect that it also > will not work with the current setup - the SMIA spec shows that the CSI > clock and data lines are tristated while the sensor is not streaming, > which means they aren't held at a guaranteed LP-11 state, even if that > driver momentarily enabled streaming. Hence, Freescale's (or is it > Synopsis') requirement may actually be difficult to satisfy. > > However, I regard runtime PM broken with the current imx-capture setup. > At the moment, power is controlled at the sensor by whether the media > links are enabled. So, if you have an enabled link coming off the > sensor, the sensor will be powered up, whether you're using it or not. > > Given that the number of applications out there that know about the > media subdevs is really quite small, this combination makes having > runtime PM in sensor devices completely pointless - they can't sleep > as long as they have an enabled link, which could be persistent over > many days or weeks. regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev driver
On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote: > Adds MIPI CSI-2 Receiver subdev driver. This subdev is required > for sensors with a MIPI CSI2 interface. > > Signed-off-by: Steve Longerbeam > --- > drivers/staging/media/imx/Makefile | 1 + > drivers/staging/media/imx/imx6-mipi-csi2.c | 573 > + > 2 files changed, 574 insertions(+) > create mode 100644 drivers/staging/media/imx/imx6-mipi-csi2.c > > diff --git a/drivers/staging/media/imx/Makefile > b/drivers/staging/media/imx/Makefile > index 878a126..3569625 100644 > --- a/drivers/staging/media/imx/Makefile > +++ b/drivers/staging/media/imx/Makefile > @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-vdic.o > obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-ic.o > > obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o > +obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o > diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c > b/drivers/staging/media/imx/imx6-mipi-csi2.c > new file mode 100644 > index 000..23dca80 > --- /dev/null > +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c > @@ -0,0 +1,573 @@ > +/* > + * MIPI CSI-2 Receiver Subdev for Freescale i.MX6 SOC. > + * > + * Copyright (c) 2012-2017 Mentor Graphics Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "imx-media.h" > + > +/* > + * there must be 5 pads: 1 input pad from sensor, and > + * the 4 virtual channel output pads > + */ > +#define CSI2_SINK_PAD 0 > +#define CSI2_NUM_SINK_PADS 1 > +#define CSI2_NUM_SRC_PADS 4 > +#define CSI2_NUM_PADS 5 > + > +struct csi2_dev { > + struct device *dev; > + struct v4l2_subdev sd; > + struct media_pad pad[CSI2_NUM_PADS]; > + struct v4l2_mbus_framefmt format_mbus; > + struct clk *dphy_clk; > + struct clk *cfg_clk; > + struct clk *pix_clk; /* what is this? */ > + void __iomem *base; > + struct v4l2_of_bus_mipi_csi2 bus; > + boolon; > + boolstream_on; > + boolsrc_linked; > + boolsink_linked[CSI2_NUM_SRC_PADS]; > +}; > + > +#define DEVICE_NAME "imx6-mipi-csi2" > + > +/* Register offsets */ > +#define CSI2_VERSION0x000 > +#define CSI2_N_LANES0x004 > +#define CSI2_PHY_SHUTDOWNZ 0x008 > +#define CSI2_DPHY_RSTZ 0x00c > +#define CSI2_RESETN 0x010 > +#define CSI2_PHY_STATE 0x014 > +#define PHY_STOPSTATEDATA_BIT 4 > +#define PHY_STOPSTATEDATA(n)BIT(PHY_STOPSTATEDATA_BIT + (n)) > +#define PHY_RXCLKACTIVEHS BIT(8) > +#define PHY_RXULPSCLKNOTBIT(9) > +#define PHY_STOPSTATECLKBIT(10) > +#define CSI2_DATA_IDS_1 0x018 > +#define CSI2_DATA_IDS_2 0x01c > +#define CSI2_ERR1 0x020 > +#define CSI2_ERR2 0x024 > +#define CSI2_MSK1 0x028 > +#define CSI2_MSK2 0x02c > +#define CSI2_PHY_TST_CTRL0 0x030 > +#define PHY_TESTCLR BIT(0) > +#define PHY_TESTCLK BIT(1) > +#define CSI2_PHY_TST_CTRL1 0x034 > +#define PHY_TESTEN BIT(16) > +#define CSI2_SFT_RESET 0xf00 > + > +static inline struct csi2_dev *sd_to_dev(struct v4l2_subdev *sdev) > +{ > + return container_of(sdev, struct csi2_dev, sd); > +} > + > +static void csi2_enable(struct csi2_dev *csi2, bool enable) > +{ > + if (enable) { > + writel(0x1, csi2->base + CSI2_PHY_SHUTDOWNZ); > + writel(0x1, csi2->base + CSI2_DPHY_RSTZ); > + writel(0x1, csi2->base + CSI2_RESETN); > + } else { > + writel(0x0, csi2->base + CSI2_PHY_SHUTDOWNZ); > + writel(0x0, csi2->base + CSI2_DPHY_RSTZ); > + writel(0x0, csi2->base + CSI2_RESETN); > + } > +} > + > +static void csi2_set_lanes(struct csi2_dev *csi2) > +{ > + int lanes = csi2->bus.num_data_lanes; > + > + writel(lanes - 1, csi2->base + CSI2_N_LANES); > +} > + > +static void dw_mipi_csi2_phy_write(struct csi2_dev *csi2, > +u32 test_code, u32 test_data) > +{ > + /* Clear PHY test interface */ > + writel(PHY_TESTCLR, csi2->base + CSI2_PHY_TST_CTRL0); > + writel(0x0, csi2->base + CSI2_PHY_TST_CTRL1); > + writel(0x0, csi2->base + CSI2_PHY_TST_CTRL0); > + > + /* Raise test interface strobe signal */ > + writel(PHY_TESTCLK, csi2->base + CSI2_PHY_TST_CTRL0); > + > + /* Configure address write on falling edge and lower strobe signal */ > + writel(PHY_TESTEN | test_code, csi2->base + CSI2_PHY_TST_CTRL1); > + writel(0x
Re: [PATCH v4 00/36] i.MX Media Driver
On Fri, Feb 17, 2017 at 11:39:11AM +0100, Philipp Zabel wrote: > On Thu, 2017-02-16 at 22:57 +, Russell King - ARM Linux wrote: > > On Thu, Feb 16, 2017 at 02:27:41PM -0800, Steve Longerbeam wrote: > > > > > > > > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote: > > > >On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote: > > > >>In version 4: > > > > > > > >With this version, I get: > > > > > > > >[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x > > > >[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110 > > > > > > > > > > Right, in the imx219, on exit from s_power(), the clock and data lanes > > > must be placed in the LP-11 state. This has been done in the ov5640 and > > > tc358743 subdevs. > > > > The only way to do that is to enable streaming from the sensor, wait > > an initialisation time, and then disable streaming, and wait for the > > current line to finish. There is _no_ other way to get the sensor to > > place its clock and data lines into LP-11 state. > > I thought going through LP-11 is part of the D-PHY transmitter > initialization, during the LP->HS wakeup sequence. But then I have no > access to MIPI specs. The D-PHY transmitter initialisation *only* happens as part of the wake-up from standby to streaming mode. That is because Sony expect that you program the sensor, and then when you switch it to streaming mode, it computes the D-PHY parameters from the PLL, input clock rate (you have to tell it the clock rate in 1/256 MHz units), number of lanes, and other parameters. It is possible to program the D-PHY parameters manually, but that doesn't change the above sequence in any way (it just avoids the chip computing the values, it doesn't result in any change of behaviour on the bus.) The IMX219 specifications are clear: the clock and data lines are held low (LP-00 state) after releasing the hardware enable signal. There's a period of chip initialisation, and then you can access the I2C bus and configure it. There's a further period of initialisation where charge pumps are getting to their operating state. Then, you set the streaming bit, and a load more initialisation happens before the CSI bus enters LP-11 state and the first frame pops out. When entering standby, the last frame is completed, and then the CSI bus enters LP-11 state. SMIA are slightly different - mostly following what I've said above, but the clock and data lines are tristated after releasing the xshutdown signal, and they remain tristated until the clock line starts toggling before the first frame appears. There appears to be no point that the clock line enters LP-11 state before it starts toggling. When entering standby, the last frame is completed, and the CSI bus enters tristate mode (so floating.) There is no way to get these sensors into LP-11 state. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev driver
On Fri, Feb 17, 2017 at 11:47:59AM +0100, Philipp Zabel wrote: > On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote: > > +static void csi2_dphy_init(struct csi2_dev *csi2) > > +{ > > + /* > > +* FIXME: 0x14 is derived from a fixed D-PHY reference > > +* clock from the HSI_TX PLL, and a fixed target lane max > > +* bandwidth of 300 Mbps. This value should be derived > > If the table in https://community.nxp.com/docs/DOC-94312 is correct, > this should be 850 Mbps. Where does this 300 Mbps value come from? I thought you had some code to compute the correct value, although I guess we've lost the ability to know how fast the sensor is going to drive the link. Note that the IMX219 currently drives the data lanes at 912Mbps almost exclusively, as I've yet to finish working out how to derive the PLL parameters. (I have something that works, but it currently takes on the order of 100k iterations to derive the parameters. gcd() doesn't help you in this instance.) -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/11] staging: comedi: jr3_pci: some bug fixes and cleanups
Some bug-fixes and clean-ups for the jr3_pci driver. I have more clean-up patches for this driver waiting in the wings, but the series was getting a bit long. Patches 01 to 03 fix various bugs. Patches 04 and 05 fix some checkpatch warnings. Patch 08 actually introduces a couple more checkpatch warnings, both related to the use of non-const 'struct comedi_lrange', which is deliberate in this driver (see the notes after the commit message for details). 01) staging: comedi: jr3_pci: fix possible null pointer dereference 02) staging: comedi: jr3_pci: cope with jiffies wraparound 03) staging: comedi: jr3_pci: Reset all DSPs 04) staging: comedi: jr3_pci: struct comedi_lrange should normally be const 05) staging: comedi: jr3_pci: re-work firmware copyright display 06) staging: comedi: jr3_pci: remove unneeded 'spriv' checks 07) staging: comedi: jr3_pci: remove next_time_max member 08) staging: comedi: jr3_pci: separate out poll state enum 09) staging: comedi: jr3_pci: re-work struct jr3_pci_subdev_private range 10) staging: comedi: jr3_pci: pass transform by reference 11) staging: comedi: jr3_pci: replace devpriv->iobase with dev->mmio drivers/staging/comedi/drivers/jr3_pci.c | 169 --- 1 file changed, 86 insertions(+), 83 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 06/11] staging: comedi: jr3_pci: remove unneeded 'spriv' checks
If `jr3_pci_auto_attach()` returns with no error, we can now be sure that the COMEDI subdevice private data structures have been allocated. Remove the tests for a valid pointer to the private data structure in `jr3_pci_ai_insn_read()`, `jr3_pci_open()`, and `jr3_pci_poll_subdevice()`, since they will not be called if the pointer is invalid. Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/jr3_pci.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/staging/comedi/drivers/jr3_pci.c b/drivers/staging/comedi/drivers/jr3_pci.c index 69ed84a385e3..500ec50d26bf 100644 --- a/drivers/staging/comedi/drivers/jr3_pci.c +++ b/drivers/staging/comedi/drivers/jr3_pci.c @@ -279,9 +279,6 @@ static int jr3_pci_ai_insn_read(struct comedi_device *dev, u16 errors; int i; - if (!spriv) - return -EINVAL; - errors = get_u16(&spriv->channel->errors); if (spriv->state != state_jr3_done || (errors & (watch_dog | watch_dog2 | sensor_change))) { @@ -309,9 +306,8 @@ static int jr3_pci_open(struct comedi_device *dev) for (i = 0; i < dev->n_subdevices; i++) { s = &dev->subdevices[i]; spriv = s->private; - if (spriv) - dev_dbg(dev->class_dev, "serial: %p %d (%d)\n", - spriv, spriv->serial_no, s->index); + dev_dbg(dev->class_dev, "serial: %p %d (%d)\n", + spriv, spriv->serial_no, s->index); } return 0; } @@ -459,9 +455,6 @@ jr3_pci_poll_subdevice(struct comedi_subdevice *s) int errors; int i; - if (!spriv) - return result; - channel = spriv->channel; errors = get_u16(&channel->errors); -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 10/11] staging: comedi: jr3_pci: pass transform by reference
Local function `set_transforms` has a parameter of type `struct jr3_pci_transform`. This has a size 32 bytes, which is quite large for passing around in a function call. Change it to use type `const struct jr3_pci_transform *`. (In practice, it is probably inlined by the compiler anyway, but doing this seems to save a few bytes.) Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/jr3_pci.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/comedi/drivers/jr3_pci.c b/drivers/staging/comedi/drivers/jr3_pci.c index 59030f3b382b..997f97089df8 100644 --- a/drivers/staging/comedi/drivers/jr3_pci.c +++ b/drivers/staging/comedi/drivers/jr3_pci.c @@ -141,19 +141,19 @@ static int is_complete(struct jr3_channel __iomem *channel) } static void set_transforms(struct jr3_channel __iomem *channel, - struct jr3_pci_transform transf, short num) + const struct jr3_pci_transform *transf, short num) { int i; num &= 0x000f; /* Make sure that 0 <= num <= 15 */ for (i = 0; i < 8; i++) { set_u16(&channel->transforms[num].link[i].link_type, - transf.link[i].link_type); + transf->link[i].link_type); udelay(1); set_s16(&channel->transforms[num].link[i].link_amount, - transf.link[i].link_amount); + transf->link[i].link_amount); udelay(1); - if (transf.link[i].link_type == end_x_form) + if (transf->link[i].link_type == end_x_form) break; } } @@ -505,7 +505,7 @@ jr3_pci_poll_subdevice(struct comedi_subdevice *s) transf.link[i].link_amount = 0; } - set_transforms(channel, transf, 0); + set_transforms(channel, &transf, 0); use_transform(channel, 0); spriv->state = state_jr3_init_transform_complete; /* Allow 20 ms for completion */ -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 11/11] staging: comedi: jr3_pci: replace devpriv->iobase with dev->mmio
The "jr3_pci" driver currently uses the `iobase` member of its private device data `struct jr3_pci_dev_private` to store a pointer to its ioremapped register region. Use the `mmio` member of the `struct comedi_device` to store this instead, and remove the `iobase` member. The `iobase` member was of type `struct jr3_t __iomem *`, with the board's complicated register layout described by `struct jr3_t`. The `mmio` member is a generic `void __iomem *`, so its value needs converting to a `struct jr3_t __iomem *` for our purposes. Change the clean-up in `jr3_pci_detach()` to call `comedi_pci_detach()` instead of `comedi_pci_disable()`, as that will iounmap `dev->mmio` for us. Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/jr3_pci.c | 32 +++- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/drivers/staging/comedi/drivers/jr3_pci.c b/drivers/staging/comedi/drivers/jr3_pci.c index 997f97089df8..297ec2f7ff34 100644 --- a/drivers/staging/comedi/drivers/jr3_pci.c +++ b/drivers/staging/comedi/drivers/jr3_pci.c @@ -95,7 +95,6 @@ struct jr3_pci_poll_delay { }; struct jr3_pci_dev_private { - struct jr3_t __iomem *iobase; struct timer_list timer; }; @@ -375,8 +374,7 @@ static int jr3_check_firmware(struct comedi_device *dev, static void jr3_write_firmware(struct comedi_device *dev, int subdev, const u8 *data, size_t size) { - struct jr3_pci_dev_private *devpriv = dev->private; - struct jr3_t __iomem *iobase = devpriv->iobase; + struct jr3_t __iomem *iobase = dev->mmio; u32 __iomem *lo; u32 __iomem *hi; int more = 1; @@ -634,7 +632,7 @@ static void jr3_pci_poll_dev(unsigned long data) static struct jr3_pci_subdev_private * jr3_pci_alloc_spriv(struct comedi_device *dev, struct comedi_subdevice *s) { - struct jr3_pci_dev_private *devpriv = dev->private; + struct jr3_t __iomem *iobase = dev->mmio; struct jr3_pci_subdev_private *spriv; int j; int k; @@ -643,7 +641,7 @@ jr3_pci_alloc_spriv(struct comedi_device *dev, struct comedi_subdevice *s) if (!spriv) return NULL; - spriv->channel = &devpriv->iobase->channel[s->index].data; + spriv->channel = &iobase->channel[s->index].data; for (j = 0; j < 8; j++) { spriv->range[j].l.length = 1; @@ -665,17 +663,17 @@ jr3_pci_alloc_spriv(struct comedi_device *dev, struct comedi_subdevice *s) spriv->maxdata_list[57] = 0x; dev_dbg(dev->class_dev, "p->channel %p %p (%tx)\n", - spriv->channel, devpriv->iobase, + spriv->channel, iobase, ((char __iomem *)spriv->channel - -(char __iomem *)devpriv->iobase)); +(char __iomem *)iobase)); return spriv; } static void jr3_pci_show_copyright(struct comedi_device *dev) { - struct jr3_pci_dev_private *devpriv = dev->private; - struct jr3_channel __iomem *ch0data = &devpriv->iobase->channel[0].data; + struct jr3_t __iomem *iobase = dev->mmio; + struct jr3_channel __iomem *ch0data = &iobase->channel[0].data; char copy[ARRAY_SIZE(ch0data->copyright) + 1]; int i; @@ -692,6 +690,7 @@ static int jr3_pci_auto_attach(struct comedi_device *dev, static const struct jr3_pci_board *board; struct jr3_pci_dev_private *devpriv; struct jr3_pci_subdev_private *spriv; + struct jr3_t __iomem *iobase; struct comedi_subdevice *s; int ret; int i; @@ -718,10 +717,12 @@ static int jr3_pci_auto_attach(struct comedi_device *dev, if (ret) return ret; - devpriv->iobase = pci_ioremap_bar(pcidev, 0); - if (!devpriv->iobase) + dev->mmio = pci_ioremap_bar(pcidev, 0); + if (!dev->mmio) return -ENOMEM; + iobase = dev->mmio; + ret = comedi_alloc_subdevices(dev, board->n_subdevs); if (ret) return ret; @@ -745,7 +746,7 @@ static int jr3_pci_auto_attach(struct comedi_device *dev, /* Reset DSP card */ for (i = 0; i < dev->n_subdevices; i++) - writel(0, &devpriv->iobase->channel[i].reset); + writel(0, &iobase->channel[i].reset); ret = comedi_load_firmware(dev, &comedi_to_pci_dev(dev)->dev, "comedi/jr3pci.idm", @@ -789,13 +790,10 @@ static void jr3_pci_detach(struct comedi_device *dev) { struct jr3_pci_dev_private *devpriv = dev->private; - if (devpriv) { + if (devpriv) del_timer_sync(&devpriv->timer); - if (devpriv->iobase) - iounmap(devpriv->iobase); - } - comedi_pci_disable(dev); + comedi_pci_detach(dev); } static struct comedi_driver jr3_pci_driver = { -- 2.11.0 ___ devel mailing list de...@l
[PATCH 04/11] staging: comedi: jr3_pci: struct comedi_lrange should normally be const
Fix three checkpatch warnings of the form: WARNING: struct comedi_lrange should normally be const Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/jr3_pci.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/staging/comedi/drivers/jr3_pci.c b/drivers/staging/comedi/drivers/jr3_pci.c index c5bff9a85727..d800a5e113ab 100644 --- a/drivers/staging/comedi/drivers/jr3_pci.c +++ b/drivers/staging/comedi/drivers/jr3_pci.c @@ -656,7 +656,7 @@ jr3_pci_alloc_spriv(struct comedi_device *dev, struct comedi_subdevice *s) for (k = 0; k < 7; k++) { spriv->range_table_list[j + k * 8] = - (struct comedi_lrange *)&spriv->range[j]; + (const struct comedi_lrange *)&spriv->range[j]; spriv->maxdata_list[j + k * 8] = 0x7fff; } } @@ -664,8 +664,10 @@ jr3_pci_alloc_spriv(struct comedi_device *dev, struct comedi_subdevice *s) spriv->range[8].range.min = 0; spriv->range[8].range.max = 65536; - spriv->range_table_list[56] = (struct comedi_lrange *)&spriv->range[8]; - spriv->range_table_list[57] = (struct comedi_lrange *)&spriv->range[8]; + spriv->range_table_list[56] = + (const struct comedi_lrange *)&spriv->range[8]; + spriv->range_table_list[57] = + (const struct comedi_lrange *)&spriv->range[8]; spriv->maxdata_list[56] = 0x; spriv->maxdata_list[57] = 0x; -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/11] staging: comedi: jr3_pci: re-work struct jr3_pci_subdev_private range
The `range` member of `struct jr3_pci_subdev_private` is an array of a tag-less `struct` type whose layout is similar to `struct comedi_lrange`. Both `struct` types end with a member also called `range`. In the case of tag-less `struct` type, it is a single `struct comedi_krange`. In the case of `struct comedi_lrange`, it is a flexible array of `struct comedi_krange`. Elements of the `range` array member in `struct jr3_pci_subdev_private` are pointed to by elements of the `range_table_list` array member, which are of type `const struct comedi_lrange *`. This requires some dodgy type casting. To avoid the dodgy type casting, change the element type of the `range` member of `struct jr3_pci_subdev_private` to be a new type `union jr3_pci_single_range`. This contains a member `l` of type `struct comedi_lrange`, and an array member `_reserved` that is large enough to encompass the `struct comedi_lrange` plus a single `struct comedi_krange`. It is the same size as the previous type. Accesses to `spriv->range[i].length` and `spriv->range[i].range` are replaced with `spriv->range[i].l.length` and `spriv->range[i].l.range[0]` respectively (where `spriv` is a `struct jr3_pci_subdev_private *`, and `i` is an array index). Type-casted pointers to `spriv->range[i]` are replaced with pointers to `spriv->range[i].l`, which do not require the type casts. Since we defined a new type, we can define local variables of the corresponding pointer type to shorten some lines of code. This is made use of in `jr3_pci_alloc_spriv()`. Signed-off-by: Ian Abbott --- Note that this patch produces these two checkpatch warnings: WARNING: struct comedi_lrange should normally be const + struct comedi_lrange l; WARNING: struct comedi_lrange should normally be const + char _reserved[offsetof(struct comedi_lrange, range[1])]; For the first one, this driver determines ranges at run-time, so they are non-const. The second one is pretty irrelevant as it is only being used as part of an offsetof() macro call.. --- drivers/staging/comedi/drivers/jr3_pci.c | 70 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/drivers/staging/comedi/drivers/jr3_pci.c b/drivers/staging/comedi/drivers/jr3_pci.c index 749f5069c42f..59030f3b382b 100644 --- a/drivers/staging/comedi/drivers/jr3_pci.c +++ b/drivers/staging/comedi/drivers/jr3_pci.c @@ -99,6 +99,11 @@ struct jr3_pci_dev_private { struct timer_list timer; }; +union jr3_pci_single_range { + struct comedi_lrange l; + char _reserved[offsetof(struct comedi_lrange, range[1])]; +}; + enum jr3_pci_poll_state { state_jr3_poll, state_jr3_init_wait_for_offset, @@ -114,10 +119,7 @@ struct jr3_pci_subdev_private { enum jr3_pci_poll_state state; int serial_no; int model_no; - struct { - int length; - struct comedi_krange range; - } range[9]; + union jr3_pci_single_range range[9]; const struct comedi_lrange *range_table_list[8 * 7 + 2]; unsigned int maxdata_list[8 * 7 + 2]; u16 errors; @@ -532,27 +534,28 @@ jr3_pci_poll_subdevice(struct comedi_subdevice *s) result = poll_delay_min_max(20, 100); } else { struct force_array __iomem *fs = &channel->full_scale; + union jr3_pci_single_range *r = spriv->range; /* Use ranges in kN or we will overflow around 2000N! */ - spriv->range[0].range.min = -get_s16(&fs->fx) * 1000; - spriv->range[0].range.max = get_s16(&fs->fx) * 1000; - spriv->range[1].range.min = -get_s16(&fs->fy) * 1000; - spriv->range[1].range.max = get_s16(&fs->fy) * 1000; - spriv->range[2].range.min = -get_s16(&fs->fz) * 1000; - spriv->range[2].range.max = get_s16(&fs->fz) * 1000; - spriv->range[3].range.min = -get_s16(&fs->mx) * 100; - spriv->range[3].range.max = get_s16(&fs->mx) * 100; - spriv->range[4].range.min = -get_s16(&fs->my) * 100; - spriv->range[4].range.max = get_s16(&fs->my) * 100; - spriv->range[5].range.min = -get_s16(&fs->mz) * 100; + r[0].l.range[0].min = -get_s16(&fs->fx) * 1000; + r[0].l.range[0].max = get_s16(&fs->fx) * 1000; + r[1].l.range[0].min = -get_s16(&fs->fy) * 1000; + r[1].l.range[0].max = get_s16(&fs->fy) * 1000; + r[2].l.range[0].min = -get_s16(&fs->fz) * 1000; + r[2].l.range[0].max = get_s16(&fs->fz) * 1000; + r[3].l.range[0].min = -get_s16(&fs->mx) * 100; + r[3].l.range[0].max = get_s16(&fs->mx) * 100; + r[4].l.range[0].min = -get_s1
[PATCH 05/11] staging: comedi: jr3_pci: re-work firmware copyright display
If debug messages are enabled, the card initialization done in `jr3_pci_auto_attach()` spits out 24 (0x18) debug messages to show the null-terminated copyright string embedded in the firmware, one character at a time, including the ASCII NUL characters at the end. Factor out the copyright display into a new function `jr3_pci_show_copyright()` and re-work it to copy the whole copyright string into a buffer, so that it can be shown with a single debug message. Incidentally, this also removes a checkpatch warning "Avoid multiple line dereference" in the original code. Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/jr3_pci.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/staging/comedi/drivers/jr3_pci.c b/drivers/staging/comedi/drivers/jr3_pci.c index d800a5e113ab..69ed84a385e3 100644 --- a/drivers/staging/comedi/drivers/jr3_pci.c +++ b/drivers/staging/comedi/drivers/jr3_pci.c @@ -679,6 +679,19 @@ jr3_pci_alloc_spriv(struct comedi_device *dev, struct comedi_subdevice *s) return spriv; } +static void jr3_pci_show_copyright(struct comedi_device *dev) +{ + struct jr3_pci_dev_private *devpriv = dev->private; + struct jr3_channel __iomem *ch0data = &devpriv->iobase->channel[0].data; + char copy[ARRAY_SIZE(ch0data->copyright) + 1]; + int i; + + for (i = 0; i < ARRAY_SIZE(ch0data->copyright); i++) + copy[i] = (char)(get_u16(&ch0data->copyright[i]) >> 8); + copy[i] = '\0'; + dev_dbg(dev->class_dev, "Firmware copyright: %s\n", copy); +} + static int jr3_pci_auto_attach(struct comedi_device *dev, unsigned long context) { @@ -762,11 +775,7 @@ static int jr3_pci_auto_attach(struct comedi_device *dev, * can read firmware version */ msleep_interruptible(25); - for (i = 0; i < 0x18; i++) { - dev_dbg(dev->class_dev, "%c\n", - get_u16(&devpriv->iobase->channel[0]. - data.copyright[i]) >> 8); - } + jr3_pci_show_copyright(dev); /* Start card timer */ for (i = 0; i < dev->n_subdevices; i++) { -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/11] staging: comedi: jr3_pci: fix possible null pointer dereference
For some reason, the driver does not consider allocation of the subdevice private data to be a fatal error when attaching the COMEDI device. It tests the subdevice private data pointer for validity at certain points, but omits some crucial tests. In particular, `jr3_pci_auto_attach()` calls `jr3_pci_alloc_spriv()` to allocate and initialize the subdevice private data, but the same function subsequently dereferences the pointer to access the `next_time_min` and `next_time_max` members without checking it first. The other missing test is in the timer expiry routine `jr3_pci_poll_dev()`, but it will crash before it gets that far. Fix the bug by returning `-ENOMEM` from `jr3_pci_auto_attach()` as soon as one of the calls to `jr3_pci_alloc_spriv()` returns `NULL`. The COMEDI core will subsequently call `jr3_pci_detach()` to clean up. Cc: # 3.15+ Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/jr3_pci.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/staging/comedi/drivers/jr3_pci.c b/drivers/staging/comedi/drivers/jr3_pci.c index 70390de66e0e..25909a936e7c 100644 --- a/drivers/staging/comedi/drivers/jr3_pci.c +++ b/drivers/staging/comedi/drivers/jr3_pci.c @@ -727,11 +727,12 @@ static int jr3_pci_auto_attach(struct comedi_device *dev, s->insn_read= jr3_pci_ai_insn_read; spriv = jr3_pci_alloc_spriv(dev, s); - if (spriv) { - /* Channel specific range and maxdata */ - s->range_table_list = spriv->range_table_list; - s->maxdata_list = spriv->maxdata_list; - } + if (!spriv) + return -ENOMEM; + + /* Channel specific range and maxdata */ + s->range_table_list = spriv->range_table_list; + s->maxdata_list = spriv->maxdata_list; } /* Reset DSP card */ -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/11] staging: comedi: jr3_pci: Reset all DSPs
The various JR3 PCI models have from 1 to 4 DSPs, one per subdevice. Prior to loading the firmware to all the DSPs, the initialization code in `jr3_pci_auto_attach()` resets the first DSP. As far as I can tell, it should reset all of them. Change it to do so. Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/jr3_pci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/jr3_pci.c b/drivers/staging/comedi/drivers/jr3_pci.c index eb0a095efe9c..c5bff9a85727 100644 --- a/drivers/staging/comedi/drivers/jr3_pci.c +++ b/drivers/staging/comedi/drivers/jr3_pci.c @@ -736,7 +736,8 @@ static int jr3_pci_auto_attach(struct comedi_device *dev, } /* Reset DSP card */ - writel(0, &devpriv->iobase->channel[0].reset); + for (i = 0; i < dev->n_subdevices; i++) + writel(0, &devpriv->iobase->channel[i].reset); ret = comedi_load_firmware(dev, &comedi_to_pci_dev(dev)->dev, "comedi/jr3pci.idm", -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/11] staging: comedi: jr3_pci: separate out poll state enum
The type of the `state` member of `struct jr3_pci_subdev_private` is defined in-situ as an enumerated type without a tag. For aesthetic reasons, define the type as `enum jr3_pci_poll_state` outside the containing `struct`. Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/jr3_pci.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/staging/comedi/drivers/jr3_pci.c b/drivers/staging/comedi/drivers/jr3_pci.c index 2876c8bdb582..749f5069c42f 100644 --- a/drivers/staging/comedi/drivers/jr3_pci.c +++ b/drivers/staging/comedi/drivers/jr3_pci.c @@ -99,16 +99,19 @@ struct jr3_pci_dev_private { struct timer_list timer; }; +enum jr3_pci_poll_state { + state_jr3_poll, + state_jr3_init_wait_for_offset, + state_jr3_init_transform_complete, + state_jr3_init_set_full_scale_complete, + state_jr3_init_use_offset_complete, + state_jr3_done +}; + struct jr3_pci_subdev_private { struct jr3_channel __iomem *channel; unsigned long next_time_min; - enum { state_jr3_poll, - state_jr3_init_wait_for_offset, - state_jr3_init_transform_complete, - state_jr3_init_set_full_scale_complete, - state_jr3_init_use_offset_complete, - state_jr3_done - } state; + enum jr3_pci_poll_state state; int serial_no; int model_no; struct { -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/11] staging: comedi: jr3_pci: remove next_time_max member
The `next_time_max` member of `struct jr3_pci_subdev_private` is assigned to, but never read. Remove it. Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/jr3_pci.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/staging/comedi/drivers/jr3_pci.c b/drivers/staging/comedi/drivers/jr3_pci.c index 500ec50d26bf..2876c8bdb582 100644 --- a/drivers/staging/comedi/drivers/jr3_pci.c +++ b/drivers/staging/comedi/drivers/jr3_pci.c @@ -102,7 +102,6 @@ struct jr3_pci_dev_private { struct jr3_pci_subdev_private { struct jr3_channel __iomem *channel; unsigned long next_time_min; - unsigned long next_time_max; enum { state_jr3_poll, state_jr3_init_wait_for_offset, state_jr3_init_transform_complete, @@ -611,8 +610,6 @@ static void jr3_pci_poll_dev(unsigned long data) spriv->next_time_min = jiffies + msecs_to_jiffies(sub_delay.min); - spriv->next_time_max = jiffies + - msecs_to_jiffies(sub_delay.max); if (sub_delay.max && sub_delay.max < delay) /* @@ -776,7 +773,6 @@ static int jr3_pci_auto_attach(struct comedi_device *dev, spriv = s->private; spriv->next_time_min = jiffies + msecs_to_jiffies(500); - spriv->next_time_max = jiffies + msecs_to_jiffies(2000); } setup_timer(&devpriv->timer, jr3_pci_poll_dev, (unsigned long)dev); -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/11] staging: comedi: jr3_pci: cope with jiffies wraparound
The timer expiry routine `jr3_pci_poll_dev()` checks for expiry by checking whether the absolute value of `jiffies` (stored in local variable `now`) is greater than the expected expiry time in jiffy units. This will fail when `jiffies` wraps around. Also, it seems to make sense to handle the expiry one jiffy earlier than the current test. Use `time_after_eq()` to check for expiry. Cc: # 3.15+ Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/jr3_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/jr3_pci.c b/drivers/staging/comedi/drivers/jr3_pci.c index 25909a936e7c..eb0a095efe9c 100644 --- a/drivers/staging/comedi/drivers/jr3_pci.c +++ b/drivers/staging/comedi/drivers/jr3_pci.c @@ -611,7 +611,7 @@ static void jr3_pci_poll_dev(unsigned long data) s = &dev->subdevices[i]; spriv = s->private; - if (now > spriv->next_time_min) { + if (time_after_eq(now, spriv->next_time_min)) { struct jr3_pci_poll_delay sub_delay; sub_delay = jr3_pci_poll_subdevice(s); -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 00/36] i.MX Media Driver
On Fri, 2017-02-17 at 10:56 +, Russell King - ARM Linux wrote: > On Fri, Feb 17, 2017 at 11:39:11AM +0100, Philipp Zabel wrote: > > On Thu, 2017-02-16 at 22:57 +, Russell King - ARM Linux wrote: > > > On Thu, Feb 16, 2017 at 02:27:41PM -0800, Steve Longerbeam wrote: > > > > > > > > > > > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote: > > > > >On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote: > > > > >>In version 4: > > > > > > > > > >With this version, I get: > > > > > > > > > >[28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x > > > > >[28762.899409] ipu1_csi0: pipeline_set_stream failed with -110 > > > > > > > > > > > > > Right, in the imx219, on exit from s_power(), the clock and data lanes > > > > must be placed in the LP-11 state. This has been done in the ov5640 and > > > > tc358743 subdevs. > > > > > > The only way to do that is to enable streaming from the sensor, wait > > > an initialisation time, and then disable streaming, and wait for the > > > current line to finish. There is _no_ other way to get the sensor to > > > place its clock and data lines into LP-11 state. > > > > I thought going through LP-11 is part of the D-PHY transmitter > > initialization, during the LP->HS wakeup sequence. But then I have no > > access to MIPI specs. > > The D-PHY transmitter initialisation *only* happens as part of the > wake-up from standby to streaming mode. That is because Sony expect > that you program the sensor, and then when you switch it to streaming > mode, it computes the D-PHY parameters from the PLL, input clock rate > (you have to tell it the clock rate in 1/256 MHz units), number of > lanes, and other parameters. > > It is possible to program the D-PHY parameters manually, but that > doesn't change the above sequence in any way (it just avoids the > chip computing the values, it doesn't result in any change of > behaviour on the bus.) > > The IMX219 specifications are clear: the clock and data lines are > held low (LP-00 state) after releasing the hardware enable signal. > There's a period of chip initialisation, and then you can access the > I2C bus and configure it. There's a further period of initialisation > where charge pumps are getting to their operating state. Then, you > set the streaming bit, and a load more initialisation happens before > the CSI bus enters LP-11 state and the first frame pops out. When > entering standby, the last frame is completed, and then the CSI bus > enters LP-11 state. How about firing off a thread in imx6-mipi-csi2 prepare_stream that spins on the LP-11 check and then continues with the receiver D-PHY initialization once the condition is met? I think we should have at least 100 us to do this, but maybe the IMX219 can be programmed to stay in LP-11 for a longer time. > SMIA are slightly different - mostly following what I've said above, > but the clock and data lines are tristated after releasing the > xshutdown signal, and they remain tristated until the clock line > starts toggling before the first frame appears. There appears to > be no point that the clock line enters LP-11 state before it starts > toggling. When entering standby, the last frame is completed, and > the CSI bus enters tristate mode (so floating.) There is no way to > get these sensors into LP-11 state. I have no idea what to do about those. regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] Staging: wlan-ng: fixed block comments should align the * on each line
Right-shift a space key to align the * on each line. Signed-off-by: Bo YU --- drivers/staging/wlan-ng/p80211metastruct.h | 88 ++-- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/drivers/staging/wlan-ng/p80211metastruct.h b/drivers/staging/wlan-ng/p80211metastruct.h index 850d897fc163..c2ffec4daa90 100644 --- a/drivers/staging/wlan-ng/p80211metastruct.h +++ b/drivers/staging/wlan-ng/p80211metastruct.h @@ -1,48 +1,48 @@ /* This file is GENERATED AUTOMATICALLY. DO NOT EDIT OR MODIFY. -* -* -* Copyright (C) 1999 AbsoluteValue Systems, Inc. All Rights Reserved. -* -* -* linux-wlan -* -* The contents of this file are subject to the Mozilla Public -* License Version 1.1 (the "License"); you may not use this file -* except in compliance with the License. You may obtain a copy of -* the License at http://www.mozilla.org/MPL/ -* -* Software distributed under the License is distributed on an "AS -* IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or -* implied. See the License for the specific language governing -* rights and limitations under the License. -* -* Alternatively, the contents of this file may be used under the -* terms of the GNU Public License version 2 (the "GPL"), in which -* case the provisions of the GPL are applicable instead of the -* above. If you wish to allow the use of your version of this file -* only under the terms of the GPL and not to allow others to use -* your version of this file under the MPL, indicate your decision -* by deleting the provisions above and replace them with the notice -* and other provisions required by the GPL. If you do not delete -* the provisions above, a recipient may use your version of this -* file under either the MPL or the GPL. -* -* -* -* Inquiries regarding the linux-wlan Open Source project can be -* made directly to: -* -* AbsoluteValue Systems Inc. -* i...@linux-wlan.com -* http://www.linux-wlan.com -* -* -* -* Portions of the development of this software were funded by -* Intersil Corporation as part of PRISM(R) chipset product development. -* -* -*/ + * + * + * Copyright (C) 1999 AbsoluteValue Systems, Inc. All Rights Reserved. + * + * + * linux-wlan + * + * The contents of this file are subject to the Mozilla Public + * License Version 1.1 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a copy of + * the License at http://www.mozilla.org/MPL/ + * + * Software distributed under the License is distributed on an "AS + * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or + * implied. See the License for the specific language governing + * rights and limitations under the License. + * + * Alternatively, the contents of this file may be used under the + * terms of the GNU Public License version 2 (the "GPL"), in which + * case the provisions of the GPL are applicable instead of the + * above. If you wish to allow the use of your version of this file + * only under the terms of the GPL and not to allow others to use + * your version of this file under the MPL, indicate your decision + * by deleting the provisions above and replace them with the notice + * and other provisions required by the GPL. If you do not delete + * the provisions above, a recipient may use your version of this + * file under either the MPL or the GPL. + * + * + * + * Inquiries regarding the linux-wlan Open Source project can be + * made directly to: + * + * AbsoluteValue Systems Inc. + * i...@linux-wlan.com + * http://www.linux-wlan.com + * + * + * + * Portions of the development of this software were funded by + * Intersil Corporation as part of PRISM(R) chipset product development. + * + * + */ #ifndef _P80211MKMETASTRUCT_H #define _P80211MKMETASTRUCT_H -- 1.7.10.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev driver
On Fri, 2017-02-17 at 11:06 +, Russell King - ARM Linux wrote: > On Fri, Feb 17, 2017 at 11:47:59AM +0100, Philipp Zabel wrote: > > On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote: > > > +static void csi2_dphy_init(struct csi2_dev *csi2) > > > +{ > > > + /* > > > + * FIXME: 0x14 is derived from a fixed D-PHY reference > > > + * clock from the HSI_TX PLL, and a fixed target lane max > > > + * bandwidth of 300 Mbps. This value should be derived > > > > If the table in https://community.nxp.com/docs/DOC-94312 is correct, > > this should be 850 Mbps. Where does this 300 Mbps value come from? > > I thought you had some code to compute the correct value, although > I guess we've lost the ability to know how fast the sensor is going > to drive the link. I had code to calculate the number of needed lanes from the bit rate and link frequency. I did not actually change the D-PHY register value. And as you pointed out, calculating the number of lanes is not useful without input from the sensor driver, as some lane configurations might not be supported. > Note that the IMX219 currently drives the data lanes at 912Mbps almost > exclusively, as I've yet to finish working out how to derive the PLL > parameters. (I have something that works, but it currently takes on > the order of 100k iterations to derive the parameters. gcd() doesn't > help you in this instance.) The tc358743 also currently only implements a fixed rate (of 594 Mbps). regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 00/36] i.MX Media Driver
On Thu, 2017-02-16 at 14:27 -0800, Steve Longerbeam wrote: > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote: > > On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote: > >> In version 4: > > > > With this version, I get: > > > > [28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x > > [28762.899409] ipu1_csi0: pipeline_set_stream failed with -110 > > > > Right, in the imx219, on exit from s_power(), the clock and data lanes > must be placed in the LP-11 state. This has been done in the ov5640 and > tc358743 subdevs. > > If we want to bring in the patch that adds a .prepare_stream() op, > the csi-2 bus would need to be placed in LP-11 in that op instead. > > Philipp, should I go ahead and add your .prepare_stream() patch? I think with Russell's explanation of how the imx219 sensor operates, we'll have to do something before calling the sensor s_stream, but right now I'm still unsure what exactly. regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: rtl8192u: Fix warnings about endianness
drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c:564:37: warning: incorrect type in assignment (different base types) drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c:564:37:expected unsigned short [unsigned] [usertype] len drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c:564:37:got restricted __be16 [usertype] Signed-off-by: maomao xu diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c index d1057b1..1e81c24 100644 --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c @@ -559,10 +559,8 @@ void ieee80211_indicate_packets(struct ieee80211_device *ieee, struct ieee80211_ memcpy(skb_push(sub_skb, ETH_ALEN), prxb->src, ETH_ALEN); memcpy(skb_push(sub_skb, ETH_ALEN), prxb->dst, ETH_ALEN); } else { - u16 len; /* Leave Ethernet header part of hdr and full payload */ - len = htons(sub_skb->len); - memcpy(skb_push(sub_skb, 2), &len, 2); + put_unaligned_be16(sub_skb->len, skb_push(sub_skb, 2)); memcpy(skb_push(sub_skb, ETH_ALEN), prxb->src, ETH_ALEN); memcpy(skb_push(sub_skb, ETH_ALEN), prxb->dst, ETH_ALEN); } -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 00/36] i.MX Media Driver
Hi Philipp, Steve and Russell, On Fri, Feb 17, 2017 at 12:43:38PM +0100, Philipp Zabel wrote: > On Thu, 2017-02-16 at 14:27 -0800, Steve Longerbeam wrote: > > > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote: > > > On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote: > > >> In version 4: > > > > > > With this version, I get: > > > > > > [28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x > > > [28762.899409] ipu1_csi0: pipeline_set_stream failed with -110 > > > > > > > Right, in the imx219, on exit from s_power(), the clock and data lanes > > must be placed in the LP-11 state. This has been done in the ov5640 and > > tc358743 subdevs. > > > > If we want to bring in the patch that adds a .prepare_stream() op, > > the csi-2 bus would need to be placed in LP-11 in that op instead. > > > > Philipp, should I go ahead and add your .prepare_stream() patch? > > I think with Russell's explanation of how the imx219 sensor operates, > we'll have to do something before calling the sensor s_stream, but right > now I'm still unsure what exactly. Indeed there appears to be no other way to achieve the LP-11 state than going through the streaming state for this particular sensor, apart from starting streaming. Is there a particular reason why you're waiting for the transmitter to transfer to LP-11 state? That appears to be the last step which is done in the csi2_s_stream() callback. What the sensor does next is to start streaming, and the first thing it does in that process is to switch to LP-11 state. Have you tried what happens if you simply drop the LP-11 check? To me that would seem the right thing to do. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 00/36] i.MX Media Driver
On Fri, Feb 17, 2017 at 02:22:14PM +0200, Sakari Ailus wrote: > Hi Philipp, Steve and Russell, > > On Fri, Feb 17, 2017 at 12:43:38PM +0100, Philipp Zabel wrote: > > I think with Russell's explanation of how the imx219 sensor operates, > > we'll have to do something before calling the sensor s_stream, but right > > now I'm still unsure what exactly. > > Indeed there appears to be no other way to achieve the LP-11 state than > going through the streaming state for this particular sensor, apart from > starting streaming. > > Is there a particular reason why you're waiting for the transmitter to > transfer to LP-11 state? That appears to be the last step which is done in > the csi2_s_stream() callback. > > What the sensor does next is to start streaming, and the first thing it does > in that process is to switch to LP-11 state. > > Have you tried what happens if you simply drop the LP-11 check? To me that > would seem the right thing to do. The Freescale documentation for iMX6's CSI2 receiver (chapter 40.3.1) specifies a very specific sequence to be followed to safely bring up the CSI2 receiver. Bold text gets used, which implies emphasis on certain points, which suggests that it's important to follow it. Presumably, the reason for this is to ensure that a state machine within the CSI2 receiver is properly synchronised to the incoming data stream, and while avoiding the sequence may work, it may not be guaranteed to work every time. I guess we need someone from NXP to comment. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev driver
On Fri, 2017-02-17 at 11:47 +0100, Philipp Zabel wrote: > On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote: > > Adds MIPI CSI-2 Receiver subdev driver. This subdev is required > > for sensors with a MIPI CSI2 interface. > > > > Signed-off-by: Steve Longerbeam > > --- > > drivers/staging/media/imx/Makefile | 1 + > > drivers/staging/media/imx/imx6-mipi-csi2.c | 573 > > + > > 2 files changed, 574 insertions(+) > > create mode 100644 drivers/staging/media/imx/imx6-mipi-csi2.c > > > > diff --git a/drivers/staging/media/imx/Makefile > > b/drivers/staging/media/imx/Makefile > > index 878a126..3569625 100644 > > --- a/drivers/staging/media/imx/Makefile > > +++ b/drivers/staging/media/imx/Makefile > > @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-vdic.o > > obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-ic.o > > > > obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o > > +obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o > > diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c > > b/drivers/staging/media/imx/imx6-mipi-csi2.c > > new file mode 100644 > > index 000..23dca80 > > --- /dev/null > > +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c > > @@ -0,0 +1,573 @@ > > +/* > > + * MIPI CSI-2 Receiver Subdev for Freescale i.MX6 SOC. > > + * > > + * Copyright (c) 2012-2017 Mentor Graphics Inc. > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include "imx-media.h" > > + > > +/* > > + * there must be 5 pads: 1 input pad from sensor, and > > + * the 4 virtual channel output pads > > + */ > > +#define CSI2_SINK_PAD 0 > > +#define CSI2_NUM_SINK_PADS 1 > > +#define CSI2_NUM_SRC_PADS 4 > > +#define CSI2_NUM_PADS 5 > > + > > +struct csi2_dev { > > + struct device *dev; > > + struct v4l2_subdev sd; > > + struct media_pad pad[CSI2_NUM_PADS]; > > + struct v4l2_mbus_framefmt format_mbus; > > + struct clk *dphy_clk; > > + struct clk *cfg_clk; > > + struct clk *pix_clk; /* what is this? */ > > + void __iomem *base; > > + struct v4l2_of_bus_mipi_csi2 bus; > > + boolon; > > + boolstream_on; > > + boolsrc_linked; > > + boolsink_linked[CSI2_NUM_SRC_PADS]; > > +}; > > + > > +#define DEVICE_NAME "imx6-mipi-csi2" > > + > > +/* Register offsets */ > > +#define CSI2_VERSION0x000 > > +#define CSI2_N_LANES0x004 > > +#define CSI2_PHY_SHUTDOWNZ 0x008 > > +#define CSI2_DPHY_RSTZ 0x00c > > +#define CSI2_RESETN 0x010 > > +#define CSI2_PHY_STATE 0x014 > > +#define PHY_STOPSTATEDATA_BIT 4 > > +#define PHY_STOPSTATEDATA(n)BIT(PHY_STOPSTATEDATA_BIT + (n)) > > +#define PHY_RXCLKACTIVEHS BIT(8) > > +#define PHY_RXULPSCLKNOTBIT(9) > > +#define PHY_STOPSTATECLKBIT(10) > > +#define CSI2_DATA_IDS_1 0x018 > > +#define CSI2_DATA_IDS_2 0x01c > > +#define CSI2_ERR1 0x020 > > +#define CSI2_ERR2 0x024 > > +#define CSI2_MSK1 0x028 > > +#define CSI2_MSK2 0x02c > > +#define CSI2_PHY_TST_CTRL0 0x030 > > +#define PHY_TESTCLRBIT(0) > > +#define PHY_TESTCLKBIT(1) > > +#define CSI2_PHY_TST_CTRL1 0x034 > > +#define PHY_TESTEN BIT(16) > > +#define CSI2_SFT_RESET 0xf00 > > + > > +static inline struct csi2_dev *sd_to_dev(struct v4l2_subdev *sdev) > > +{ > > + return container_of(sdev, struct csi2_dev, sd); > > +} > > + > > +static void csi2_enable(struct csi2_dev *csi2, bool enable) > > +{ > > + if (enable) { > > + writel(0x1, csi2->base + CSI2_PHY_SHUTDOWNZ); > > + writel(0x1, csi2->base + CSI2_DPHY_RSTZ); > > + writel(0x1, csi2->base + CSI2_RESETN); > > + } else { > > + writel(0x0, csi2->base + CSI2_PHY_SHUTDOWNZ); > > + writel(0x0, csi2->base + CSI2_DPHY_RSTZ); > > + writel(0x0, csi2->base + CSI2_RESETN); > > + } > > +} > > + > > +static void csi2_set_lanes(struct csi2_dev *csi2) > > +{ > > + int lanes = csi2->bus.num_data_lanes; > > + > > + writel(lanes - 1, csi2->base + CSI2_N_LANES); > > +} > > + > > +static void dw_mipi_csi2_phy_write(struct csi2_dev *csi2, > > + u32 test_code, u32 test_data) > > +{ > > + /* Clear PHY test interface */ > > + writel(PHY_TESTCLR, csi2->base + CSI2_PHY_TST_CTRL0); > > + writel(0x0, csi2->base + CSI2_PHY_TST_CTRL1); > > + writel(0x0, csi2->base + CSI2_PHY_TST_CTRL0); > > + > > +
Re: [PATCH v7 1/2] staging: wlan-ng: move else if statement to a single line
On Thu, Feb 16, 2017 at 10:27:24AM -0800, Greg Kroah-Hartman wrote: > On Wed, Feb 15, 2017 at 10:11:00AM -0500, Maksymilian Piechota wrote: > > move else if statement to a single line > > > > Signed-off-by: Maksymilian Piechota > > --- > > drivers/staging/wlan-ng/prism2mgmt.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/staging/wlan-ng/prism2mgmt.c > > b/drivers/staging/wlan-ng/prism2mgmt.c > > index 16fb2d3..a45ff00 100644 > > --- a/drivers/staging/wlan-ng/prism2mgmt.c > > +++ b/drivers/staging/wlan-ng/prism2mgmt.c > > @@ -1307,8 +1307,7 @@ int prism2mgmt_wlansniff(struct wlandevice *wlandev, > > void *msgp) > > && (msg->prismheader.data == P80211ENUM_truth_true)) { > > hw->sniffhdr = 0; > > wlandev->netdev->type = ARPHRD_IEEE80211_PRISM; > > - } else > > - if ((msg->wlanheader.status == > > + } else if ((msg->wlanheader.status == > > P80211ENUM_msgitem_status_data_ok) > > && (msg->wlanheader.data == P80211ENUM_truth_true)) { > > hw->sniffhdr = 1; > > This patch does not apply to my tree at all :( > > Please rebase and resend the series. > > thanks, > > greg k-h ok, I've got newest sources and this change is already applied. So what are the next steps suggested? I've heard there is a possibility to obtain mentor, right? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: octeon: remove unused variable
A cleanup patch left one local variable without a reference: drivers/staging/octeon/ethernet-rx.c:339:28: warning: unused variable 'priv' [-Wunused-variable] This removes the declaration too. Fixes: 66812da3a689 ("staging: octeon: Use net_device_stats from struct net_device") Signed-off-by: Arnd Bergmann --- drivers/staging/octeon/ethernet-rx.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c index 7f8cf875157c..65a285631994 100644 --- a/drivers/staging/octeon/ethernet-rx.c +++ b/drivers/staging/octeon/ethernet-rx.c @@ -336,7 +336,6 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget) if (likely((port < TOTAL_NUMBER_OF_PORTS) && cvm_oct_device[port])) { struct net_device *dev = cvm_oct_device[port]; - struct octeon_ethernet *priv = netdev_priv(dev); /* * Only accept packets for devices that are -- 2.9.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 00/36] i.MX Media Driver
On Fri, 2017-02-17 at 14:22 +0200, Sakari Ailus wrote: > Hi Philipp, Steve and Russell, > > On Fri, Feb 17, 2017 at 12:43:38PM +0100, Philipp Zabel wrote: > > On Thu, 2017-02-16 at 14:27 -0800, Steve Longerbeam wrote: > > > > > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote: > > > > On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote: > > > >> In version 4: > > > > > > > > With this version, I get: > > > > > > > > [28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x > > > > [28762.899409] ipu1_csi0: pipeline_set_stream failed with -110 > > > > > > > > > > Right, in the imx219, on exit from s_power(), the clock and data lanes > > > must be placed in the LP-11 state. This has been done in the ov5640 and > > > tc358743 subdevs. > > > > > > If we want to bring in the patch that adds a .prepare_stream() op, > > > the csi-2 bus would need to be placed in LP-11 in that op instead. > > > > > > Philipp, should I go ahead and add your .prepare_stream() patch? > > > > I think with Russell's explanation of how the imx219 sensor operates, > > we'll have to do something before calling the sensor s_stream, but right > > now I'm still unsure what exactly. > > Indeed there appears to be no other way to achieve the LP-11 state than > going through the streaming state for this particular sensor, apart from > starting streaming. > > Is there a particular reason why you're waiting for the transmitter to > transfer to LP-11 state? That appears to be the last step which is done in > the csi2_s_stream() callback. > > What the sensor does next is to start streaming, and the first thing it does > in that process is to switch to LP-11 state. > > Have you tried what happens if you simply drop the LP-11 check? To me that > would seem the right thing to do. Removing the wait for LP-11 alone might not be an issue in my case, as the TC358743 is known to be in stop state all along. So I just have to make sure that the time between s_stream(csi2) starting the receiver and s_stream(tc358743) causing LP-11 to be changed to the next state is long enough for the receiver to detect LP-11 (which I really can't, I just have to pray I2C transmissions are slow enough). The problems start if we have to enable the D-PHY and deassert resets either before the sensor enters LP-11 state or after it already started streaming, because we don't know when the sensor drives that state on the bus. The latter case I is simulated easily by again changing the order so that the "sensor" (tc358743) is enabled before the CSI-2 receiver D-PHY initialization. The result is that captures time out, presumably because the receiver never entered HS mode because it didn't see LP-11. The PHY_STATE register contains 0x200, meaning RXCLKACTIVEHS (which we should wait for in step 7.) is never set. I tried to test the former by instead modifying the tc358743 driver a bit: --8<-- diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c index 39d4cdd328c0f..43df80903215b 100644 --- a/drivers/media/i2c/tc358743.c +++ b/drivers/media/i2c/tc358743.c @@ -1378,8 +1378,6 @@ static int tc358743_s_dv_timings(struct v4l2_subdev *sd, state->timings = *timings; enable_stream(sd, false); - tc358743_set_pll(sd); - tc358743_set_csi(sd); return 0; } @@ -1469,6 +1467,11 @@ static int tc358743_g_mbus_config(struct v4l2_subdev *sd, static int tc358743_s_stream(struct v4l2_subdev *sd, int enable) { + if (enable) { + tc358743_set_pll(sd); + tc358743_set_csi(sd); + tc358743_set_csi_color_space(sd); + } enable_stream(sd, enable); if (!enable) { /* Put all lanes in PL-11 state (STOPSTATE) */ @@ -1657,9 +1660,6 @@ static int tc358743_set_fmt(struct v4l2_subdev *sd, state->vout_color_sel = vout_color_sel; enable_stream(sd, false); - tc358743_set_pll(sd); - tc358743_set_csi(sd); - tc358743_set_csi_color_space(sd); return 0; } -->8-- That should enable the CSI-2 Tx and put it in LP-11 only after the CSI-2 receiver is enabled, right before starting streaming. That did seem to work the few times I tested, but I have no idea how this will behave with other chips that do something else to the bus while not streaming, and whether it is ok to enable the CSI right after the sensor without waiting for the CSI-2 bus to settle. regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [media] Staging: media/lirc: don't call put_ir_rx on rx twice
From: Colin Ian King There is an exit path where rx is kfree'd on put_ir_rx and then a jump to label out_put_xx will again kfree it with another call to put_ir_rx. Fix this by adding a new label that avoids this 2nd call to put_ir_rx for this specific case. Detected with CoverityScan, CID#145119 ("Use after free") Signed-off-by: Colin Ian King --- drivers/staging/media/lirc/lirc_zilog.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/lirc/lirc_zilog.c b/drivers/staging/media/lirc/lirc_zilog.c index 34aac3e..5dd1e62 100644 --- a/drivers/staging/media/lirc/lirc_zilog.c +++ b/drivers/staging/media/lirc/lirc_zilog.c @@ -1597,7 +1597,7 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) i2c_set_clientdata(client, NULL); put_ir_rx(rx, true); ir->l.features &= ~LIRC_CAN_REC_LIRCCODE; - goto out_put_xx; + goto out_put_tx; } /* Proceed only if the Tx client is also ready */ @@ -1637,6 +1637,7 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) out_put_xx: if (rx != NULL) put_ir_rx(rx, true); +out_put_tx: if (tx != NULL) put_ir_tx(tx, true); out_put_ir: -- 2.10.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/3] x86/vdso: Add Hyper-V TSC page clocksource support
On Fri, Feb 17, 2017 at 2:14 AM, Vitaly Kuznetsov wrote: > Thomas Gleixner writes: > >> On Wed, 15 Feb 2017, Vitaly Kuznetsov wrote: >>> Actually, we already have an implementation of TSC page update in KVM >>> (see arch/x86/kvm/hyperv.c, kvm_hv_setup_tsc_page()) and the update does >>> the following: >>> >>> 0) stash seq into seq_prev >>> 1) seq = 0 making all reads from the page invalid >>> 2) smp_wmb() >>> 3) update tsc_scale, tsc_offset >>> 4) smp_wmb() >>> 5) set seq = seq_prev + 1 >> >> I hope they handle the case where seq_prev overflows and becomes 0 :) >> >>> As far as I understand this helps with situations you described above as >>> guest will notice either invalid value of 0 or seq change. In case the >>> implementation in real Hyper-V is the same we're safe with compile >>> barriers only. >> >> On x86 that's correct. smp_rmb() resolves to barrier(), but you certainly >> need the smp_wmb() on the writer side. >> >> Now looking at the above your reader side code is bogus: >> >> + while (1) { >> + sequence = tsc_pg->tsc_sequence; >> + if (!sequence) >> + break; >> >> Why would you break out of the loop when seq is 0? The 0 is just telling >> you that there is an update in progress. > > Not only. As far as I understand (and I *think* K. Y. pointed this out) > when VM is migrating to another host TSC page clocksource is disabled for > extended period of time so we're better off reading from MSR than > looping here. With regards to VDSO this means reverting to doing normal > syscall. That's a crappy design. The guest really ought to be able to distinguish "busy, try again" from "bail and use MSR". Thomas, I can see valid reasons why the hypervisor might want to temporarily disable shared page-based timing, but I think it's silly that it's conflated with indicating "retry". But if this is indeed the ABI, we're stuck with it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] hv: hide unused label
On Tue, 14 Feb 2017 22:17:17 +0100 Arnd Bergmann wrote: > This new 32-bit warning just showed up: > > arch/x86/hyperv/hv_init.c: In function 'hyperv_init': > arch/x86/hyperv/hv_init.c:167:1: error: label 'register_msr_cs' defined but > not used [-Werror=unused-label] > > The easiest solution is to move the label up into the existing #ifdef that > has the goto. > > Fixes: dee863b571b0 ("hv: export current Hyper-V clocksource") > Signed-off-by: Arnd Bergmann Acked-by: Stephen Hemminger ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] pci-hyperv: Use device serial number as PCI domain
On Mon, 13 Feb 2017 18:10:11 + Haiyang Zhang wrote: > This allows PCI domain numbers starts with 1, and also unique > on the same VM. So names, such as VF NIC names, that include > domain number as part of the name, can be shorter than that > based on part of bus UUID previously. The new names will also > stay same for VMs created with copied VHD and same number of > devices. > > Signed-off-by: Haiyang Zhang > Reviewed-by: K. Y. Srinivasan Looks like a good solution to persistent names. Signed-off-by: Stephen Hemminger ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/3] staging: rtl8192u: Prefer using the BIT macro
Replaces left shift operation (1 << d) by BIT(x) macro. Fix the checkpatch.pl issue: CHECK: Prefer using the BIT macro replacing Signed-off-by: simran singhal --- V2: -modified patch message. drivers/staging/rtl8192u/ieee80211/ieee80211.h | 140 - 1 file changed, 70 insertions(+), 70 deletions(-) diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h index a020e04..648806b 100644 --- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h @@ -502,28 +502,28 @@ do { if (ieee80211_debug_level & (level)) \ * */ -#define IEEE80211_DL_INFO (1<<0) -#define IEEE80211_DL_WX(1<<1) -#define IEEE80211_DL_SCAN (1<<2) -#define IEEE80211_DL_STATE (1<<3) -#define IEEE80211_DL_MGMT (1<<4) -#define IEEE80211_DL_FRAG (1<<5) -#define IEEE80211_DL_EAP (1<<6) -#define IEEE80211_DL_DROP (1<<7) - -#define IEEE80211_DL_TX(1<<8) -#define IEEE80211_DL_RX(1<<9) - -#define IEEE80211_DL_HT (1<<10) //HT -#define IEEE80211_DL_BA (1<<11) //ba -#define IEEE80211_DL_TS (1<<12) //TS -#define IEEE80211_DL_QOS (1<<13) -#define IEEE80211_DL_REORDER (1<<14) -#define IEEE80211_DL_IOT (1<<15) -#define IEEE80211_DL_IPS (1<<16) -#define IEEE80211_DL_TRACE(1<<29) //trace function, need to user net_ratelimit() together in order not to print too much to the screen -#define IEEE80211_DL_DATA (1<<30) //use this flag to control whether print data buf out. -#define IEEE80211_DL_ERR (1<<31) //always open +#define IEEE80211_DL_INFO BIT(0) +#define IEEE80211_DL_WXBIT(1) +#define IEEE80211_DL_SCAN BIT(2) +#define IEEE80211_DL_STATE BIT(3) +#define IEEE80211_DL_MGMT BIT(4) +#define IEEE80211_DL_FRAG BIT(5) +#define IEEE80211_DL_EAP BIT(6) +#define IEEE80211_DL_DROP BIT(7) + +#define IEEE80211_DL_TXBIT(8) +#define IEEE80211_DL_RXBIT(9) + +#define IEEE80211_DL_HT BIT(10) //HT +#define IEEE80211_DL_BA BIT(11) //ba +#define IEEE80211_DL_TS BIT(12) //TS +#define IEEE80211_DL_QOS BIT(13) +#define IEEE80211_DL_REORDER BIT(14) +#define IEEE80211_DL_IOT BIT(15) +#define IEEE80211_DL_IPS BIT(16) +#define IEEE80211_DL_TRACEBIT(29) //trace function, need to user net_ratelimit() together in order not to print too much to the screen +#define IEEE80211_DL_DATA BIT(30) //use this flag to control whether print data buf out. +#define IEEE80211_DL_ERR BIT(31) //always open #define IEEE80211_ERROR(f, a...) printk(KERN_ERR "ieee80211: " f, ## a) #define IEEE80211_WARNING(f, a...) printk(KERN_WARNING "ieee80211: " f, ## a) #define IEEE80211_DEBUG_INFO(f, a...) IEEE80211_DEBUG(IEEE80211_DL_INFO, f, ## a) @@ -625,18 +625,18 @@ struct ieee80211_snap_hdr { #define IEEE80211_OFDM_RATE_54MB 0x6C #define IEEE80211_BASIC_RATE_MASK 0x80 -#define IEEE80211_CCK_RATE_1MB_MASK(1<<0) -#define IEEE80211_CCK_RATE_2MB_MASK(1<<1) -#define IEEE80211_CCK_RATE_5MB_MASK(1<<2) -#define IEEE80211_CCK_RATE_11MB_MASK (1<<3) -#define IEEE80211_OFDM_RATE_6MB_MASK (1<<4) -#define IEEE80211_OFDM_RATE_9MB_MASK (1<<5) -#define IEEE80211_OFDM_RATE_12MB_MASK (1<<6) -#define IEEE80211_OFDM_RATE_18MB_MASK (1<<7) -#define IEEE80211_OFDM_RATE_24MB_MASK (1<<8) -#define IEEE80211_OFDM_RATE_36MB_MASK (1<<9) -#define IEEE80211_OFDM_RATE_48MB_MASK (1<<10) -#define IEEE80211_OFDM_RATE_54MB_MASK (1<<11) +#define IEEE80211_CCK_RATE_1MB_MASKBIT(0) +#define IEEE80211_CCK_RATE_2MB_MASKBIT(1) +#define IEEE80211_CCK_RATE_5MB_MASKBIT(2) +#define IEEE80211_CCK_RATE_11MB_MASK BIT(3) +#define IEEE80211_OFDM_RATE_6MB_MASK BIT(4) +#define IEEE80211_OFDM_RATE_9MB_MASK BIT(5) +#define IEEE80211_OFDM_RATE_12MB_MASK BIT(6) +#define IEEE80211_OFDM_RATE_18MB_MASK BIT(7) +#define IEEE80211_OFDM_RATE_24MB_MASK BIT(8) +#define IEEE80211_OFDM_RATE_36MB_MASK BIT(9) +#define IEEE80211_OFDM_RATE_48MB_MASK BIT(10) +#define IEEE80211_OFDM_RATE_54MB_MASK BIT(11) #define IEEE80211_CCK_RATES_MASK 0x000F #define IEEE80211_CCK_BASIC_RATES_MASK (IEEE80211_CCK_RATE_1MB_MASK | \ @@ -794,16 +794,16 @@ struct ieee80211_device; #include "ieee80211_crypt.h" -#define SEC_KEY_1 (1<<0) -#define SEC_KEY_2 (1<<1) -#define SEC_KEY_3 (1<<2) -#define SEC_KEY_4 (1<<3) -#define SEC_ACTIVE_KEY(1<<4) -#define SEC_AUTH_MODE (1<<5) -#define SEC_UNICAST_GROUP (1<<6) -
[PATCH 01/10] vmbus: only reschedule tasklet if time limit exceeded
The change to reschedule tasklet if more data arrives in ring buffer can cause performance regression if host timing is such that the next response happens in small window. Go back to a modified version of the original looping behavior. If the race occurs in a small time, then loop. But if the tasklet has been running for a long interval due to flood, then reschedule the tasklet to allow migration to ksoftirqd. --- drivers/hv/connection.c | 65 ++--- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index a8366fec1458..fce27fb141cc 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -296,44 +296,47 @@ struct vmbus_channel *relid2channel(u32 relid) /* * vmbus_on_event - Process a channel event notification + * + * For batched channels (default) optimize host to guest signaling + * by ensuring: + * 1. While reading the channel, we disable interrupts from host. + * 2. Ensure that we process all posted messages from the host + *before returning from this callback. + * 3. Once we return, enable signaling from the host. Once this + *state is set we check to see if additional packets are + *available to read. In this case we repeat the process. + *If this tasklet has been running for a long time + *then reschedule ourselves. */ void vmbus_on_event(unsigned long data) { struct vmbus_channel *channel = (void *) data; - void (*callback_fn)(void *); + unsigned long time_limit = jiffies + 2; - /* -* A channel once created is persistent even when there -* is no driver handling the device. An unloading driver -* sets the onchannel_callback to NULL on the same CPU -* as where this interrupt is handled (in an interrupt context). -* Thus, checking and invoking the driver specific callback takes -* care of orderly unloading of the driver. -*/ - callback_fn = READ_ONCE(channel->onchannel_callback); - if (unlikely(callback_fn == NULL)) - return; - - (*callback_fn)(channel->channel_callback_context); - - if (channel->callback_mode == HV_CALL_BATCHED) { - /* -* This callback reads the messages sent by the host. -* We can optimize host to guest signaling by ensuring: -* 1. While reading the channel, we disable interrupts from -*host. -* 2. Ensure that we process all posted messages from the host -*before returning from this callback. -* 3. Once we return, enable signaling from the host. Once this -*state is set we check to see if additional packets are -*available to read. In this case we repeat the process. + do { + void (*callback_fn)(void *); + + /* A channel once created is persistent even when +* there is no driver handling the device. An +* unloading driver sets the onchannel_callback to NULL. */ - if (hv_end_read(&channel->inbound) != 0) { - hv_begin_read(&channel->inbound); + callback_fn = READ_ONCE(channel->onchannel_callback); + if (unlikely(callback_fn == NULL)) + return; - tasklet_schedule(&channel->callback_event); - } - } + (*callback_fn)(channel->channel_callback_context); + + if (channel->callback_mode != HV_CALL_BATCHED) + return; + + if (likely(hv_end_read(&channel->inbound) == 0)) + return; + + hv_begin_read(&channel->inbound); + } while (likely(time_before(jiffies, time_limit))); + + /* The time limit (2 jiffies) has been reached */ + tasklet_schedule(&channel->callback_event); } /* -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/10] hyperv: fix warning about missing prototype
Compiling with warnings enabled finds missing prototype for hv_do_hypercall. Signed-off-by: Stephen Hemminger --- arch/x86/hyperv/hv_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index db64baf0e500..d4a5f820af5d 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -25,7 +25,7 @@ #include #include #include - +#include #ifdef CONFIG_X86_64 -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/10] vmbus: remove useless return's
No need for empty return at end of void function Signed-off-by: Stephen Hemminger --- drivers/hv/hv_balloon.c | 2 -- drivers/hv/hv_fcopy.c| 2 -- drivers/hv/hv_kvp.c | 2 -- drivers/hv/hv_snapshot.c | 2 -- drivers/hv/ring_buffer.c | 2 -- drivers/hv/vmbus_drv.c | 2 -- include/linux/hyperv.h | 2 -- 7 files changed, 14 deletions(-) diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index 5fd03e59cee5..f5728deff893 100644 --- a/drivers/hv/hv_balloon.c +++ b/drivers/hv/hv_balloon.c @@ -722,8 +722,6 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size, 5*HZ); post_status(&dm_device); } - - return; } static void hv_online_page(struct page *pg) diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c index 9aee6014339d..3ce7559d7b41 100644 --- a/drivers/hv/hv_fcopy.c +++ b/drivers/hv/hv_fcopy.c @@ -187,8 +187,6 @@ static void fcopy_send_data(struct work_struct *dummy) } } kfree(smsg_out); - - return; } /* diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index de263712e247..a65b7f88d7aa 100644 --- a/drivers/hv/hv_kvp.c +++ b/drivers/hv/hv_kvp.c @@ -484,8 +484,6 @@ kvp_send_key(struct work_struct *dummy) } kfree(message); - - return; } /* diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c index bcc03f0748d6..216d02277759 100644 --- a/drivers/hv/hv_snapshot.c +++ b/drivers/hv/hv_snapshot.c @@ -213,8 +213,6 @@ static void vss_send_op(void) } kfree(vss_msg); - - return; } static void vss_handle_request(struct work_struct *dummy) diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index 87799e81af97..d0ff5b41161a 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -73,8 +73,6 @@ static void hv_signal_on_write(u32 old_write, struct vmbus_channel *channel) */ if (old_write == READ_ONCE(rbi->ring_buffer->read_index)) vmbus_setevent(channel); - - return; } /* Get the next write location for the specified ring buffer. */ diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 971ecb7c4795..9ddbf4d03536 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -785,8 +785,6 @@ static void vmbus_shutdown(struct device *child_device) if (drv->shutdown) drv->shutdown(dev); - - return; } diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index c4c7ae91f9d1..443788e1a54e 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1507,8 +1507,6 @@ static inline void hv_signal_on_read(struct vmbus_channel *channel) cached_write_sz = hv_get_cached_bytes_to_write(rbi); if (cached_write_sz < pending_sz) vmbus_setevent(channel); - - return; } static inline void -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 00/10] vmbus: related patches
This is mostly the same set of vmbus cleanups and bug fixes as v2. In addition: * Addresses rcu sparse annotation issue caught by kbuild * Exposes ring debug info -- intended for later debugfs hook in networking Stephen Hemminger (10): vmbus: only reschedule tasklet if time limit exceeded vmbus: use rcu for per-cpu channel list hyperv: fix warning about missing prototype vmbus: remove useless return's vmbus: remove unnecessary initialization vmbus: fix spelling errors hyperv: remove unnecessary return variable vmbus: make channel_message table constant vmbus: cleanup header file style vmbus: expose debug info for drivers arch/x86/hyperv/hv_init.c | 2 +- drivers/hv/channel.c | 10 drivers/hv/channel_mgmt.c | 55 +++ drivers/hv/connection.c | 65 +-- drivers/hv/hv_balloon.c | 2 -- drivers/hv/hv_fcopy.c | 2 -- drivers/hv/hv_kvp.c | 12 - drivers/hv/hv_snapshot.c | 2 -- drivers/hv/hyperv_vmbus.h | 29 +++-- drivers/hv/ring_buffer.c | 22 ++-- drivers/hv/vmbus_drv.c| 10 +--- include/linux/hyperv.h| 38 +-- 12 files changed, 126 insertions(+), 123 deletions(-) -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/10] hyperv: remove unnecessary return variable
hv_ringbuffer_read cleanup. Signed-off-by: Stephen Hemminger --- drivers/hv/ring_buffer.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index 52d0556a5a25..8a249740b4b4 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -341,13 +341,11 @@ int hv_ringbuffer_read(struct vmbus_channel *channel, struct vmpacket_descriptor desc; u32 offset; u32 packetlen; - int ret = 0; struct hv_ring_buffer_info *inring_info = &channel->inbound; if (buflen <= 0) return -EINVAL; - *buffer_actual_len = 0; *requestid = 0; @@ -358,7 +356,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel, * No error is set when there is even no header, drivers are * supposed to analyze buffer_actual_len. */ - return ret; + return 0; } init_cached_read_index(channel); @@ -403,5 +401,5 @@ int hv_ringbuffer_read(struct vmbus_channel *channel, hv_signal_on_read(channel); - return ret; + return 0; } -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/10] vmbus: make channel_message table constant
This table is immutable and should be const. Cleanup indentation and whitespace for this as well. Signed-off-by: Stephen Hemminger --- drivers/hv/channel_mgmt.c | 48 +++ drivers/hv/hyperv_vmbus.h | 2 +- drivers/hv/vmbus_drv.c| 2 +- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index d2cfa3eb71a2..e8f52c0f6dc6 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -1098,30 +1098,30 @@ static void vmbus_onversion_response( } /* Channel message dispatch table */ -struct vmbus_channel_message_table_entry - channel_message_table[CHANNELMSG_COUNT] = { - {CHANNELMSG_INVALID,0, NULL}, - {CHANNELMSG_OFFERCHANNEL, 0, vmbus_onoffer}, - {CHANNELMSG_RESCIND_CHANNELOFFER, 0, vmbus_onoffer_rescind}, - {CHANNELMSG_REQUESTOFFERS, 0, NULL}, - {CHANNELMSG_ALLOFFERS_DELIVERED,1, vmbus_onoffers_delivered}, - {CHANNELMSG_OPENCHANNEL,0, NULL}, - {CHANNELMSG_OPENCHANNEL_RESULT, 1, vmbus_onopen_result}, - {CHANNELMSG_CLOSECHANNEL, 0, NULL}, - {CHANNELMSG_GPADL_HEADER, 0, NULL}, - {CHANNELMSG_GPADL_BODY, 0, NULL}, - {CHANNELMSG_GPADL_CREATED, 1, vmbus_ongpadl_created}, - {CHANNELMSG_GPADL_TEARDOWN, 0, NULL}, - {CHANNELMSG_GPADL_TORNDOWN, 1, vmbus_ongpadl_torndown}, - {CHANNELMSG_RELID_RELEASED, 0, NULL}, - {CHANNELMSG_INITIATE_CONTACT, 0, NULL}, - {CHANNELMSG_VERSION_RESPONSE, 1, vmbus_onversion_response}, - {CHANNELMSG_UNLOAD, 0, NULL}, - {CHANNELMSG_UNLOAD_RESPONSE,1, vmbus_unload_response}, - {CHANNELMSG_18, 0, NULL}, - {CHANNELMSG_19, 0, NULL}, - {CHANNELMSG_20, 0, NULL}, - {CHANNELMSG_TL_CONNECT_REQUEST, 0, NULL}, +const struct vmbus_channel_message_table_entry +channel_message_table[CHANNELMSG_COUNT] = { + { CHANNELMSG_INVALID, 0, NULL }, + { CHANNELMSG_OFFERCHANNEL, 0, vmbus_onoffer }, + { CHANNELMSG_RESCIND_CHANNELOFFER, 0, vmbus_onoffer_rescind }, + { CHANNELMSG_REQUESTOFFERS, 0, NULL }, + { CHANNELMSG_ALLOFFERS_DELIVERED, 1, vmbus_onoffers_delivered }, + { CHANNELMSG_OPENCHANNEL, 0, NULL }, + { CHANNELMSG_OPENCHANNEL_RESULT,1, vmbus_onopen_result }, + { CHANNELMSG_CLOSECHANNEL, 0, NULL }, + { CHANNELMSG_GPADL_HEADER, 0, NULL }, + { CHANNELMSG_GPADL_BODY,0, NULL }, + { CHANNELMSG_GPADL_CREATED, 1, vmbus_ongpadl_created }, + { CHANNELMSG_GPADL_TEARDOWN,0, NULL }, + { CHANNELMSG_GPADL_TORNDOWN,1, vmbus_ongpadl_torndown }, + { CHANNELMSG_RELID_RELEASED,0, NULL }, + { CHANNELMSG_INITIATE_CONTACT, 0, NULL }, + { CHANNELMSG_VERSION_RESPONSE, 1, vmbus_onversion_response }, + { CHANNELMSG_UNLOAD,0, NULL }, + { CHANNELMSG_UNLOAD_RESPONSE, 1, vmbus_unload_response }, + { CHANNELMSG_18,0, NULL }, + { CHANNELMSG_19,0, NULL }, + { CHANNELMSG_20,0, NULL }, + { CHANNELMSG_TL_CONNECT_REQUEST,0, NULL }, }; /* diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 884f83bba1ab..b552c3a4dd3c 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -376,7 +376,7 @@ struct vmbus_channel_message_table_entry { void (*message_handler)(struct vmbus_channel_message_header *msg); }; -extern struct vmbus_channel_message_table_entry +extern const struct vmbus_channel_message_table_entry channel_message_table[CHANNELMSG_COUNT]; diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 9ddbf4d03536..7fe8ef311795 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -851,7 +851,7 @@ void vmbus_on_msg_dpc(unsigned long data) struct hv_message *msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT; struct vmbus_channel_message_header *hdr; - struct vmbus_channel_message_table_entry *entry; + const struct vmbus_channel_message_table_entry *entry; struct onmessage_work_context *ctx; u32 message_type = msg->header.message_type; -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/10] vmbus: use rcu for per-cpu channel list
The per-cpu channel list is now referred to in the interrupt routine. This is mostly safe since the host will not normally generate an interrupt when channel is being deleted but if it did then there would be a use after free problem. To solve, this use RCU protection on ther per-cpu list. Signed-off-by: Stephen Hemminger --- drivers/hv/channel_mgmt.c | 7 --- drivers/hv/vmbus_drv.c| 6 +- include/linux/hyperv.h| 7 +++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index f33465d78a02..d2cfa3eb71a2 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -350,7 +350,8 @@ static struct vmbus_channel *alloc_channel(void) static void free_channel(struct vmbus_channel *channel) { tasklet_kill(&channel->callback_event); - kfree(channel); + + kfree_rcu(channel, rcu); } static void percpu_channel_enq(void *arg) @@ -359,14 +360,14 @@ static void percpu_channel_enq(void *arg) struct hv_per_cpu_context *hv_cpu = this_cpu_ptr(hv_context.cpu_context); - list_add_tail(&channel->percpu_list, &hv_cpu->chan_list); + list_add_tail_rcu(&channel->percpu_list, &hv_cpu->chan_list); } static void percpu_channel_deq(void *arg) { struct vmbus_channel *channel = arg; - list_del(&channel->percpu_list); + list_del_rcu(&channel->percpu_list); } diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index f7f6b9144b07..971ecb7c4795 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -937,8 +937,10 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu) if (relid == 0) continue; + rcu_read_lock(); + /* Find channel based on relid */ - list_for_each_entry(channel, &hv_cpu->chan_list, percpu_list) { + list_for_each_entry_rcu(channel, &hv_cpu->chan_list, percpu_list) { if (channel->offermsg.child_relid != relid) continue; @@ -954,6 +956,8 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu) tasklet_schedule(&channel->callback_event); } } + + rcu_read_unlock(); } } diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 62bbf3c1aa4a..c4c7ae91f9d1 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -845,6 +845,13 @@ struct vmbus_channel { * link up channels based on their CPU affinity. */ struct list_head percpu_list; + + /* +* Defer freeing channel until after all cpu's have +* gone through grace period. +*/ + struct rcu_head rcu; + /* * For performance critical channels (storage, networking * etc,), Hyper-V has a mechanism to enhance the throughput -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/10] vmbus: remove unnecessary initialization
Don't initialize variables that are then set a few lines later. Signed-off-by: Stephen Hemminger --- drivers/hv/ring_buffer.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index d0ff5b41161a..52d0556a5a25 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -265,14 +265,13 @@ void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info) int hv_ringbuffer_write(struct vmbus_channel *channel, const struct kvec *kv_list, u32 kv_count) { - int i = 0; + int i; u32 bytes_avail_towrite; - u32 totalbytes_towrite = 0; - + u32 totalbytes_towrite = sizeof(u64); u32 next_write_location; u32 old_write; - u64 prev_indices = 0; - unsigned long flags = 0; + u64 prev_indices; + unsigned long flags; struct hv_ring_buffer_info *outring_info = &channel->outbound; if (channel->rescind) @@ -281,8 +280,6 @@ int hv_ringbuffer_write(struct vmbus_channel *channel, for (i = 0; i < kv_count; i++) totalbytes_towrite += kv_list[i].iov_len; - totalbytes_towrite += sizeof(u64); - spin_lock_irqsave(&outring_info->ring_lock, flags); bytes_avail_towrite = hv_get_bytes_to_write(outring_info); @@ -339,7 +336,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel, u64 *requestid, bool raw) { u32 bytes_avail_toread; - u32 next_read_location = 0; + u32 next_read_location; u64 prev_indices = 0; struct vmpacket_descriptor desc; u32 offset; -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/10] vmbus: cleanup header file style
Minor changes to align hyper-v vmbus include files with current linux kernel style. Signed-off-by: Stephen Hemminger --- drivers/hv/hyperv_vmbus.h | 16 include/linux/hyperv.h| 12 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index b552c3a4dd3c..a69b52de8d56 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -218,8 +218,8 @@ struct hv_per_cpu_context { struct hv_context { /* We only support running on top of Hyper-V - * So at this point this really can only contain the Hyper-V ID - */ +* So at this point this really can only contain the Hyper-V ID +*/ u64 guestid; void *tsc_page; @@ -403,17 +403,17 @@ int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep); void vmbus_on_event(unsigned long data); void vmbus_on_msg_dpc(unsigned long data); -int hv_kvp_init(struct hv_util_service *); +int hv_kvp_init(struct hv_util_service *srv); void hv_kvp_deinit(void); -void hv_kvp_onchannelcallback(void *); +void hv_kvp_onchannelcallback(void *context); -int hv_vss_init(struct hv_util_service *); +int hv_vss_init(struct hv_util_service *srv); void hv_vss_deinit(void); -void hv_vss_onchannelcallback(void *); +void hv_vss_onchannelcallback(void *context); -int hv_fcopy_init(struct hv_util_service *); +int hv_fcopy_init(struct hv_util_service *srv); void hv_fcopy_deinit(void); -void hv_fcopy_onchannelcallback(void *); +void hv_fcopy_onchannelcallback(void *context); void vmbus_initiate_unload(bool crash); static inline void hv_poll_channel(struct vmbus_channel *channel, diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 443788e1a54e..9d5df04bb678 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -524,10 +524,10 @@ struct vmbus_channel_open_channel { u32 target_vp; /* - * The upstream ring buffer begins at offset zero in the memory - * described by RingBufferGpadlHandle. The downstream ring buffer - * follows it at this offset (in pages). - */ +* The upstream ring buffer begins at offset zero in the memory +* described by RingBufferGpadlHandle. The downstream ring buffer +* follows it at this offset (in pages). +*/ u32 downstream_ringbuffer_pageoffset; /* User-specific data to be passed along to the server endpoint. */ @@ -1013,7 +1013,7 @@ extern int vmbus_open(struct vmbus_channel *channel, u32 recv_ringbuffersize, void *userdata, u32 userdatalen, - void(*onchannel_callback)(void *context), + void (*onchannel_callback)(void *context), void *context); extern void vmbus_close(struct vmbus_channel *channel); @@ -1428,7 +1428,7 @@ struct hyperv_service_callback { char *log_msg; uuid_le data; struct vmbus_channel *channel; - void (*callback) (void *context); + void (*callback)(void *context); }; #define MAX_SRV_VER0x7ff -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 06/10] vmbus: fix spelling errors
Several spelling errors in comments Signed-off-by: Stephen Hemminger --- drivers/hv/channel.c | 10 +- drivers/hv/hv_kvp.c | 10 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index 81a80c82f1bd..deb852238b2d 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -333,7 +333,7 @@ static int create_gpadl_header(void *kbuffer, u32 size, * Gpadl is u32 and we are using a pointer which could * be 64-bit * This is governed by the guest/host protocol and -* so the hypervisor gurantees that this is ok. +* so the hypervisor guarantees that this is ok. */ for (i = 0; i < pfncurr; i++) gpadl_body->pfn[i] = slow_virt_to_phys( @@ -380,7 +380,7 @@ static int create_gpadl_header(void *kbuffer, u32 size, } /* - * vmbus_establish_gpadl - Estabish a GPADL for the specified buffer + * vmbus_establish_gpadl - Establish a GPADL for the specified buffer * * @channel: a channel * @kbuffer: from kmalloc or vmalloc @@ -732,7 +732,7 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, /* Setup the descriptor */ desc.type = VM_PKT_DATA_USING_GPA_DIRECT; desc.flags = flags; - desc.dataoffset8 = descsize >> 3; /* in 8-bytes grandularity */ + desc.dataoffset8 = descsize >> 3; /* in 8-bytes granularity */ desc.length8 = (u16)(packetlen_aligned >> 3); desc.transactionid = requestid; desc.rangecount = pagecount; @@ -793,7 +793,7 @@ int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel, /* Setup the descriptor */ desc->type = VM_PKT_DATA_USING_GPA_DIRECT; desc->flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED; - desc->dataoffset8 = desc_size >> 3; /* in 8-bytes grandularity */ + desc->dataoffset8 = desc_size >> 3; /* in 8-bytes granularity */ desc->length8 = (u16)(packetlen_aligned >> 3); desc->transactionid = requestid; desc->rangecount = 1; @@ -843,7 +843,7 @@ int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel, /* Setup the descriptor */ desc.type = VM_PKT_DATA_USING_GPA_DIRECT; desc.flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED; - desc.dataoffset8 = descsize >> 3; /* in 8-bytes grandularity */ + desc.dataoffset8 = descsize >> 3; /* in 8-bytes granularity */ desc.length8 = (u16)(packetlen_aligned >> 3); desc.transactionid = requestid; desc.rangecount = 1; diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index a65b7f88d7aa..b0a36e8a7268 100644 --- a/drivers/hv/hv_kvp.c +++ b/drivers/hv/hv_kvp.c @@ -69,7 +69,7 @@ static const int fw_versions[] = { * * While the request/response protocol is guaranteed by the host, we further * ensure this by serializing packet processing in this driver - we do not - * read additional packets from the VMBUs until the current packet is fully + * read additional packets from the VMBUS until the current packet is fully * handled. */ @@ -398,7 +398,7 @@ kvp_send_key(struct work_struct *dummy) * the max lengths specified. We will however, reserve room * for the string terminating character - in the utf16s_utf8s() * function we limit the size of the buffer where the converted -* string is placed to HV_KVP_EXCHANGE_MAX_*_SIZE -1 to gaurantee +* string is placed to HV_KVP_EXCHANGE_MAX_*_SIZE -1 to guarantee * that the strings can be properly terminated! */ @@ -532,7 +532,7 @@ kvp_respond_to_host(struct hv_kvp_msg *msg_to_host, int error) */ if (error) { /* -* Something failed or we have timedout; +* Something failed or we have timed out; * terminate the current host-side iteration. */ goto response_done; @@ -606,8 +606,8 @@ kvp_respond_to_host(struct hv_kvp_msg *msg_to_host, int error) * This callback is invoked when we get a KVP message from the host. * The host ensures that only one KVP transaction can be active at a time. * KVP implementation in Linux needs to forward the key to a user-mde - * component to retrive the corresponding value. Consequently, we cannot - * respond to the host in the conext of this callback. Since the host + * component to retrieve the corresponding value. Consequently, we cannot + * respond to the host in the context of this callback. Since the host * guarantees that at most only one transaction can be active at a time, * we stash away the transaction state in a set of global variables. */ -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/
[PATCH 10/10] vmbus: expose debug info for drivers
Allow driver to get debug information about state of the ring. Signed-off-by: Stephen Hemminger --- drivers/hv/hyperv_vmbus.h | 11 --- drivers/hv/ring_buffer.c | 1 + include/linux/hyperv.h| 17 + 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index a69b52de8d56..6113e915c50e 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -248,14 +248,6 @@ struct hv_context { extern struct hv_context hv_context; -struct hv_ring_buffer_debug_info { - u32 current_interrupt_mask; - u32 current_read_index; - u32 current_write_index; - u32 bytes_avail_toread; - u32 bytes_avail_towrite; -}; - /* Hv Interface */ extern int hv_init(void); @@ -289,9 +281,6 @@ int hv_ringbuffer_read(struct vmbus_channel *channel, void *buffer, u32 buflen, u32 *buffer_actual_len, u64 *requestid, bool raw); -void hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info, -struct hv_ring_buffer_debug_info *debug_info); - /* * Maximum channels is determined by the size of the interrupt page * which is PAGE_SIZE. 1/2 of PAGE_SIZE is for send endpoint interrupt diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index 8a249740b4b4..cfacca566e3f 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -206,6 +206,7 @@ void hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info, ring_info->ring_buffer->interrupt_mask; } } +EXPORT_SYMBOL_GPL(hv_ringbuffer_get_debuginfo); /* Initialize the ring buffer. */ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info, diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 9d5df04bb678..1f97f9325dae 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -491,6 +491,12 @@ struct vmbus_channel_rescind_offer { u32 child_relid; } __packed; +static inline u32 +hv_ringbuffer_pending_size(const struct hv_ring_buffer_info *rbi) +{ + return rbi->ring_buffer->pending_send_sz; +} + /* * Request Offer -- no parameters, SynIC message contains the partition ID * Set Snoop -- no parameters, SynIC message contains the partition ID @@ -1155,6 +1161,17 @@ static inline void *hv_get_drvdata(struct hv_device *dev) return dev_get_drvdata(&dev->device); } +struct hv_ring_buffer_debug_info { + u32 current_interrupt_mask; + u32 current_read_index; + u32 current_write_index; + u32 bytes_avail_toread; + u32 bytes_avail_towrite; +}; + +void hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info, + struct hv_ring_buffer_debug_info *debug_info); + /* Vmbus interface */ #define vmbus_driver_register(driver) \ __vmbus_driver_register(driver, THIS_MODULE, KBUILD_MODNAME) -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/3] x86/vdso: Add Hyper-V TSC page clocksource support
On Fri, 17 Feb 2017, Andy Lutomirski wrote: > On Fri, Feb 17, 2017 at 2:14 AM, Vitaly Kuznetsov wrote: > > Not only. As far as I understand (and I *think* K. Y. pointed this out) > > when VM is migrating to another host TSC page clocksource is disabled for > > extended period of time so we're better off reading from MSR than > > looping here. With regards to VDSO this means reverting to doing normal > > syscall. > > That's a crappy design. The guest really ought to be able to > distinguish "busy, try again" from "bail and use MSR". > > Thomas, I can see valid reasons why the hypervisor might want to > temporarily disable shared page-based timing, but I think it's silly > that it's conflated with indicating "retry". It's beyond silly if that's the case. > But if this is indeed the ABI, we're stuck with it. No argument. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev driver
On 02/17/2017 06:16 AM, Philipp Zabel wrote: On Fri, 2017-02-17 at 11:47 +0100, Philipp Zabel wrote: On Wed, 2017-02-15 at 18:19 -0800, Steve Longerbeam wrote: +static void csi2_dphy_init(struct csi2_dev *csi2) +{ + /* +* FIXME: 0x14 is derived from a fixed D-PHY reference +* clock from the HSI_TX PLL, and a fixed target lane max +* bandwidth of 300 Mbps. This value should be derived If the table in https://community.nxp.com/docs/DOC-94312 is correct, this should be 850 Mbps. Where does this 300 Mbps value come from? I got it, the dptdin_map value for 300 Mbps is 0x14 in the Rockchip DSI driver. But that value is written to the register as HSFREQRANGE_SEL(x): #define HSFREQRANGE_SEL(val)(((val) & 0x3f) << 1) Ah you are right, 0x14 would be a "testdin" value of 0x0a, which from the Rockchip table would be 950 MHz per lane. But thanks for pointing the table at https://community.nxp.com/docs/DOC-94312. That table is what should be referenced in the above comment (850 MHz per lane for a 27MHz reference clock). I will update the comment based on that table. Steve which is 0x28. Further, the Rockchip D-PHY probably is another version, as its max_mbps goes up to 1500. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: bcm2835-audio: bcm2835.h: fix various coding style issues
The following coding style issues (as per checkpatch.pl) were resolved. WARNING: Block comments use * on subsequent lines WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... WARNING: Missing a blank line after declarations WARNING: Use of volatile is usually wrong:... WARNING: line over 80 characters CHECK: Concatenated strings should use spaces between elements CHECK: Macro argument 'vol' may be better as '(vol)' to avoid precedence issues Signed-off-by: Nathan Howard --- drivers/staging/bcm2835-audio/bcm2835.h | 31 ++- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/drivers/staging/bcm2835-audio/bcm2835.h b/drivers/staging/bcm2835-audio/bcm2835.h index 36e3ef8..16d7006 100644 --- a/drivers/staging/bcm2835-audio/bcm2835.h +++ b/drivers/staging/bcm2835-audio/bcm2835.h @@ -27,8 +27,8 @@ #include /* -#define AUDIO_DEBUG_ENABLE -#define AUDIO_VERBOSE_DEBUG_ENABLE + * #define AUDIO_DEBUG_ENABLE + * #define AUDIO_VERBOSE_DEBUG_ENABLE */ /* Debug macros */ @@ -37,10 +37,10 @@ #ifdef AUDIO_VERBOSE_DEBUG_ENABLE #define audio_debug(fmt, arg...) \ - printk(KERN_INFO"%s:%d " fmt, __func__, __LINE__, ##arg) + pr_info("%s:%d " fmt, __func__, __LINE__, ##arg) #define audio_info(fmt, arg...) \ - printk(KERN_INFO"%s:%d " fmt, __func__, __LINE__, ##arg) + pr_info("%s:%d " fmt, __func__, __LINE__, ##arg) #else @@ -59,13 +59,13 @@ #endif /* AUDIO_DEBUG_ENABLE */ #define audio_error(fmt, arg...) \ - printk(KERN_ERR"%s:%d " fmt, __func__, __LINE__, ##arg) + pr_err("%s:%d " fmt, __func__, __LINE__, ##arg) #define audio_warning(fmt, arg...) \ - printk(KERN_WARNING"%s:%d " fmt, __func__, __LINE__, ##arg) + pr_warn("%s:%d " fmt, __func__, __LINE__, ##arg) #define audio_alert(fmt, arg...) \ - printk(KERN_ALERT"%s:%d " fmt, __func__, __LINE__, ##arg) + pr_alert("%s:%d " fmt, __func__, __LINE__, ##arg) #define MAX_SUBSTREAMS (8) #define AVAIL_SUBSTREAMS_MASK (0xff) @@ -77,8 +77,11 @@ enum { /* macros for alsa2chip and chip2alsa, instead of functions */ -#define alsa2chip(vol) (uint)(-((vol << 8) / 100)) /* convert alsa to chip volume (defined as macro rather than function call) */ -#define chip2alsa(vol) -((vol * 100) >> 8) /* convert chip to alsa volume */ +// convert alsa to chip volume (defined as macro rather than function call) +#define alsa2chip(vol) (uint)(-(((vol) << 8) / 100)) + +// convert chip to alsa volume +#define chip2alsa(vol) -(((vol) * 100) >> 8) /* Some constants for values .. */ enum snd_bcm2835_route { @@ -122,8 +125,8 @@ struct bcm2835_alsa_stream { struct semaphore buffers_update_sem; struct semaphore control_sem; spinlock_t lock; - volatile unsigned int control; - volatile unsigned int status; + unsigned int control; + unsigned int status; int open; int running; @@ -160,8 +163,10 @@ int bcm2835_audio_write(struct bcm2835_alsa_stream *alsa_stream, unsigned int count, void *src); void bcm2835_playback_fifo(struct bcm2835_alsa_stream *alsa_stream); -unsigned int bcm2835_audio_retrieve_buffers(struct bcm2835_alsa_stream *alsa_stream); +unsigned int bcm2835_audio_retrieve_buffers( + struct bcm2835_alsa_stream *alsa_stream); void bcm2835_audio_flush_buffers(struct bcm2835_alsa_stream *alsa_stream); -void bcm2835_audio_flush_playback_buffers(struct bcm2835_alsa_stream *alsa_stream); +void bcm2835_audio_flush_playback_buffers( + struct bcm2835_alsa_stream *alsa_stream); #endif /* __SOUND_ARM_BCM2835_H */ -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: bcm2835-audio: bcm2835.h: fix various coding style issues
On Fri, Feb 17, 2017 at 01:39:07PM -0500, Nathan Howard wrote: > The following coding style issues (as per checkpatch.pl) were resolved. > > WARNING: Block comments use * on subsequent lines > WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... > WARNING: Missing a blank line after declarations > WARNING: Use of volatile is usually wrong:... > WARNING: line over 80 characters > CHECK: Concatenated strings should use spaces between elements > CHECK: Macro argument 'vol' may be better as '(vol)' to avoid precedence > issues Please only do one thing per patch, and no, "fix style issues" is not one thing :( This should be a patch series, one per type of thing done. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] pci-hyperv: Use device serial number as PCI domain
On Mon, Feb 13, 2017 at 06:10:11PM +, Haiyang Zhang wrote: > > This allows PCI domain numbers starts with 1, and also unique > on the same VM. So names, such as VF NIC names, that include > domain number as part of the name, can be shorter than that > based on part of bus UUID previously. The new names will also > stay same for VMs created with copied VHD and same number of > devices. > > Signed-off-by: Haiyang Zhang > Reviewed-by: K. Y. Srinivasan Applied to pci/host-hv for v4.11, thanks! I assume Stephen meant a "Reviewed-by", not a "Signed-off-by", so that's what I added. > --- > drivers/pci/host/pci-hyperv.c | 10 ++ > 1 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 3efcc7b..b92b565 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1315,6 +1315,16 @@ static void put_pcichild(struct hv_pci_dev *hpdev, > get_pcichild(hpdev, hv_pcidev_ref_initial); > get_pcichild(hpdev, hv_pcidev_ref_childlist); > spin_lock_irqsave(&hbus->device_list_lock, flags); > + /* When a device is being added into the bus, we set the PCI domain > + * number to be the device serial number, which is non zero and > + * unique on the same VM. The serial numbers start with 1, and > + * increase by 1 for each device. So device names including this > + * can have shorter names than based on the bus instance UUID. > + * Only the first device serial number is used for domain, so the > + * domain number will not change after the first device is added. > + */ > + if (list_empty(&hbus->children)) > + hbus->sysdata.domain = desc->ser; > list_add_tail(&hpdev->list_entry, &hbus->children); > spin_unlock_irqrestore(&hbus->device_list_lock, flags); > return hpdev; > -- > 1.7.1 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/5] staging: bcm2835-audio: bcm2835.h: fix block comment warning
Fix checkpatch.pl warning: "Block comments use * on subsequent lines." Signed-off-by: Nathan Howard --- drivers/staging/bcm2835-audio/bcm2835.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/bcm2835-audio/bcm2835.h b/drivers/staging/bcm2835-audio/bcm2835.h index 36e3ef8..66af445 100644 --- a/drivers/staging/bcm2835-audio/bcm2835.h +++ b/drivers/staging/bcm2835-audio/bcm2835.h @@ -27,8 +27,8 @@ #include /* -#define AUDIO_DEBUG_ENABLE -#define AUDIO_VERBOSE_DEBUG_ENABLE + * #define AUDIO_DEBUG_ENABLE + * #define AUDIO_VERBOSE_DEBUG_ENABLE */ /* Debug macros */ -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/5] staging: bcm2835-audio: bcm2835.h: fix printk coding style issue
Fix checkpatch.pl warning of the form "WARNING: Prefer subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ..." Signed-off-by: Nathan Howard --- drivers/staging/bcm2835-audio/bcm2835.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/bcm2835-audio/bcm2835.h b/drivers/staging/bcm2835-audio/bcm2835.h index 66af445..81fdb10 100644 --- a/drivers/staging/bcm2835-audio/bcm2835.h +++ b/drivers/staging/bcm2835-audio/bcm2835.h @@ -37,10 +37,10 @@ #ifdef AUDIO_VERBOSE_DEBUG_ENABLE #define audio_debug(fmt, arg...) \ - printk(KERN_INFO"%s:%d " fmt, __func__, __LINE__, ##arg) + pr_info("%s:%d " fmt, __func__, __LINE__, ##arg) #define audio_info(fmt, arg...) \ - printk(KERN_INFO"%s:%d " fmt, __func__, __LINE__, ##arg) + pr_info("%s:%d " fmt, __func__, __LINE__, ##arg) #else @@ -59,13 +59,13 @@ #endif /* AUDIO_DEBUG_ENABLE */ #define audio_error(fmt, arg...) \ - printk(KERN_ERR"%s:%d " fmt, __func__, __LINE__, ##arg) + pr_err("%s:%d " fmt, __func__, __LINE__, ##arg) #define audio_warning(fmt, arg...) \ - printk(KERN_WARNING"%s:%d " fmt, __func__, __LINE__, ##arg) + pr_warn("%s:%d " fmt, __func__, __LINE__, ##arg) #define audio_alert(fmt, arg...) \ - printk(KERN_ALERT"%s:%d " fmt, __func__, __LINE__, ##arg) + pr_alert("%s:%d " fmt, __func__, __LINE__, ##arg) #define MAX_SUBSTREAMS (8) #define AVAIL_SUBSTREAMS_MASK (0xff) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/5] staging: bcm2835-audio: bcm2835.h: fix macro coding style issue
Fix checkpatch.pl warning of the form "CHECK: Macro argument 'vol' may be better as '(vol)' to avoid precedence issues." Signed-off-by: Nathan Howard --- drivers/staging/bcm2835-audio/bcm2835.h | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/staging/bcm2835-audio/bcm2835.h b/drivers/staging/bcm2835-audio/bcm2835.h index 81fdb10..2f9d1c9 100644 --- a/drivers/staging/bcm2835-audio/bcm2835.h +++ b/drivers/staging/bcm2835-audio/bcm2835.h @@ -77,8 +77,11 @@ enum { /* macros for alsa2chip and chip2alsa, instead of functions */ -#define alsa2chip(vol) (uint)(-((vol << 8) / 100)) /* convert alsa to chip volume (defined as macro rather than function call) */ -#define chip2alsa(vol) -((vol * 100) >> 8) /* convert chip to alsa volume */ +// convert alsa to chip volume (defined as macro rather than function call) +#define alsa2chip(vol) (uint)(-(((vol) << 8) / 100)) + +// convert chip to alsa volume +#define chip2alsa(vol) -(((vol) * 100) >> 8) /* Some constants for values .. */ enum snd_bcm2835_route { -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/5] staging: bcm2835-audio: bcm2835.h: fix line length coding style issue
Fix checkpatch.pl warning of the form "WARNING: line over 80 characters." Signed-off-by: Nathan Howard --- drivers/staging/bcm2835-audio/bcm2835.h | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/staging/bcm2835-audio/bcm2835.h b/drivers/staging/bcm2835-audio/bcm2835.h index 08f7ad6..16d7006 100644 --- a/drivers/staging/bcm2835-audio/bcm2835.h +++ b/drivers/staging/bcm2835-audio/bcm2835.h @@ -163,8 +163,10 @@ int bcm2835_audio_write(struct bcm2835_alsa_stream *alsa_stream, unsigned int count, void *src); void bcm2835_playback_fifo(struct bcm2835_alsa_stream *alsa_stream); -unsigned int bcm2835_audio_retrieve_buffers(struct bcm2835_alsa_stream *alsa_stream); +unsigned int bcm2835_audio_retrieve_buffers( + struct bcm2835_alsa_stream *alsa_stream); void bcm2835_audio_flush_buffers(struct bcm2835_alsa_stream *alsa_stream); -void bcm2835_audio_flush_playback_buffers(struct bcm2835_alsa_stream *alsa_stream); +void bcm2835_audio_flush_playback_buffers( + struct bcm2835_alsa_stream *alsa_stream); #endif /* __SOUND_ARM_BCM2835_H */ -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/5] staging: bcm2835-audio: bcm2835.h: fix volatile coding style issue
Fix checkpatch.pl warning of the form "WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst." Signed-off-by: Nathan Howard --- drivers/staging/bcm2835-audio/bcm2835.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/bcm2835-audio/bcm2835.h b/drivers/staging/bcm2835-audio/bcm2835.h index 2f9d1c9..08f7ad6 100644 --- a/drivers/staging/bcm2835-audio/bcm2835.h +++ b/drivers/staging/bcm2835-audio/bcm2835.h @@ -125,8 +125,8 @@ struct bcm2835_alsa_stream { struct semaphore buffers_update_sem; struct semaphore control_sem; spinlock_t lock; - volatile unsigned int control; - volatile unsigned int status; + unsigned int control; + unsigned int status; int open; int running; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: bcm2835-audio: bcm2835.h: fix various coding style issues
On Fri, 2017-02-17 at 13:39 -0500, Nathan Howard wrote: > The following coding style issues (as per checkpatch.pl) were resolved. What Greg said is true, and the volatile conversion especially needs to be verified. > diff --git a/drivers/staging/bcm2835-audio/bcm2835.h > b/drivers/staging/bcm2835-audio/bcm2835.h [] > @@ -27,8 +27,8 @@ > #include > > /* > -#define AUDIO_DEBUG_ENABLE > -#define AUDIO_VERBOSE_DEBUG_ENABLE > + * #define AUDIO_DEBUG_ENABLE > + * #define AUDIO_VERBOSE_DEBUG_ENABLE > */ Using #define DEBUG would be more common. > /* Debug macros */ > @@ -37,10 +37,10 @@ > #ifdef AUDIO_VERBOSE_DEBUG_ENABLE > > #define audio_debug(fmt, arg...) \ > - printk(KERN_INFO"%s:%d " fmt, __func__, __LINE__, ##arg) > + pr_info("%s:%d " fmt, __func__, __LINE__, ##arg) > > #define audio_info(fmt, arg...) \ > - printk(KERN_INFO"%s:%d " fmt, __func__, __LINE__, ##arg) > + pr_info("%s:%d " fmt, __func__, __LINE__, ##arg) > > #else > > @@ -59,13 +59,13 @@ > #endif /* AUDIO_DEBUG_ENABLE */ > > #define audio_error(fmt, arg...) \ > - printk(KERN_ERR"%s:%d " fmt, __func__, __LINE__, ##arg) > + pr_err("%s:%d " fmt, __func__, __LINE__, ##arg) > > #define audio_warning(fmt, arg...) \ > - printk(KERN_WARNING"%s:%d " fmt, __func__, __LINE__, ##arg) > + pr_warn("%s:%d " fmt, __func__, __LINE__, ##arg) > > #define audio_alert(fmt, arg...) \ > - printk(KERN_ALERT"%s:%d " fmt, __func__, __LINE__, ##arg) > + pr_alert("%s:%d " fmt, __func__, __LINE__, ##arg) These might as well be removed and converted to the direct pr_ equivalents and have #define pr_fmt(fmt) "%s:%d: " fmt, __func__, __LINE__ added before any include, but honestly the __func__ and __LINE__ aren't particularly useful. > @@ -122,8 +125,8 @@ struct bcm2835_alsa_stream { > struct semaphore buffers_update_sem; > struct semaphore control_sem; > spinlock_t lock; > - volatile unsigned int control; > - volatile unsigned int status; > + unsigned int control; > + unsigned int status; Unless you can absolutely verify that that doesn't change hardware access, you should leave this alone. ' ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: ks7010: ks_*: Braces should be used on all arms of these statements
Braces should be used on all arms of these statements (CHECK).. Signed-off-by: Shiva Kerdel --- drivers/staging/ks7010/ks_hostif.c | 6 -- drivers/staging/ks7010/ks_wlan_net.c | 42 +++- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c index 97d7b56c592a..fad1cf5361b0 100644 --- a/drivers/staging/ks7010/ks_hostif.c +++ b/drivers/staging/ks7010/ks_hostif.c @@ -2148,8 +2148,9 @@ void hostif_sme_mode_setup(struct ks_wlan_private *priv) else rate_octet[i] = priv->reg.rate_set.body[i]; - } else + } else { break; + } } } else {/* D_11G_ONLY_MODE or D_11BG_COMPATIBLE_MODE */ @@ -2163,8 +2164,9 @@ void hostif_sme_mode_setup(struct ks_wlan_private *priv) else rate_octet[i] = priv->reg.rate_set.body[i]; - } else + } else { break; + } } } rate_size = i; diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c index 121e1530fdba..9cec19f1babc 100644 --- a/drivers/staging/ks7010/ks_wlan_net.c +++ b/drivers/staging/ks7010/ks_wlan_net.c @@ -225,9 +225,9 @@ static int ks_wlan_set_freq(struct net_device *dev, fwrq->m = c + 1; } /* Setting by channel number */ - if ((fwrq->m > 1000) || (fwrq->e > 0)) + if ((fwrq->m > 1000) || (fwrq->e > 0)) { rc = -EOPNOTSUPP; - else { + } else { int channel = fwrq->m; /* We should do a better check than that, * based on the card capability !!! */ @@ -977,8 +977,9 @@ static int ks_wlan_set_encode(struct net_device *dev, if (priv->reg.wep_key[index].size) { priv->reg.wep_index = index; priv->need_commit |= SME_WEP_INDEX; - } else + } else { return -EINVAL; + } } } } @@ -1054,8 +1055,9 @@ static int ks_wlan_get_encode(struct net_device *dev, if ((index >= 0) && (index < 4)) memcpy(extra, priv->reg.wep_key[index].val, dwrq->length); - } else + } else { memcpy(extra, zeros, dwrq->length); + } #endif return 0; } @@ -1272,8 +1274,9 @@ static int ks_wlan_set_power(struct net_device *dev, priv->reg.powermgt = POWMGT_SAVE2_MODE; else return -EINVAL; - } else + } else { return -EINVAL; + } hostif_sme_enqueue(priv, SME_POW_MNGMT_REQUEST); @@ -1942,8 +1945,9 @@ static int ks_wlan_set_encode_ext(struct net_device *dev, return -EINVAL; } priv->wpa.key[index].alg = enc->alg; - } else + } else { return -EINVAL; + } if (commit) { if (commit & SME_WEP_INDEX) @@ -2201,8 +2205,9 @@ static int ks_wlan_set_detach(struct net_device *dev, } else if (*uwrq == DISCONNECT_STATUS) {/* 1 */ priv->connect_status |= FORCE_DISCONNECT; netif_carrier_off(dev); - } else + } else { return -EINVAL; + } return 0; } @@ -2256,8 +2261,9 @@ static int ks_wlan_set_preamble(struct net_device *dev, priv->reg.preamble = LONG_PREAMBLE; } else if (*uwrq == SHORT_PREAMBLE) { /* 1 */ priv->reg.preamble = SHORT_PREAMBLE; - } else + } else { return -EINVAL; + } priv->need_commit |= SME_MODE_SET; return -EINPROGRESS;/* Call commit handler */ @@ -2305,8 +2311,9 @@ static int ks_wlan_set_powermgt(struct net_device *dev, priv->reg.powermgt = POWMGT_SAVE2_MODE; else return -EINVAL; - } else + } else { return -EINVAL; + } hostif_sme_enqueue(priv, SME_POW_MNGMT_REQUEST); @@ -2346,8 +2353,9 @@ static int ks_wlan_set_scan_type(struct net_device *dev, priv->reg.scan_type = ACTIVE_SCAN; } else if (*uwrq == PASSIVE_SCAN) { /* 1 */ priv->reg.scan_type = PASSIVE_SCAN; - } else + } else { return -EINVAL; +
Re: [PATCH] [media] Staging: media/lirc: don't call put_ir_rx on rx twice
This one is a false positive. The original code is correct. I was looking through my mail boxes to see the history of this and why it hadn't been fixed earlier. Someone tried to fix it in 2011: https://www.spinics.net/lists/linux-driver-devel/msg17403.html Then I complained about it again in 2014 when I was looking at a different bug in that same function. Now you're the third person to think this code is suspicious. I think part of the problem is that get_ir_rx(ir) is hidden as a function parameter instead of on its own line. But really even that wouldn't totally fix the issue. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch] staging: bcm2835/mmal-vchiq: unlock on error in buffer_from_host()
We should unlock before returning on this error path. Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera driver.") Signed-off-by: Dan Carpenter diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c index f0639ee6c8b9..fdfb6a620a43 100644 --- a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c +++ b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c @@ -397,8 +397,10 @@ buffer_from_host(struct vchiq_mmal_instance *instance, /* get context */ msg_context = get_msg_context(instance); - if (msg_context == NULL) - return -ENOMEM; + if (!msg_context) { + ret = -ENOMEM; + goto unlock; + } /* store bulk message context for when data arrives */ msg_context->u.bulk.instance = instance; @@ -454,6 +456,7 @@ buffer_from_host(struct vchiq_mmal_instance *instance, vchi_service_release(instance->handle); +unlock: mutex_unlock(&instance->bulk_mutex); return ret; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch v2] staging: bcm2835-camera: fix error handling in init
The unwinding here isn't right. We don't free gdev[0] and instead free 1 step past what was allocated. Also we can't allocate "dev" then we should unwind instead of returning directly. Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera driver.") Signed-off-by: Dan Carpenter --- v2: Change the style to make Walter Harms happy. Fix some additional bugs I missed in the first patch. diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c index ca15a698e018..c4dad30dd133 100644 --- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c +++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c @@ -1901,6 +1901,7 @@ static int __init bm2835_mmal_init(void) unsigned int num_cameras; struct vchiq_mmal_instance *instance; unsigned int resolutions[MAX_BCM2835_CAMERAS][2]; + int i; ret = vchiq_mmal_init(&instance); if (ret < 0) @@ -1914,8 +1915,10 @@ static int __init bm2835_mmal_init(void) for (camera = 0; camera < num_cameras; camera++) { dev = kzalloc(sizeof(struct bm2835_mmal_dev), GFP_KERNEL); - if (!dev) - return -ENOMEM; + if (!dev) { + ret = -ENOMEM; + goto cleanup_gdev; + } dev->camera_num = camera; dev->max_width = resolutions[camera][0]; @@ -1998,9 +2001,10 @@ static int __init bm2835_mmal_init(void) free_dev: kfree(dev); - for ( ; camera > 0; camera--) { - bcm2835_cleanup_instance(gdev[camera]); - gdev[camera] = NULL; +cleanup_gdev: + for (i = 0; i < camera; i++) { + bcm2835_cleanup_instance(gdev[i]); + gdev[i] = NULL; } pr_info("%s: error %d while loading driver\n", BM2835_MMAL_MODULE_NAME, ret); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vc04_services: fix missing check of return value for query of dt_node property
This appears to be an ancient issue with the old github.com sources. If the cache-line-size property is missing, then the driver probe should fail as no dev since the kernel and dt may be out of sync. The fix is to add a check for the return value of of_property_read_u32. Signed-off-by: Michael Zoran --- drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c index e6241fb5cfa6..1f800f833fbe 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c @@ -121,8 +121,12 @@ int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state) if (err < 0) return err; - (void)of_property_read_u32(dev->of_node, "cache-line-size", + err = of_property_read_u32(dev->of_node, "cache-line-size", &g_cache_line_size); + + if (err < 0) + return -ENODEV; + g_fragments_size = 2 * g_cache_line_size; /* Allocate space for the channels in coherent memory */ -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/5] staging: bcm2835-audio: bcm2835.h: fix volatile coding style issue
Thank you, Joe. Please remove this patch at this time; it was sent in error. On Fri, Feb 17, 2017 at 6:04 PM, Joe Perches wrote: > On Fri, 2017-02-17 at 15:16 -0500, Nathan Howard wrote: >> Fix checkpatch.pl warning of the form "WARNING: Use of volatile is >> usually wrong: see Documentation/process/volatile-considered-harmful.rst." > > Why are you sure the volatile use is not necessary? > >> Signed-off-by: Nathan Howard >> --- >> drivers/staging/bcm2835-audio/bcm2835.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/staging/bcm2835-audio/bcm2835.h >> b/drivers/staging/bcm2835-audio/bcm2835.h >> index 2f9d1c9..08f7ad6 100644 >> --- a/drivers/staging/bcm2835-audio/bcm2835.h >> +++ b/drivers/staging/bcm2835-audio/bcm2835.h >> @@ -125,8 +125,8 @@ struct bcm2835_alsa_stream { >> struct semaphore buffers_update_sem; >> struct semaphore control_sem; >> spinlock_t lock; >> - volatile unsigned int control; >> - volatile unsigned int status; >> + unsigned int control; >> + unsigned int status; >> >> int open; >> int running; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates
On 02/15/2017 06:19 PM, Steve Longerbeam wrote: From: Russell King Setting and getting frame rates is part of the negotiation mechanism between subdevs. The lack of support means that a frame rate at the sensor can't be negotiated through the subdev path. Add support at MIPI CSI2 level for handling this part of the negotiation. Signed-off-by: Russell King Signed-off-by: Steve Longerbeam Hi Russell, I signed-off on this but after more review I'm not sure this is right. The CSI-2 receiver really has no control over frame rate. It's output frame rate is the same as the rate that is delivered to it. So this subdev should either not implement these ops, or it should refer them to the attached source subdev. Steve --- drivers/staging/media/imx/imx6-mipi-csi2.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c index 23dca80..c62f14e 100644 --- a/drivers/staging/media/imx/imx6-mipi-csi2.c +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c @@ -34,6 +34,7 @@ struct csi2_dev { struct v4l2_subdev sd; struct media_pad pad[CSI2_NUM_PADS]; struct v4l2_mbus_framefmt format_mbus; + struct v4l2_fract frame_interval; struct clk *dphy_clk; struct clk *cfg_clk; struct clk *pix_clk; /* what is this? */ @@ -397,6 +398,30 @@ static int csi2_set_fmt(struct v4l2_subdev *sd, return 0; } +static int csi2_g_frame_interval(struct v4l2_subdev *sd, +struct v4l2_subdev_frame_interval *fi) +{ + struct csi2_dev *csi2 = sd_to_dev(sd); + + fi->interval = csi2->frame_interval; + + return 0; +} + +static int csi2_s_frame_interval(struct v4l2_subdev *sd, +struct v4l2_subdev_frame_interval *fi) +{ + struct csi2_dev *csi2 = sd_to_dev(sd); + + /* Output pads mirror active input pad, no limits on input pads */ + if (fi->pad != CSI2_SINK_PAD) + fi->interval = csi2->frame_interval; + + csi2->frame_interval = fi->interval; + + return 0; +} + /* * retrieve our pads parsed from the OF graph by the media device */ @@ -430,6 +455,8 @@ static struct v4l2_subdev_core_ops csi2_core_ops = { static struct v4l2_subdev_video_ops csi2_video_ops = { .s_stream = csi2_s_stream, + .g_frame_interval = csi2_g_frame_interval, + .s_frame_interval = csi2_s_frame_interval, }; static struct v4l2_subdev_pad_ops csi2_pad_ops = { -- Steve Longerbeam | Senior Embedded Engineer, ESD Services Mentor Embedded(tm) | 46871 Bayside Parkway, Fremont, CA 94538 P 510.354.5838 | M 408.410.2735 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates
On 02/15/2017 06:19 PM, Steve Longerbeam wrote: From: Russell King Setting and getting frame rates is part of the negotiation mechanism between subdevs. The lack of support means that a frame rate at the sensor can't be negotiated through the subdev path. Add support at MIPI CSI2 level for handling this part of the negotiation. Signed-off-by: Russell King Signed-off-by: Steve Longerbeam Hi Russell, I signed-off on this but after more review I'm not sure this is right. The CSI-2 receiver really has no control over frame rate. It's output frame rate is the same as the rate that is delivered to it. So this subdev should either not implement these ops, or it should refer them to the attached source subdev. Steve --- drivers/staging/media/imx/imx6-mipi-csi2.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c index 23dca80..c62f14e 100644 --- a/drivers/staging/media/imx/imx6-mipi-csi2.c +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c @@ -34,6 +34,7 @@ struct csi2_dev { struct v4l2_subdev sd; struct media_pad pad[CSI2_NUM_PADS]; struct v4l2_mbus_framefmt format_mbus; + struct v4l2_fract frame_interval; struct clk *dphy_clk; struct clk *cfg_clk; struct clk *pix_clk; /* what is this? */ @@ -397,6 +398,30 @@ static int csi2_set_fmt(struct v4l2_subdev *sd, return 0; } +static int csi2_g_frame_interval(struct v4l2_subdev *sd, +struct v4l2_subdev_frame_interval *fi) +{ + struct csi2_dev *csi2 = sd_to_dev(sd); + + fi->interval = csi2->frame_interval; + + return 0; +} + +static int csi2_s_frame_interval(struct v4l2_subdev *sd, +struct v4l2_subdev_frame_interval *fi) +{ + struct csi2_dev *csi2 = sd_to_dev(sd); + + /* Output pads mirror active input pad, no limits on input pads */ + if (fi->pad != CSI2_SINK_PAD) + fi->interval = csi2->frame_interval; + + csi2->frame_interval = fi->interval; + + return 0; +} + /* * retrieve our pads parsed from the OF graph by the media device */ @@ -430,6 +455,8 @@ static struct v4l2_subdev_core_ops csi2_core_ops = { static struct v4l2_subdev_video_ops csi2_video_ops = { .s_stream = csi2_s_stream, + .g_frame_interval = csi2_g_frame_interval, + .s_frame_interval = csi2_s_frame_interval, }; static struct v4l2_subdev_pad_ops csi2_pad_ops = { -- Steve Longerbeam | Senior Embedded Engineer, ESD Services Mentor Embedded(tm) | 46871 Bayside Parkway, Fremont, CA 94538 P 510.354.5838 | M 408.410.2735 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] staging: bcm2835-audio: bcm2835.h: fix line length coding style issue
On Fri, 2017-02-17 at 15:16 -0500, Nathan Howard wrote: > Fix checkpatch.pl warning of the form "WARNING: line over 80 characters." [] > diff --git a/drivers/staging/bcm2835-audio/bcm2835.h > b/drivers/staging/bcm2835-audio/bcm2835.h [] > @@ -163,8 +163,10 @@ int bcm2835_audio_write(struct bcm2835_alsa_stream > *alsa_stream, > unsigned int count, > void *src); > void bcm2835_playback_fifo(struct bcm2835_alsa_stream *alsa_stream); > -unsigned int bcm2835_audio_retrieve_buffers(struct bcm2835_alsa_stream > *alsa_stream); > +unsigned int bcm2835_audio_retrieve_buffers( > + struct bcm2835_alsa_stream *alsa_stream); This is not a good change. This line exceeds 80 columns only because it uses very long identifiers (30+ chars). Anything that uses these very long names is going to be silly looking when forced to use 80 column line length maximums. Basically, it's OK as it is and if you really want to change it for any reason the other style to use is to have the return value on a separate line like: unsigned int bcm2836_audio_retrieve_buffers(struct bcm2835_also_stream *alsa_stream); Even so, that's not a good change either. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/5] staging: bcm2835-audio: bcm2835.h: fix volatile coding style issue
On Fri, 2017-02-17 at 15:16 -0500, Nathan Howard wrote: > Fix checkpatch.pl warning of the form "WARNING: Use of volatile is > usually wrong: see Documentation/process/volatile-considered-harmful.rst." Why are you sure the volatile use is not necessary? > Signed-off-by: Nathan Howard > --- > drivers/staging/bcm2835-audio/bcm2835.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/bcm2835-audio/bcm2835.h > b/drivers/staging/bcm2835-audio/bcm2835.h > index 2f9d1c9..08f7ad6 100644 > --- a/drivers/staging/bcm2835-audio/bcm2835.h > +++ b/drivers/staging/bcm2835-audio/bcm2835.h > @@ -125,8 +125,8 @@ struct bcm2835_alsa_stream { > struct semaphore buffers_update_sem; > struct semaphore control_sem; > spinlock_t lock; > - volatile unsigned int control; > - volatile unsigned int status; > + unsigned int control; > + unsigned int status; > > int open; > int running; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] staging: bcm2835-audio: bcm2835.h: fix line length coding style issue
Thanks, Joe. Is this to say that scripts/checkpatch.pl should be updated to some higher column limit? I have made these cleanup changes before in a like manner. On Fri, Feb 17, 2017 at 8:17 PM, Joe Perches wrote: > On Fri, 2017-02-17 at 15:16 -0500, Nathan Howard wrote: >> Fix checkpatch.pl warning of the form "WARNING: line over 80 characters." > [] >> diff --git a/drivers/staging/bcm2835-audio/bcm2835.h >> b/drivers/staging/bcm2835-audio/bcm2835.h > [] >> @@ -163,8 +163,10 @@ int bcm2835_audio_write(struct bcm2835_alsa_stream >> *alsa_stream, >> unsigned int count, >> void *src); >> void bcm2835_playback_fifo(struct bcm2835_alsa_stream *alsa_stream); >> -unsigned int bcm2835_audio_retrieve_buffers(struct bcm2835_alsa_stream >> *alsa_stream); >> +unsigned int bcm2835_audio_retrieve_buffers( >> + struct bcm2835_alsa_stream *alsa_stream); > > This is not a good change. > > This line exceeds 80 columns only because > it uses very long identifiers (30+ chars). > > Anything that uses these very long names > is going to be silly looking when forced > to use 80 column line length maximums. > > Basically, it's OK as it is and if you > really want to change it for any reason > the other style to use is to have the > return value on a separate line like: > > unsigned int > bcm2836_audio_retrieve_buffers(struct bcm2835_also_stream *alsa_stream); > > Even so, that's not a good change either. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] staging: bcm2835-audio: bcm2835.h: fix line length coding style issue
On Fri, 2017-02-17 at 20:32 -0500, Adan Hawthorn wrote: > Thanks, Joe. > > Is this to say that scripts/checkpatch.pl should be updated to some > higher column limit? I have made these cleanup changes before in a > like manner. Hard to say. There could be some sensitivity to long identifier name lengths added to checkpatch, but < 80 column line length is still currently "strongly preferred". I don't care much one way or another if it's 80 or 100 or something else as long as it's context appropriate. Awhile ago, Linus Torvalds wrote: On Thu, 2016-12-15 at 18:10 -0800, Linus Torvalds wrote: > On Thu, Dec 15, 2016 at 5:57 PM, Joe Perches wrote: > > > > > > In fact, I thought we already upped the check-patch limit to 100? > > > > Nope, CodingStyle neither. > > > > Last time I tried was awhile ago. > > Ok, it must have been just talked about, and with the exceptions for > strings etc I may not have seen as many of the really annoying line > breaks lately. > > I don't mind a 80-column "soft limit" per se: if some code > consistently goes over 80 columns, there really is something seriously > wrong there. So 80 columns may well be the right limit for that kind > of check (or even less). > > But if we have just a couple of lines that are longer (in a file that > is 3k+ lines), I'd rather not break those. > > I tend use "git grep" a lot, and it's much easier to see function > argument use if it's all on one line. > > Of course, some function calls really are *so* long that they have to > be broken up, but that's where the "if it's a couple of lines that go > a bit over the 80 column limit..." exception basically comes in. > > Put another way: long lines definitely aren't good. But breaking long > lines has some downsides too, so there should be a balance between the > two, rather than some black-and-white limit. > > In fact, we've seldom had cases where black-and-white limits work well. > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] staging: bcm2835-audio: bcm2835.h: fix line length coding style issue
Ok, I personally like that better anyhow. Also, there was mention of the line limit even exceeding 100 characters (130ish?) and that was being tossed around at one point (if I remember correctly). On Fri, Feb 17, 2017 at 8:38 PM, Joe Perches wrote: > On Fri, 2017-02-17 at 20:32 -0500, Adan Hawthorn wrote: >> Thanks, Joe. >> >> Is this to say that scripts/checkpatch.pl should be updated to some >> higher column limit? I have made these cleanup changes before in a >> like manner. > > Hard to say. > > There could be some sensitivity to long identifier > name lengths added to checkpatch, but < 80 column > line length is still currently "strongly preferred". > > I don't care much one way or another if it's 80 > or 100 or something else as long as it's context > appropriate. > > Awhile ago, Linus Torvalds wrote: > > On Thu, 2016-12-15 at 18:10 -0800, Linus Torvalds wrote: >> On Thu, Dec 15, 2016 at 5:57 PM, Joe Perches wrote: >> > > >> > > In fact, I thought we already upped the check-patch limit to 100? >> > >> > Nope, CodingStyle neither. >> > >> > Last time I tried was awhile ago. >> >> Ok, it must have been just talked about, and with the exceptions for >> strings etc I may not have seen as many of the really annoying line >> breaks lately. >> >> I don't mind a 80-column "soft limit" per se: if some code >> consistently goes over 80 columns, there really is something seriously >> wrong there. So 80 columns may well be the right limit for that kind >> of check (or even less). >> >> But if we have just a couple of lines that are longer (in a file that >> is 3k+ lines), I'd rather not break those. >> >> I tend use "git grep" a lot, and it's much easier to see function >> argument use if it's all on one line. >> >> Of course, some function calls really are *so* long that they have to >> be broken up, but that's where the "if it's a couple of lines that go >> a bit over the 80 column limit..." exception basically comes in. >> >> Put another way: long lines definitely aren't good. But breaking long >> lines has some downsides too, so there should be a balance between the >> two, rather than some black-and-white limit. >> >> In fact, we've seldom had cases where black-and-white limits work well. >> ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: lustre: ko2iblnd: Adapt to the removal of ib_get_dma_mr()
In Linux kernel 4.9-rc1, the function ib_get_dma_mr() was removed and a second parameter was added to ib_alloc_pd(). As this broke the building of the ko2iblnd module in staging, the Kconfig for LNet has marked ko2iblnd as broken and stopped building it. This patch fixes this breakage by: - Removing the BROKEN tag from lnet/Kconfig. - Make it so the module parameter map_on_demand can no longer be zero (we have to configure FMR/FastReg pools; it can no longer be off). - No longer try to use the global DMA memory region, but make use of the FMR/FastReg pool for all RDMA Tx operations. - Everywhere we are using the device DMA mr to derive the L-key for non-registered memory regions, use the pd->local_dma_lkey value instead. - Make the default map_on_demand = 256. This will allow nodes with this patch to still connected to older nodes without this patch and FMR/FastReg turned off. When FMR/FastReg is turned off, we use 256 as the max frags so the two sides will still be able to communicate and work. - Fix a mistake with BUILD_BUG_ON calls in o2iblnd.c which caused compiling to fail. Signed-off-by: Doug Oucharek Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-9026 Reviewed-on: https://review.whamcloud.com/#/c/24931/ Reviewed-by: James Simmons Changelog: v1) Initial patch v2) Rebased and handle a fix to BUILD_BUG_ON --- drivers/staging/lustre/lnet/Kconfig| 1 - .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c| 77 ++ .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h| 3 - .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 17 + .../lustre/lnet/klnds/o2iblnd/o2iblnd_modparams.c | 12 ++-- 5 files changed, 16 insertions(+), 94 deletions(-) diff --git a/drivers/staging/lustre/lnet/Kconfig b/drivers/staging/lustre/lnet/Kconfig index 13b4327..2b59301 100644 --- a/drivers/staging/lustre/lnet/Kconfig +++ b/drivers/staging/lustre/lnet/Kconfig @@ -35,7 +35,6 @@ config LNET_SELFTEST config LNET_XPRT_IB tristate "LNET infiniband support" depends on LNET && INFINIBAND && INFINIBAND_ADDR_TRANS - depends on BROKEN default LNET && INFINIBAND help This option allows the LNET users to use infiniband as an diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c index b1e8508..0618b79 100644 --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c @@ -1281,27 +1281,6 @@ static void kiblnd_map_tx_pool(struct kib_tx_pool *tpo) } } -struct ib_mr *kiblnd_find_rd_dma_mr(struct lnet_ni *ni, struct kib_rdma_desc *rd, - int negotiated_nfrags) -{ - struct kib_net *net = ni->ni_data; - struct kib_hca_dev *hdev = net->ibn_dev->ibd_hdev; - struct lnet_ioctl_config_o2iblnd_tunables *tunables; - __u16 nfrags; - int mod; - - tunables = &ni->ni_lnd_tunables->lt_tun_u.lt_o2ib; - mod = tunables->lnd_map_on_demand; - nfrags = (negotiated_nfrags != -1) ? negotiated_nfrags : mod; - - LASSERT(hdev->ibh_mrs); - - if (mod > 0 && nfrags <= rd->rd_nfrags) - return NULL; - - return hdev->ibh_mrs; -} - static void kiblnd_destroy_fmr_pool(struct kib_fmr_pool *fpo) { LASSERT(!fpo->fpo_map_count); @@ -2168,21 +2147,12 @@ static int kiblnd_net_init_pools(struct kib_net *net, lnet_ni_t *ni, __u32 *cpts int ncpts) { struct lnet_ioctl_config_o2iblnd_tunables *tunables; - unsigned long flags; int cpt; int rc; int i; tunables = &ni->ni_lnd_tunables->lt_tun_u.lt_o2ib; - read_lock_irqsave(&kiblnd_data.kib_global_lock, flags); - if (!tunables->lnd_map_on_demand) { - read_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags); - goto create_tx_pool; - } - - read_unlock_irqrestore(&kiblnd_data.kib_global_lock, flags); - if (tunables->lnd_fmr_pool_size < *kiblnd_tunables.kib_ntx / 4) { CERROR("Can't set fmr pool size (%d) < ntx / 4(%d)\n", tunables->lnd_fmr_pool_size, @@ -2227,7 +2197,6 @@ static int kiblnd_net_init_pools(struct kib_net *net, lnet_ni_t *ni, __u32 *cpts if (i > 0) LASSERT(i == ncpts); - create_tx_pool: /* * cfs_precpt_alloc is creating an array of struct kib_tx_poolset * The number of struct kib_tx_poolsets create is equal to the @@ -2283,20 +2252,8 @@ static int kiblnd_hdev_get_attr(struct kib_hca_dev *hdev) return -EINVAL; } -static void kiblnd_hdev_cleanup_mrs(struct kib_hca_dev *hdev) -{ - if (!hdev->ibh_mrs) - return; - - ib_dereg_mr(hdev->ibh_mrs); - - hdev->ibh_mrs = NULL; -} - void kiblnd_hdev_destroy(struct kib_hca_dev *hdev) { - kiblnd_hdev_cleanup_mrs(hdev); - if (hdev->i
Re: [PATCH] Staging: ks7010: ks_*: Braces should be used on all arms of these statements
On Fri, 2017-02-17 at 14:50 -0800, Joe Perches wrote: > On Fri, 2017-02-17 at 22:41 +0100, Shiva Kerdel wrote: > > Braces should be used on all arms of these statements (CHECK).. > > [] > > diff --git a/drivers/staging/ks7010/ks_hostif.c > > b/drivers/staging/ks7010/ks_hostif.c > > [] > > @@ -2148,8 +2148,9 @@ void hostif_sme_mode_setup(struct ks_wlan_private > > *priv) > > else > > rate_octet[i] = > > priv->reg.rate_set.body[i]; > > - } else > > + } else { > > break; > > + } > > Generally, any time you see a form like this, > the test should be reversed > > for/while/do { > if (foo) { > [bar...] > } else { > break; > } > > should be: > > for/while/do { > if (!foo) > break; > [bar...] > } btw: the code would read better using a temporary. Something like: if (priv->reg.phy_type == D_11B_ONLY_MODE) { for (i = 0; i < priv->reg.rate_set.size; i++) { u8 rate = priv->reg.rate_set.body[i]; if (!IS_11B_RATE(rate)) break; rate_octet[i] = ((rate & RATE_MASK) >= TX_RATE_5M) ? (rate & RATE_MASK) : rate; } ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: ks7010: ks_*: Braces should be used on all arms of these statements
On Fri, 2017-02-17 at 22:41 +0100, Shiva Kerdel wrote: > Braces should be used on all arms of these statements (CHECK).. [] > diff --git a/drivers/staging/ks7010/ks_hostif.c > b/drivers/staging/ks7010/ks_hostif.c [] > @@ -2148,8 +2148,9 @@ void hostif_sme_mode_setup(struct ks_wlan_private *priv) > else > rate_octet[i] = > priv->reg.rate_set.body[i]; > - } else > + } else { > break; > + } Generally, any time you see a form like this, the test should be reversed for/while/do { if (foo) { [bar...] } else { break; } should be: for/while/do { if (!foo) break; [bar...] } ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] bcm2048: Fix checkpatch checks
Fix following checks: CHECK: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() + BUG_ON((index+2) >= BCM2048_MAX_RDS_RT); CHECK: spaces preferred around that '+' (ctx:VxV) + BUG_ON((index+2) >= BCM2048_MAX_RDS_RT); ^ CHECK: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() + BUG_ON((index+4) >= BCM2048_MAX_RDS_RT); CHECK: spaces preferred around that '+' (ctx:VxV) + BUG_ON((index+4) >= BCM2048_MAX_RDS_RT); ^ --- drivers/staging/media/bcm2048/radio-bcm2048.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c b/drivers/staging/media/bcm2048/radio-bcm2048.c index 37bd439..d5ee279 100644 --- a/drivers/staging/media/bcm2048/radio-bcm2048.c +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c @@ -1534,7 +1534,7 @@ static int bcm2048_parse_rt_match_c(struct bcm2048_device *bdev, int i, if (crc == BCM2048_RDS_CRC_UNRECOVARABLE) return 0; - BUG_ON((index+2) >= BCM2048_MAX_RDS_RT); + WARN_ON((index + 2) >= BCM2048_MAX_RDS_RT); if ((bdev->rds_info.radio_text[i] & BCM2048_RDS_BLOCK_MASK) == BCM2048_RDS_BLOCK_C) { @@ -1557,7 +1557,7 @@ static void bcm2048_parse_rt_match_d(struct bcm2048_device *bdev, int i, if (crc == BCM2048_RDS_CRC_UNRECOVARABLE) return; - BUG_ON((index+4) >= BCM2048_MAX_RDS_RT); + WARN_ON((index + 4) >= BCM2048_MAX_RDS_RT); if ((bdev->rds_info.radio_text[i] & BCM2048_RDS_BLOCK_MASK) == BCM2048_RDS_BLOCK_D) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192u: Fix warnings about endianness
On February 16, 2017 4:31:16 AM GMT+08:00, Dan Carpenter wrote: >On Wed, Feb 15, 2017 at 09:33:15AM +0100, Arnd Bergmann wrote: >> I see the same warning was addressed very differently in 99277c1f9962 >> ("Staging: rtl8192e: Fix Sparse warning of cast to restricted __le16 >in >> rtllib_crypt_tkip.c"), which was for a close relative of that >driver. >> >> Only one of the two approaches (at most) can be correct, so we >> regardless of your patch either rtl8192e or rtl8192u is broken on >> big-endian machines. > >99277c1f9962 ("Staging: rtl8192e: Fix Sparse warning of cast to >restricted __le16 in >rtllib_crypt_tkip.c") is obviously broken. Can you send a patch to >change it back? > >regards, >dan carpenter yes,I have done.But I have receive nothing about this patch from lkml for days -- Sent from Kaiten Mail. Please excuse my brevity. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/6] staging: rtl8192e: Replaced comparison to NULL statements
This patch corrects check generated by checkpatch.pl by replacing comparison to null statements with equivalent statements in the form of "!x". Signed-off-by: simran singhal --- drivers/staging/rtl8192e/rtl819x_BAProc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/rtl8192e/rtl819x_BAProc.c b/drivers/staging/rtl8192e/rtl819x_BAProc.c index 20260af..bdbd21c 100644 --- a/drivers/staging/rtl8192e/rtl819x_BAProc.c +++ b/drivers/staging/rtl8192e/rtl819x_BAProc.c @@ -83,12 +83,12 @@ static struct sk_buff *rtllib_ADDBA(struct rtllib_device *ieee, u8 *Dst, netdev_dbg(ieee->dev, "%s(): frame(%d) sentd to: %pM, ieee->dev:%p\n", __func__, type, Dst, ieee->dev); - if (pBA == NULL) { + if (!pBA) { netdev_warn(ieee->dev, "pBA is NULL\n"); return NULL; } skb = dev_alloc_skb(len + sizeof(struct rtllib_hdr_3addr)); - if (skb == NULL) + if (!skb) return NULL; memset(skb->data, 0, sizeof(struct rtllib_hdr_3addr)); @@ -154,7 +154,7 @@ static struct sk_buff *rtllib_DELBA(struct rtllib_device *ieee, u8 *dst, DelbaParamSet.field.TID = pBA->BaParamSet.field.TID; skb = dev_alloc_skb(len + sizeof(struct rtllib_hdr_3addr)); - if (skb == NULL) + if (!skb) return NULL; skb_reserve(skb, ieee->tx_headroom); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel