RE: [PATCH] [repost] Drivers: hv: vmbus: Offload the handling of channels to two workqueues
> From: gre...@linuxfoundation.org
> Sent: Wednesday, November 28, 2018 11:45 PM
> >
> > There is no change in this repost. I just rebased this patch to today's
> > char-misc's char-misc-next branch. Previously KY posted the patch with his
> > Signed-off-by (which is kept in this repost), but there was a conflict
> > issue.
> >
> > Note: the patch can't be cleanly applied to char-misc's char-misc-linus
> > branch
> --
> > to do that, we need to cherry-pick the supporting patch first:
> > 4d3c5c69191f ("Drivers: hv: vmbus: Remove the useless API
> vmbus_get_outgoing_channel()")
>
> That is not going to work for the obvious reason that this dependant
> patch is not going to be merged into 4.20-final.
It looks the dependent patch (4d3c5c69191f) is going to miss the v4.20 release.
This is not a big issue, as the dependent patch isn't really important.
> So, what do you expect us to do here? The only way this can be accepted
> is to have it go into my -next branch, which means it will show up in
> 4.21-rc1, is that ok?
Is there any chance for this patch ("Drivers: hv: vmbus: Offload the handling
...") to
go into v4.20?
If yes, I can quickly do a rebase to char-misc's char-misc-linus branch,
because actually the conflict can be very easily fixed. And I can help to fix
any
conflict when the dependent patch is backported to v4.20.1.
If no, I think this patch and the dependent patch can both go into v4.21, and
they can be both backported to v4.20.1 in future.
> But then, if that happens, it will fail to apply to any stable tree for
> 4.20 and older, like you are asking it to be done for.
>
> So what do you expect me to do here with this?
>
> totally confused,
>
> greg k-h
I hope my above reply made me clear. Sorry, I'm not really know how exactly
the releasing procedure works...
Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: correct inconsistent indenting
On Wed, 2018-11-28 at 19:17 +0100, Michael Straube wrote:
> Correct inconsistent indenting reported by smatch.
[]
> diff --git a/drivers/staging/wilc1000/wilc_spi.c
> b/drivers/staging/wilc1000/wilc_spi.c
[]
> @@ -963,7 +963,7 @@ static int wilc_spi_read_int(struct wilc *wilc, u32
> *int_status)
> dev_err(&spi->dev,
> "Unexpected interrupt(2):j=%d,tmp=%x,mask=%x\n",
> j, tmp, unknown_mask);
> - happened = 1;
> + happened = 1;s a
> }
>
> j++;
Perhaps a little refactoring instead
---
drivers/staging/wilc1000/wilc_spi.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_spi.c
b/drivers/staging/wilc1000/wilc_spi.c
index 35ff432587fe..a38ddb1f0a1f 100644
--- a/drivers/staging/wilc1000/wilc_spi.c
+++ b/drivers/staging/wilc1000/wilc_spi.c
@@ -927,7 +927,8 @@ static int wilc_spi_read_int(struct wilc *wilc, u32
*int_status)
int ret;
u32 tmp;
u32 byte_cnt;
- int happened, j;
+ bool unexpected_irq;
+ int j;
u32 unknown_mask;
u32 irq_flags;
int k = IRG_FLAGS_OFFSET + 5;
@@ -947,8 +948,6 @@ static int wilc_spi_read_int(struct wilc *wilc, u32
*int_status)
j = 0;
do {
- happened = 0;
-
wilc_spi_read_reg(wilc, 0x1a90, &irq_flags);
tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
@@ -959,15 +958,14 @@ static int wilc_spi_read_int(struct wilc *wilc, u32
*int_status)
unknown_mask = ~((1ul << spi_priv->nint) - 1);
- if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
+ unexpected_irq = (tmp >> IRG_FLAGS_OFFSET) & unknown_mask;
+ if (unexpected_irq)
dev_err(&spi->dev,
"Unexpected interrupt(2):j=%d,tmp=%x,mask=%x\n",
j, tmp, unknown_mask);
- happened = 1;
- }
j++;
- } while (happened);
+ } while (unexpected_irq);
*int_status = tmp;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: cedrus: Remove global IRQ spin lock from the driver
Hi, On Fri, 2018-11-16 at 11:47 +0100, Maxime Ripard wrote: > On Thu, Nov 15, 2018 at 03:39:55PM +0100, Paul Kocialkowski wrote: > > We initially introduced a spin lock to ensure that the VPU registers > > are not accessed concurrently between our setup function and IRQ > > handler. Because the V4L2 M2M API only allows one job to run at a time > > and our jobs are completed following the IRQ handler, there is actually > > no chance of an interrupt happening while programming the VPU registers. > > That's not entirely true. There's no chance that the interrupt > signaling the end of the frame decoding can happen. > > However, spurious interrupts can happen at any point in time as soon > as you have the interrupts enabled. > > > In addition, holding a spin lock is problematic when doing more than > > simply configuring the registers in the setup operation. H.265 support > > currently involves a CMA allocation in that step, which is not > > compatible with an atomic context. > > That's not really true either. Any allocation can be done with > GFP_ATOMIC or GFP_NOWAIT, and be compatible with an atomic > context. Whether it's something we want is a different story :) > > And since the h265 code isn't upstream, I'm not really sure it's > relevant to mention it here. Thanks for your comments, I have just made associated changes and sent out v2. Cheers! * Paul > > As a result, remove the global IRQ spin lock. > > > > Signed-off-by: Paul Kocialkowski > > Acked-by: Maxime Ripard > > Maxime > -- Paul Kocialkowski, Bootlin (formerly Free Electrons) Embedded Linux and kernel engineering https://bootlin.com signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: iio: ad7780: Add gain & filter gpio support
On Mi, 2018-11-28 at 16:15 -0200, Giuliano Belinassi wrote:
> Previously, the AD7780 driver only supported gpio for the 'powerdown'
> pin. This commit adds suppport for the 'gain' and 'filter' pin.
>
> Signed-off-by: Giuliano Belinassi
> ---
> Changes in v2:
> - Now this patch is part of the patchset that aims to remove ad7780
> out of staging. https://marc.info/?l=linux-iio&m=154282349808890&w=2
> - Also, now it reads voltage and filter values from the userspace
> instead of gpio pin states.
Hello,
Please see bellow.
>
> drivers/staging/iio/adc/ad7780.c | 78 --
> include/linux/iio/adc/ad_sigma_delta.h | 5 ++
> 2 files changed, 79 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7780.c
> b/drivers/staging/iio/adc/ad7780.c
> index c4a85789c2db..05979a79fda3 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -39,6 +39,12 @@
> #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2)
> #define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 |
> AD7170_PAT2)
>
> +#define AD7780_GAIN_GPIO 0
> +#define AD7780_FILTER_GPIO 1
> +
> +#define AD7780_GAIN_MIDPOINT 64
> +#define AD7780_FILTER_MIDPOINT 13350
> +
> struct ad7780_chip_info {
> struct iio_chan_specchannel;
> unsigned intpattern_mask;
> @@ -50,6 +56,8 @@ struct ad7780_state {
> const struct ad7780_chip_info *chip_info;
> struct regulator*reg;
> struct gpio_desc*powerdown_gpio;
> + struct gpio_desc*gain_gpio;
> + struct gpio_desc*filter_gpio;
> unsigned intgain;
>
> struct ad_sigma_delta sd;
> @@ -115,18 +123,65 @@ static int ad7780_read_raw(struct iio_dev
> *indio_dev,
> return -EINVAL;
> }
>
> +static int ad7780_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
> + long m)
> +{
> + struct ad7780_state *st = iio_priv(indio_dev);
> + const struct ad7780_chip_info *chip_info = st->chip_info;
> + int uvref, gain;
> + unsigned int full_scale;
> +
> + if (!chip_info->is_ad778x)
> + return 0;
> +
> + switch (m) {
> + case IIO_CHAN_INFO_SCALE:
> + if (val != 0)
> + return -EINVAL;
> +
> + uvref = regulator_get_voltage(st->reg);
regulator_get_voltage() has already been called in the probe function and
the result is stored in st->int_vref_mv.
My suggestion would be to use a local vref variable declared as unsigned
int. It is my fault that I haven't explained correctly in the previous
email, but you need to multiply vref_mv with 100LL in order to get the
right precision: vref = st->int_vref_mv * 100LL. Afterwards you will be
able to perform the divisions.
> +
> + if (uvref < 0)
> + return uvref;
> +
> + full_scale = 1 << (chip_info->channel.scan_type.realbits
> - 1);
> + gain = DIV_ROUND_CLOSEST(uvref, full_scale);
> + gain = DIV_ROUND_CLOSEST(gain, val2);
> +
> + gpiod_set_value(st->gain_gpio, gain <
> AD7780_GAIN_MIDPOINT ? 0 : 1);
Once the gain is set, you can store it in st->gain variable.
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (val2 != 0)
> + return -EINVAL;
> +
> + gpiod_set_value(st->filter_gpio, val <
> AD7780_FILTER_MIDPOINT ? 0 : 1);
This is probably fine, although I am not a big fan of the ternary operator.
A simple if else statement would do. However, I don't feel strongly about
it, so feel free to disagree.
> + break;
> + }
> +
> + return 0;
> +}
> +
> static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta,
> unsigned int raw_sample)
> {
> struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta);
> const struct ad7780_chip_info *chip_info = st->chip_info;
> + int val;
>
> if ((raw_sample & AD7780_ERR) ||
> ((raw_sample & chip_info->pattern_mask) != chip_info-
> >pattern))
> return -EIO;
>
> if (chip_info->is_ad778x) {
> - if (raw_sample & AD7780_GAIN)
> + val = raw_sample & AD7780_GAIN;
> +
> + if (val != gpiod_get_value(st->gain_gpio))
> + return -EIO;
It is not obvious to me what is the point of this check. Maybe you can add
a comment?
> +
> + if (val)
> st->gain = 1;
> else
> st->gain = 128;
Do we still need this? I am not convinced.
> @@ -141,18 +196,20 @@ static const struct ad_sigma_delta_info
> ad7780_sigma_delta_info = {
> .has_registers = false,
> };
>
> -#define AD7780_CHANNEL(bits, wordsize) \
> +#define AD7170_CHANNEL(bits, word
[bug report] staging: Import the BCM2835 MMAL-based V4L2 camera driver.
Hello Eric Anholt,
The patch 7b3ad5abf027: "staging: Import the BCM2835 MMAL-based V4L2
camera driver." from Jan 27, 2017, leads to the following static
checker warning:
drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c:1640
vchiq_mmal_component_init()
error: buffer overflow 'component->input' 4 <= 187
drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
1607 int vchiq_mmal_component_init(struct vchiq_mmal_instance *instance,
1608const char *name,
1609struct vchiq_mmal_component
**component_out)
1610 {
1611 int ret;
1612 int idx;/* port index */
1613 struct vchiq_mmal_component *component;
1614
1615 if (mutex_lock_interruptible(&instance->vchiq_mutex))
1616 return -EINTR;
1617
1618 if (instance->component_idx == VCHIQ_MMAL_MAX_COMPONENTS) {
1619 ret = -EINVAL; /* todo is this correct error? */
1620 goto unlock;
1621 }
1622
1623 component = &instance->component[instance->component_idx];
1624
1625 ret = create_component(instance, component, name);
1626 if (ret < 0)
1627 goto unlock;
1628
1629 /* ports info needs gathering */
1630 component->control.type = MMAL_PORT_TYPE_CONTROL;
1631 component->control.index = 0;
1632 component->control.component = component;
1633 spin_lock_init(&component->control.slock);
1634 INIT_LIST_HEAD(&component->control.buffers);
1635 ret = port_info_get(instance, &component->control);
1636 if (ret < 0)
1637 goto release_component;
1638
1639 for (idx = 0; idx < component->inputs; idx++) {
^
This is set in create_component(). I have no idea why Smatch thinks
that it is 187, but it does seem like it should be capped to make sure
it's not larger than 4.
1640 component->input[idx].type = MMAL_PORT_TYPE_INPUT;
1641 component->input[idx].index = idx;
1642 component->input[idx].component = component;
1643 spin_lock_init(&component->input[idx].slock);
1644 INIT_LIST_HEAD(&component->input[idx].buffers);
1645 ret = port_info_get(instance, &component->input[idx]);
1646 if (ret < 0)
1647 goto release_component;
1648 }
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad7780: Add gain & filter gpio support
On Tue, 2018-11-27 at 06:11 -0500, Popa, Stefan Serban wrote: > On Lu, 2018-11-26 at 17:24 -0200, Giuliano Belinassi wrote: > Hi, please see bellow > One note from me here. > > Hi, thank you for the review > > > > > > > > On Thu, 22 Nov 2018 11:01:00 + > > > "Popa, Stefan Serban" wrote: > > > > > > > > I think that instead of setting the gain directly, we should use > > > > the IIO_CHAN_INFO_SCALE attribute. At page 12 of the ad7780 > > > > datasheet > > > > there > > > > is a formula from which the output code can be calculated: > > > > Code = 2^(N − 1) > > > > × [(AIN × Gain /VREF) + 1]. So, by setting the scale from user > > > > space, > > > > the > > > > driver can calculate the correct gain by using the formula above. > > > > Also, it > > > > would be useful to introduce scale available. > > > > Furthermore, there is a new > > > > ad7124 adc driver which does this exact thing. Take a look here: > > > > http > > > > s://gi > > > > thub.com/analogdevicesinc/linux/blob/master/drivers/iio/adc/ad7124. > > > > c# > > > > L337. > > > > We have some questions about the code you provided to us: > > 1-) What is exactly the inputs for the write_raw function? > > In your write_raw() function you need to add IIO_CHAN_INFO_SCALE case. > Setting the scale from user space looks something like this: > root:/sys/bus/iio/devices/iio:device0> echo 0.000298 > in_voltage_scale . > Furthermore, in your write_raw() function, val=0 and val2=298. > Knowing that full_scale_voltage = Vref / (gain * scale), we can calculate > the gain = Vref / (full_scale_voltage * scale). We only support two gains > (1 and 128), so we need to determine which one fits better with the > desired > scale. Finally, all we have left to do is to set the gain. > > > 2-) Could you give more details about the math around lines 346-348? > > Is it correct to assume that the multiplication at line 346 won't > > overflow? (vref is an uint) > > It is correct that Vref is in microvolts, so for example, Vref of 2.5V = > 25uV. It won't overflow since we use the Vref as nominator, while > full_scale_voltage and scale are the denominators. > [Regarding the AD7124] I guess I should be noted that the code can overflow, but in this case it shouldn't, because (according to the device datasheet) the maximum VREF can be 3600 mV. A user could specify (in the devicetree) something larger than 4300 mV (and that would overflow) but that would also make the readings useless as the external VREF would be invalid ; and there is also the risk of frying the chip in that case [something you really can't protect the user against]. The internal VREF however is 2500 mV, so things should be fine from that point of view. Typically, in datasheets (at least from Analog Devices) it's good to take a look at the specifications sections. [For AD7124] You will see that the internal VREF [page 8] is 2.5V (with approximation of +/-0.2%) and for external reference it goes all the way up to AVDD, which has typical values of 2.9V - 3.6V. So, for u32 this code should be fine and not overflow. One small thing that can be confusing about that code in AD7124 is that it gets multiplied with 100LL (which is signed long-long), but that should be fine, since the operation should be converted to u32 (unsigned int) representation [by being assigned to vref], which ends up being fine in the end. > > > > And regarding our code: > > 1-) The val in our write_raw function should be, in case of GAIN, a > > number that best approximate the actual gain value of 1 or 128? For > > instance, if the user inputs 126, we should default to 128? > > We should not allow the the user to input the gain, he needs to input the > scale (see the mail from Jonathan and the above explanation). However, if > the calculated gain is one that is not supported, such as 126, we will > set > the closest matching value, in this case 128. > > > 2-) In the case of FILTER, is it the same? Is the user sending the > > value in mHz (milihertz)? > > Yes, it is the same with the FILTER. You need to add > a IIO_CHAN_INFO_SAMP_FREQ case in your write_raw() function. From user > space, the input value should be in Hz, something like this: > root:/sys/bus/iio/devices/iio:device0> echo 10 > > in_voltage_sampling_frequency > > > > > Thank you ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: iio: ad7780: Add gain & filter gpio support
Hi
On Thu, Nov 29, 2018 at 9:18 AM Popa, Stefan Serban
wrote:
>
> On Mi, 2018-11-28 at 16:15 -0200, Giuliano Belinassi wrote:
> > Previously, the AD7780 driver only supported gpio for the 'powerdown'
> > pin. This commit adds suppport for the 'gain' and 'filter' pin.
> >
> > Signed-off-by: Giuliano Belinassi
> > ---
> > Changes in v2:
> > - Now this patch is part of the patchset that aims to remove ad7780
> > out of staging. https://marc.info/?l=linux-iio&m=154282349808890&w=2
> > - Also, now it reads voltage and filter values from the userspace
> > instead of gpio pin states.
>
> Hello,
> Please see bellow.
>
> >
> > drivers/staging/iio/adc/ad7780.c | 78 --
> > include/linux/iio/adc/ad_sigma_delta.h | 5 ++
> > 2 files changed, 79 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7780.c
> > b/drivers/staging/iio/adc/ad7780.c
> > index c4a85789c2db..05979a79fda3 100644
> > --- a/drivers/staging/iio/adc/ad7780.c
> > +++ b/drivers/staging/iio/adc/ad7780.c
> > @@ -39,6 +39,12 @@
> > #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2)
> > #define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 |
> > AD7170_PAT2)
> >
> > +#define AD7780_GAIN_GPIO 0
> > +#define AD7780_FILTER_GPIO 1
> > +
> > +#define AD7780_GAIN_MIDPOINT 64
> > +#define AD7780_FILTER_MIDPOINT 13350
> > +
> > struct ad7780_chip_info {
> > struct iio_chan_specchannel;
> > unsigned intpattern_mask;
> > @@ -50,6 +56,8 @@ struct ad7780_state {
> > const struct ad7780_chip_info *chip_info;
> > struct regulator*reg;
> > struct gpio_desc*powerdown_gpio;
> > + struct gpio_desc*gain_gpio;
> > + struct gpio_desc*filter_gpio;
> > unsigned intgain;
> >
> > struct ad_sigma_delta sd;
> > @@ -115,18 +123,65 @@ static int ad7780_read_raw(struct iio_dev
> > *indio_dev,
> > return -EINVAL;
> > }
> >
> > +static int ad7780_write_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int val,
> > + int val2,
> > + long m)
> > +{
> > + struct ad7780_state *st = iio_priv(indio_dev);
> > + const struct ad7780_chip_info *chip_info = st->chip_info;
> > + int uvref, gain;
> > + unsigned int full_scale;
> > +
> > + if (!chip_info->is_ad778x)
> > + return 0;
> > +
> > + switch (m) {
> > + case IIO_CHAN_INFO_SCALE:
> > + if (val != 0)
> > + return -EINVAL;
> > +
> > + uvref = regulator_get_voltage(st->reg);
>
> regulator_get_voltage() has already been called in the probe function and
> the result is stored in st->int_vref_mv.
This was removed in commit 9eae69ddbc4717a0bd702eddac76c7848773bf71
because the value was not being updated. But I agree if the vref
voltage is not going to change at all after the initialization, then
this value should be kept in memory.
> My suggestion would be to use a local vref variable declared as unsigned
> int. It is my fault that I haven't explained correctly in the previous
> email, but you need to multiply vref_mv with 100LL in order to get the
> right precision: vref = st->int_vref_mv * 100LL. Afterwards you will be
> able to perform the divisions.
Thanks for this info! :-)
Shouldn't we store this in uV (microVolts)? This will yield a more
accurate result after the multiplication.
> > +
> > + if (uvref < 0)
> > + return uvref;
> > +
> > + full_scale = 1 << (chip_info->channel.scan_type.realbits
> > - 1);
> > + gain = DIV_ROUND_CLOSEST(uvref, full_scale);
> > + gain = DIV_ROUND_CLOSEST(gain, val2);
> > +
> > + gpiod_set_value(st->gain_gpio, gain <
> > AD7780_GAIN_MIDPOINT ? 0 : 1);
>
> Once the gain is set, you can store it in st->gain variable.
Yes, we forgot it.
>
> > + case IIO_CHAN_INFO_SAMP_FREQ:
> > + if (val2 != 0)
> > + return -EINVAL;
> > +
> > + gpiod_set_value(st->filter_gpio, val <
> > AD7780_FILTER_MIDPOINT ? 0 : 1);
>
> This is probably fine, although I am not a big fan of the ternary operator.
> A simple if else statement would do. However, I don't feel strongly about
> it, so feel free to disagree.
>
> > + break;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int ad7780_postprocess_sample(struct ad_sigma_delta *sigma_delta,
> >unsigned int raw_sample)
> > {
> > struct ad7780_state *st = ad_sigma_delta_to_ad7780(sigma_delta);
> > const struct ad7780_chip_info *chip_info = st->chip_info;
> > + int val;
> >
> > if ((raw_sample & AD7780_ERR) ||
> > ((raw_sample & chip_info->pattern_mask) != chip_info-
> > >pattern))
> > return -EIO;
> >
> > if (ch
Re: [PATCH] staging: wilc1000: correct inconsistent indenting
On 11/29/18 7:24 AM, Joe Perches wrote:
On Wed, 2018-11-28 at 19:17 +0100, Michael Straube wrote:
Correct inconsistent indenting reported by smatch.
[]
diff --git a/drivers/staging/wilc1000/wilc_spi.c
b/drivers/staging/wilc1000/wilc_spi.c
[]
@@ -963,7 +963,7 @@ static int wilc_spi_read_int(struct wilc *wilc, u32
*int_status)
dev_err(&spi->dev,
"Unexpected interrupt(2):j=%d,tmp=%x,mask=%x\n",
j, tmp, unknown_mask);
- happened = 1;
+ happened = 1;s a
}
j++;
Perhaps a little refactoring instead
---
drivers/staging/wilc1000/wilc_spi.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_spi.c
b/drivers/staging/wilc1000/wilc_spi.c
index 35ff432587fe..a38ddb1f0a1f 100644
--- a/drivers/staging/wilc1000/wilc_spi.c
+++ b/drivers/staging/wilc1000/wilc_spi.c
@@ -927,7 +927,8 @@ static int wilc_spi_read_int(struct wilc *wilc, u32
*int_status)
int ret;
u32 tmp;
u32 byte_cnt;
- int happened, j;
+ bool unexpected_irq;
+ int j;
u32 unknown_mask;
u32 irq_flags;
int k = IRG_FLAGS_OFFSET + 5;
@@ -947,8 +948,6 @@ static int wilc_spi_read_int(struct wilc *wilc, u32
*int_status)
j = 0;
do {
- happened = 0;
-
wilc_spi_read_reg(wilc, 0x1a90, &irq_flags);
tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
@@ -959,15 +958,14 @@ static int wilc_spi_read_int(struct wilc *wilc, u32 *int_status)
unknown_mask = ~((1ul << spi_priv->nint) - 1);
- if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
+ unexpected_irq = (tmp >> IRG_FLAGS_OFFSET) & unknown_mask;
+ if (unexpected_irq)
dev_err(&spi->dev,
"Unexpected interrupt(2):j=%d,tmp=%x,mask=%x\n",
j, tmp, unknown_mask);
- happened = 1;
- }
j++;
- } while (happened);
+ } while (unexpected_irq);
*int_status = tmp;
Hi Joe,
that looks good to me.
Naming the loop condition unexpected_irq also improves readability.
I will send a new patch, thanks.
Michael
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: wilc1000: correct inconsistent indenting
Correct inconsistent indenting reported by smatch.
Instead of simply remove indentation, refactor the
loop to also improve readabilitiy.
Suggested-by: Joe Perches
Signed-off-by: Michael Straube
---
v1 -> v2
Refactor loop instead of simply remove indentation
as suggested by Joe Perches.
drivers/staging/wilc1000/wilc_spi.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_spi.c
b/drivers/staging/wilc1000/wilc_spi.c
index 35ff432587fe..153e120eff00 100644
--- a/drivers/staging/wilc1000/wilc_spi.c
+++ b/drivers/staging/wilc1000/wilc_spi.c
@@ -927,7 +927,8 @@ static int wilc_spi_read_int(struct wilc *wilc, u32
*int_status)
int ret;
u32 tmp;
u32 byte_cnt;
- int happened, j;
+ bool unexpected_irq;
+ int j;
u32 unknown_mask;
u32 irq_flags;
int k = IRG_FLAGS_OFFSET + 5;
@@ -947,8 +948,6 @@ static int wilc_spi_read_int(struct wilc *wilc, u32
*int_status)
j = 0;
do {
- happened = 0;
-
wilc_spi_read_reg(wilc, 0x1a90, &irq_flags);
tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
@@ -959,15 +958,15 @@ static int wilc_spi_read_int(struct wilc *wilc, u32
*int_status)
unknown_mask = ~((1ul << spi_priv->nint) - 1);
- if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
+ unexpected_irq = (tmp >> IRG_FLAGS_OFFSET) & unknown_mask;
+ if (unexpected_irq) {
dev_err(&spi->dev,
"Unexpected interrupt(2):j=%d,tmp=%x,mask=%x\n",
j, tmp, unknown_mask);
- happened = 1;
}
j++;
- } while (happened);
+ } while (unexpected_irq);
*int_status = tmp;
--
2.19.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v2] android: ion: How to properly clean caches for uncached allocations
On Wed, Nov 28, 2018 at 11:03:37PM -0800, Liam Mark wrote:
> On Wed, 28 Nov 2018, Brian Starkey wrote:
> > On Tue, Nov 27, 2018 at 10:46:07PM -0800, Liam Mark wrote:
> > > On Tue, 27 Nov 2018, Brian Starkey wrote:
> > > > On Mon, Nov 26, 2018 at 08:59:44PM -0800, Liam Mark wrote:
> > > > > On Tue, 20 Nov 2018, Brian Starkey wrote:
[snip]
> > > >
> > >
> > > Sounds like you are suggesting using carveouts to support uncached?
> > >
> >
> > No, I'm just saying that ion can't give out uncached _CPU_ mappings
> > for pages which are already mapped on the CPU as cached.
Probably this should have been: s/can't/shouldn't/
> >
>
> Okay then I guess I am not clear on where you would get this memory
> which doesn't have a cached kernel mapping.
> It sounded like you wanted to define sections of memory in the DT as not
> mapped in the kernel and then hand this memory to
> dma_declare_coherent_memory (so that it can be managed) and then use an
> ION heap as the allocator. If the memory was defined this way it sounded
> a lot like a carveout. But I guess you have some thoughts on how this
> memory which doesn't have a kernel mapping can be made available for general
> use (for example available in buddy)?
>
> Perhaps you were thinking of dynamically removing the kernel mappings
> before handing it out as uncached, but this would have a general system
> performance impact as this memory could come from anywhere so we would
> quickly lose our 1GB block mappings (and likely many of our 2MB block
> mappings as well).
>
All I'm saying, with respect to non-cached memory and mappings, is
this:
I don't think ion should create non-cached CPU mappings of memory
which is mapped in the kernel as cached.
By extension, that means that in my opinion, the only way userspace
should be able to get a non-cached mapping, is by allocating from a
carveout.
However, I don't think this should be what we do in our complicated
media-heavy systems - carveouts are clearly impractical, as is
removing memory from the kernel map. What I think we should do, is
always do CPU access via cached mappings, for memory which is mapped
in the kernel as cached.
[snip]
> >
>
> I am not sure I am properly understanding as this is what my V2 patch
> does, then when it gets an opportunity it allows the memory to be
> re-mapped as uncached.
It's the remapping as uncached part which I'm not so keen on. It just
seems rather fragile to have mappings for the same memory with
different attributes around.
>
> Or are you perhaps suggesting that if the memory is allocated from a
> cached region then it always remains as cached, so only provide uncached
> if it was allocated from an uncached region? If so I view all memory
> available to the ION system heap for uncached allocations as having a
> cached mapping (since it is all part of the kernel logical mappigns), so I
> can't see how this would ever be able to support uncached allocations.
Yeah, that's exactly what I'm saying. The system heap should not
allow uncached allocations, and, memory allocated from the system heap
should always be mapped as cached for CPU accesses.
Non-cached allocations would only be allowed from carveouts (but as
discussed, I don't think carveouts are a practical solution for the
general case).
The summary of my proposal is that instead of focussing on getting
non-cached allocations, we should make cached allocations work better,
so that non-cached aliases of cached memory aren't required.
[snip]
>
> Unfortunately if are only using cached mappings it isn't only the first
> time you dma map the buffer you need to do cache maintenance, you need to
> almost always do it because you don't know what CPU access happened (or
> will happen) without a device.
I think you can always know if CPU _has_ accessed the buffer - in
begin_cpu_access, ion can set a flag, which it checks in map_dma_buf.
If that flag says it's been touched, then a cache clean is needed.
Of course you can't predict the future - there's no way to know if the
CPU _will_ access the buffer - which I think is what you're getting at
below.
> Explained more below.
>
> > > But with this cached memory you get poor performance because you are
> > > frequently doing cache mainteance uncessarily because there *could* be
> > > CPU access.
> > >
> > > The reason we want to use uncached allocations, with uncached mappings,
> > > is
> > > to avoid all this uncessary cache maintenance.
> > >
> >
> > OK I think this is the key - you don't actually care whether the
> > mappings are non-cached, you just don't want to pay a sync penalty if
> > the CPU never touched the buffer.
> >
> > In that case, then to me the right thing to do is make ion use
> > dma_map_sg_attrs(..., DMA_ATTR_SKIP_CPU_SYNC) in ion_map_dma_buf(), if
> > it knows that the CPU hasn't touched the buffer (which it does - from
> > {begin,end}_cpu_access).
> >
>
> Unfortunately that isn't the case we are trying to optimize for
[PATCH v3 1/2] staging: iio: ad7606: Move out of staging
Move ad7606 ADC driver out of staging and into the mainline. Signed-off-by: Stefan Popa --- Changes in v2: - Simplified the Kconfig menu. - Added SPDX-License-Identifier. - Ordered the includes alphabetically. - Used a threaded interrupt. - Replaced ad7606_poll_bh_to_ring() with ad7606_trigger_handler(). - Used a trigger. - Replaced wait_event_interruptible() with wait_for_completion_timeout(). - Replaced wake_up_interruptible() with complete(). - Used devm_iio_triggered_buffer_setup(). - Added buffer_ops. - Used single line comments where needed. - Removed the gap between docs and struct. - Added ad7606_of_match[]. Changes in v3: - Added a comment which offers more information of the way the interrupt is triggered. - Fixed the way a new conversion is triggered. - endianness = IIO_CPU - Removed unnecessary mutex locks. - Removed the buffer_postdisable ops and replaced it with buffer_predisable. - Added a devm_add_action_or_reset() which deals with regulator_disable(). - Misc style fixes. MAINTAINERS | 7 + drivers/iio/adc/Kconfig | 28 ++ drivers/iio/adc/Makefile | 3 + drivers/iio/adc/ad7606.c | 605 +++ drivers/iio/adc/ad7606.h | 106 ++ drivers/iio/adc/ad7606_par.c | 105 ++ drivers/iio/adc/ad7606_spi.c | 82 + drivers/staging/iio/adc/Kconfig | 34 -- drivers/staging/iio/adc/Makefile | 3 - drivers/staging/iio/adc/ad7606.c | 565 drivers/staging/iio/adc/ad7606.h | 106 -- drivers/staging/iio/adc/ad7606_par.c | 113 --- drivers/staging/iio/adc/ad7606_spi.c | 79 - 13 files changed, 936 insertions(+), 900 deletions(-) create mode 100644 drivers/iio/adc/ad7606.c create mode 100644 drivers/iio/adc/ad7606.h create mode 100644 drivers/iio/adc/ad7606_par.c create mode 100644 drivers/iio/adc/ad7606_spi.c delete mode 100644 drivers/staging/iio/adc/ad7606.c delete mode 100644 drivers/staging/iio/adc/ad7606.h delete mode 100644 drivers/staging/iio/adc/ad7606_par.c delete mode 100644 drivers/staging/iio/adc/ad7606_spi.c diff --git a/MAINTAINERS b/MAINTAINERS index f642044..843545d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -839,6 +839,13 @@ S: Supported F: drivers/iio/dac/ad5758.c F: Documentation/devicetree/bindings/iio/dac/ad5758.txt +ANALOG DEVICES INC AD7606 DRIVER +M: Stefan Popa +L: linux-...@vger.kernel.org +W: http://ez.analog.com/community/linux-device-drivers +S: Supported +F: drivers/iio/adc/ad7606.c + ANALOG DEVICES INC AD9389B DRIVER M: Hans Verkuil L: linux-me...@vger.kernel.org diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index a52fea8..c3f61c9 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -58,6 +58,34 @@ config AD7476 To compile this driver as a module, choose M here: the module will be called ad7476. +config AD7606 + tristate + depends on GPIOLIB || COMPILE_TEST + depends on HAS_IOMEM + select IIO_BUFFER + select IIO_TRIGGERED_BUFFER + +config AD7606_IFACE_PARALLEL + tristate "Analog Devices AD7606 ADC driver with parallel interface support" + select AD7606 + help + Say yes here to build parallel interface support for Analog Devices: + ad7605-4, ad7606, ad7606-6, ad7606-4 analog to digital converters (ADC). + + To compile this driver as a module, choose M here: the + module will be called ad7606_parallel. + +config AD7606_IFACE_SPI + tristate "Analog Devices AD7606 ADC driver with spi interface support" + depends on SPI + select AD7606 + help + Say yes here to build spi interface support for Analog Devices: + ad7605-4, ad7606, ad7606-6, ad7606-4 analog to digital converters (ADC). + + To compile this driver as a module, choose M here: the + module will be called ad7606_spi. + config AD7766 tristate "Analog Devices AD7766/AD7767 ADC driver" depends on SPI_MASTER diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index a6e6a0b..b734f4f 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -8,6 +8,9 @@ obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o obj-$(CONFIG_AD7266) += ad7266.o obj-$(CONFIG_AD7291) += ad7291.o obj-$(CONFIG_AD7298) += ad7298.o +obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o +obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o +obj-$(CONFIG_AD7606) += ad7606.o obj-$(CONFIG_AD7923) += ad7923.o obj-$(CONFIG_AD7476) += ad7476.o obj-$(CONFIG_AD7766) += ad7766.o diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c new file mode 100644 index 000..7eb06b3 --- /dev/null +++ b/driv
[PATCH] staging: mt7621-pci: add comment clarifying inverted reset lines
To avoid people reading this code being very confused, add a comment clarifying the need for invert resets on some chip revisions. Suggested-by: Dan Carpenter Signed-off-by: Sergio Paracuellos --- drivers/staging/mt7621-pci/pci-mt7621.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c index c5e33fbdf225..31310b6fb7db 100644 --- a/drivers/staging/mt7621-pci/pci-mt7621.c +++ b/drivers/staging/mt7621-pci/pci-mt7621.c @@ -589,6 +589,10 @@ static int mt7621_pcie_init_port(struct mt7621_pcie_port *port) u32 slot = port->slot; u32 val = 0; + /* +* Any MT7621 Ralink pcie controller that doesn't have 0x0101 at +* the end of the chip_id has inverted PCI resets. +*/ mt7621_reset_port(port); val = read_config(pcie, slot, PCIE_FTS_NUM); -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/15] arm64: dts: allwinner: h5: Add system-control node with SRAM C1
On Fri, Nov 16, 2018 at 12:52 AM Chen-Yu Tsai wrote:
>
> On Thu, Nov 15, 2018 at 10:50 PM Paul Kocialkowski
> wrote:
> >
> > Add the H5-specific system control node description to its device-tree
> > with support for the SRAM C1 section, that will be used by the video
> > codec node later on.
> >
> > Signed-off-by: Paul Kocialkowski
> > ---
> > arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 22
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
> > b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
> > index b41dc1aab67d..c2d14b22b8c1 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
> > @@ -94,6 +94,28 @@
> > };
> >
> > soc {
> > + system-control@1c0 {
> > + compatible = "allwinner,sun50i-h5-system-control";
> > + reg = <0x01c0 0x1000>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges;
> > +
> > + sram_c1: sram@1d0 {
> > + compatible = "mmio-sram";
> > + reg = <0x01d0 0x8>;
>
> I'll try to check this one tomorrow.
>
> I did find something interesting on the H3: there also seems to be SRAM at
> 0x01dc to 0x01dcfeff , again mapped by the same bits as SRAM C1.
>
> And on the A33, the SRAM C1 range is 0x01d0 to 0x01d478ff.
>
> This was found by mapping the SRAM to the CPU, then using devmem to poke
> around the register range. If there's SRAM, the first read would typically
> return random data, and a subsequent write to it would set some value that
> would be read back correctly. If there's no SRAM, a read either returns 0x0
> or some random data that can't be overwritten.
>
> You might want to check the other SoCs.
This range seems to contain stuff other than SRAM, possibly fixed lookup
tables. Since this is entirely unknown, lets just stick to the known full
range instead.
ChenYu
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges = <0 0x01d0 0x8>;
> > +
> > + ve_sram: sram-section@0 {
> > + compatible =
> > "allwinner,sun50i-h5-sram-c1",
> > +
> > "allwinner,sun4i-a10-sram-c1";
> > + reg = <0x00 0x8>;
> > + };
> > + };
> > + };
> > +
> > mali: gpu@1e8 {
> > compatible = "allwinner,sun50i-h5-mali",
> > "arm,mali-450";
> > reg = <0x01e8 0x3>;
> > --
> > 2.19.1
> >
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
