Re: [PATCH v3 2/2] dt-bindings: net: dsa: ksz9477: add sample of switch bindings managed in i2c mode
Hi Florian, On Mon, Dec 17, 2018 at 11:22 PM Florian Fainelli wrote: > > On 12/17/18 12:44 PM, Sergio Paracuellos wrote: > > Add device-tree binding example of the ksz9477 switch managed in i2c mode. > > > > Cc: devicet...@vger.kernel.org > > Signed-off-by: Sergio Paracuellos > > This looks good, although it looks like you have 2x the amount of tabs > necessary. Yes, I know but the original spi sample included in the same file was created with that indentation, so I only followed the same file style. > -- > Florian Best regards, Sergio Paracuellos ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 2/2] dt-bindings: net: dsa: ksz9477: add sample of switch bindings managed in i2c mode
On 12/17/18 12:44 PM, Sergio Paracuellos wrote: > Add device-tree binding example of the ksz9477 switch managed in i2c mode. > > Cc: devicet...@vger.kernel.org > Signed-off-by: Sergio Paracuellos This looks good, although it looks like you have 2x the amount of tabs necessary. -- Florian ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: iio: adc: ad7280a: fix codestyle in ad7280_event_handler
staging: iio: adc: ad7280a: fix codestyle in ad7280_event_handler
Signed-off-by: Sergey Efimochkin
---
Changes in v2:
- changed patch prefix
- changed patch description
- removed incorrect fixes to codestyle
drivers/staging/iio/adc/ad7280a.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7280a.c
b/drivers/staging/iio/adc/ad7280a.c
index 58420dcb406d..f167ee1b2127 100644
--- a/drivers/staging/iio/adc/ad7280a.c
+++ b/drivers/staging/iio/adc/ad7280a.c
@@ -725,8 +725,7 @@ static irqreturn_t ad7280_event_handler(int irq, void
*private)
} else {
if (((channels[i] >> 11) & 0xFFF) >= st->aux_threshhigh)
iio_push_event(indio_dev,
- IIO_UNMOD_EVENT_CODE(
- IIO_TEMP,
+ IIO_UNMOD_EVENT_CODE(IIO_TEMP,
0,
IIO_EV_TYPE_THRESH,
IIO_EV_DIR_RISING),
@@ -734,8 +733,7 @@ static irqreturn_t ad7280_event_handler(int irq, void
*private)
else if (((channels[i] >> 11) & 0xFFF) <=
st->aux_threshlow)
iio_push_event(indio_dev,
- IIO_UNMOD_EVENT_CODE(
- IIO_TEMP,
+ IIO_UNMOD_EVENT_CODE(IIO_TEMP,
0,
IIO_EV_TYPE_THRESH,
IIO_EV_DIR_FALLING),
--
2.15.1 (Apple Git-101)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/11] staging: iio: adt7316: fix dac_bits assignment
On Sun, Dec 16, 2018 at 11:37:56AM +, Jonathan Cameron wrote:
> On Tue, 11 Dec 2018 17:54:55 -0700
> Jeremy Fertic wrote:
>
> > The only assignment to dac_bits is in adt7316_store_da_high_resolution().
> > This function enables or disables 10 bit dac resolution for the adt7316/7
> > and adt7516/7 when they're set to output voltage proportional to
> > temperature. Remove these assignments since they're unnecessary for the
> > dac high resolution functionality.
> >
> > Instead, assign a value to dac_bits in adt7316_probe() since the number
> > of dac bits might be needed as soon as the device is registered and
> > available to userspace.
> >
> > Signed-off-by: Jeremy Fertic
>
> I'm a little confused as it seems to me that in the orignal code
> if we were setting high resolution 'off' we would fall back to 8
> bits for either type of device. Now you just have a check on the
> device type?
>
> The result will be that we read more bytes than we want to
> in show_DAC.
>
> I'd need a pretty strong argument to persuade me it is worth
> supporting the 8 bit mode at all btw on devices that will go
> higher. It adds interface complexity and the number of usecases
> where the saving in bus traffic is worthwhile are probably fairly
> few!
>
> Jonathan
Thanks for the feedback Jonathan, and sorry for the confusion on this one.
My commit message should have been clearer. This is not a general purpose
option to change the dac resolution. It is specific to an optional feature
where one or two of the four dacs can be set to output voltage proportional
to temperature. If the user chooses to set dac a and/or dac b to ouput
voltage proportional to temperature, this da_high_resolution attribute can
optionally be enabled to use 10 bit resolution rather than the default 8
bits. It is only available on the 10 and 12 bit dac devices. If the user
attempts to read or write dacs a or b under these settings, the driver's
current behaviour is to return an error. The dacs c and d continue to
operate normally under these conditions.
With the above in mind, dac_bits should have never been assigned to in this
da_high_resolution attribute. Before this patch, if the user didn't write
to this optional attribute, dac_bits would be 0 since it was never assigned
to anywhere else. This would result in incorrect calculations in show_DAC
and store_DAC.
Jeremy
> > ---
> > drivers/staging/iio/addac/adt7316.c | 18 +-
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/staging/iio/addac/adt7316.c
> > b/drivers/staging/iio/addac/adt7316.c
> > index e5e1f9d6357f..a9990e7f2a4d 100644
> > --- a/drivers/staging/iio/addac/adt7316.c
> > +++ b/drivers/staging/iio/addac/adt7316.c
> > @@ -651,17 +651,10 @@ static ssize_t
> > adt7316_store_da_high_resolution(struct device *dev,
> > u8 config3;
> > int ret;
> >
> > - chip->dac_bits = 8;
> > -
> > - if (buf[0] == '1') {
> > + if (buf[0] == '1')
> > config3 = chip->config3 | ADT7316_DA_HIGH_RESOLUTION;
> > - if (chip->id == ID_ADT7316 || chip->id == ID_ADT7516)
> > - chip->dac_bits = 12;
> > - else if (chip->id == ID_ADT7317 || chip->id == ID_ADT7517)
> > - chip->dac_bits = 10;
> > - } else {
> > + else
> > config3 = chip->config3 & (~ADT7316_DA_HIGH_RESOLUTION);
> > - }
> >
> > ret = chip->bus.write(chip->bus.client, ADT7316_CONFIG3, config3);
> > if (ret)
> > @@ -2123,6 +2116,13 @@ int adt7316_probe(struct device *dev, struct
> > adt7316_bus *bus,
> > else
> > return -ENODEV;
> >
> > + if (chip->id == ID_ADT7316 || chip->id == ID_ADT7516)
> > + chip->dac_bits = 12;
> > + else if (chip->id == ID_ADT7317 || chip->id == ID_ADT7517)
> > + chip->dac_bits = 10;
> > + else
> > + chip->dac_bits = 8;
> > +
> > chip->ldac_pin = devm_gpiod_get_optional(dev, "adi,ldac",
> > GPIOD_OUT_LOW);
> > if (IS_ERR(chip->ldac_pin)) {
> > ret = PTR_ERR(chip->ldac_pin);
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] comedi/ni_pcidio: make all defines uppercase
We agree that preserving the reverse-engineered knowledge is more important than reducing code size. In turn, we have modified the patch to keep these defines and converted them to uppercase as well. Also, we agree that further abbreviating would be a bad idea. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] comedi/ni_pcidio: make all defines uppercase
According to the Linux coding guidelines, defines should be written in uppercase. This patch converts all define-statements in the ni_pcidio.c file to uppercase, thus matching the coding style of the kernel. Signed-off-by: Alexander Schroth Signed-off-by: Marco Ammon --- drivers/staging/comedi/drivers/ni_pcidio.c | 442 +++-- 1 file changed, 222 insertions(+), 220 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_pcidio.c b/drivers/staging/comedi/drivers/ni_pcidio.c index b9a0dc6eac44..407922b096a3 100644 --- a/drivers/staging/comedi/drivers/ni_pcidio.c +++ b/drivers/staging/comedi/drivers/ni_pcidio.c @@ -49,116 +49,117 @@ /* defines for the PCI-DIO-32HS */ -#define Window_Address 4 /* W */ -#define Interrupt_And_Window_Status4 /* R */ -#define IntStatus1 BIT(0) -#define IntStatus2 BIT(1) -#define WindowAddressStatus_mask 0x7c - -#define Master_DMA_And_Interrupt_Control 5 /* W */ -#define InterruptLine(x) ((x) & 3) -#define OpenIntBIT(2) -#define Group_Status 5 /* R */ -#define DataLeft BIT(0) -#define ReqBIT(2) -#define StopTrig BIT(3) - -#define Group_1_Flags 6 /* R */ -#define Group_2_Flags 7 /* R */ -#define TransferReady BIT(0) -#define CountExpired BIT(1) -#define Waited BIT(5) -#define PrimaryTC BIT(6) -#define SecondaryTCBIT(7) +#define WINDOW_ADDRESS 4 /* W */ +#define INTERRUPT_AND_WINDOW_STATUS4 /* R */ +#define INT_STATUS_1 BIT(0) +#define INT_STATUS_2 BIT(1) +#define WINDOW_ADDRESS_STATUS_MASK 0x7c + +#define MASTER_DMA_AND_INTERRUPT_CONTROL 5 /* W */ +#define INTERRUPT_LINE(x) ((x) & 3) +#define OPEN_INT BIT(2) +#define GROUP_STATUS 5 /* R */ +#define DATA_LEFT BIT(0) +#define REQBIT(2) +#define STOP_TRIG BIT(3) + +#define GROUP_1_FLAGS 6 /* R */ +#define GROUP_2_FLAGS 7 /* R */ +#define TRANSFER_READY BIT(0) +#define COUNT_EXPIRED BIT(1) +#define WAITED BIT(5) +#define PRIMARY_TC BIT(6) +#define SECONDARY_TC BIT(7) /* #define SerialRose */ /* #define ReqRose */ /* #define Paused */ -#define Group_1_First_Clear6 /* W */ -#define Group_2_First_Clear7 /* W */ -#define ClearWaitedBIT(3) -#define ClearPrimaryTC BIT(4) -#define ClearSecondaryTC BIT(5) -#define DMAReset BIT(6) -#define FIFOReset BIT(7) -#define ClearAll 0xf8 - -#define Group_1_FIFO 8 /* W */ -#define Group_2_FIFO 12 /* W */ - -#define Transfer_Count 20 -#define Chip_ID_D 24 -#define Chip_ID_I 25 -#define Chip_ID_O 26 -#define Chip_Version 27 -#define Port_IO(x) (28 + (x)) -#define Port_Pin_Directions(x) (32 + (x)) -#define Port_Pin_Mask(x) (36 + (x)) -#define Port_Pin_Polarities(x) (40 + (x)) - -#define Master_Clock_Routing 45 -#define RTSIClocking(x)(((x) & 3) << 4) - -#define Group_1_Second_Clear 46 /* W */ -#define Group_2_Second_Clear 47 /* W */ -#define ClearExpired BIT(0) - -#define Port_Pattern(x)(48 + (x)) - -#define Data_Path 64 -#define FIFOEnableABIT(0) -#define FIFOEnableBBIT(1) -#define FIFOEnableCBIT(2) -#define FIFOEnableDBIT(3) -#define Funneling(x) (((x) & 3) << 4) -#define GroupDirection BIT(7) - -#define Protocol_Register_165 -#define OpMode Protocol_Register_1 -#define RunMode(x) ((x) & 7) -#define Numbered BIT(3) - -#define Protocol_Register_266 -#define ClockReg Protocol_Register_2 -#define ClockLine(x) (((x) & 3) << 5) -#define InvertStopTrig BIT(7) -#define DataLatching(x) (((x) & 3) << 5) - -#define Protocol_Register_367 -#define Sequence
Re: [PATCH 1/2] net: dsa: ksz9477: add I2C managed mode support
On Mon, Dec 17, 2018 at 8:11 PM Dan Carpenter wrote:
>
> On Mon, Dec 17, 2018 at 07:22:51PM +0100, Sergio Paracuellos wrote:
> > > > +static int ksz_i2c_write(struct ksz_device *dev, u32 reg, u8 *val,
> > > > + unsigned int len)
> > > > +{
> > > > + struct i2c_client *client = dev->priv;
> > > > + unsigned int cnt = len;
> > > > + int i = 0;
> > > > + u8 txb[4];
> > > > +
> > > > + do {
> > > > + txb[i++] = (u8)(*val >> (8 * (cnt - 1)));
> > > ^^^
> > >
> > > Can "cnt" be zero from ksz_i2c_set()? If so this loop will corrupt
> > > memory.
> >
> > This get and set interface seems to be introduced recently but it is
> > not being used yet, so
> > in this moment the answer is not. For me, there is no sense in call
> > 'set' with no len. Should
> > I add a check for zero len and return EINVAL just in case?
> >
>
> Yes, please.
Done. v3 with this change already sent.
>
> > > > +static int ksz_i2c_probe(struct i2c_client *client,
> > > > + const struct i2c_device_id *id)
> > > > +{
> > > > + struct ksz_device *dev;
> > > > + int ret;
> > > > +
> > > > + dev = ksz_switch_alloc(>dev, _i2c_ops, client);
> > > > + if (!dev)
> > > > + return -ENOMEM;
> > > > +
> > > > + if (client->dev.platform_data)
> > > > + dev->pdata = client->dev.platform_data;
> > > > +
> > > > + i2c_set_clientdata(client, dev);
> > > > +
> > > > + ret = ksz9477_switch_register(dev);
> > > > + if (ret) {
> > > > + dev_err(>dev, "registering switch (ret: %d)\n",
> > > > ret);
> > >
> > >
> > > free dev on this error path?
> >
> > Internally all of this are using devm_* funcions, so I think is there
> > is no need to free anything. Also, the
> > spi managed driver for this does nothing also about this.
>
> You're right. My bad. I should have looked at this.
>
> regards,
> dan carpenter
>
Best regards,
Sergio Paracuellos
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/2] dt-bindings: net: dsa: ksz9477: add sample of switch bindings managed in i2c mode
Add device-tree binding example of the ksz9477 switch managed in i2c mode.
Cc: devicet...@vger.kernel.org
Signed-off-by: Sergio Paracuellos
---
Changes v3:
- No changes. Just resent patches together.
Changes v2:
- Use generic name for label of the switch and make sure reg and @X
have the same value.
.../devicetree/bindings/net/dsa/ksz.txt | 50 +++
1 file changed, 50 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt
b/Documentation/devicetree/bindings/net/dsa/ksz.txt
index 0f407fb371ce..d3c4b9d4f416 100644
--- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
+++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
@@ -74,3 +74,53 @@ Ethernet switch connected via SPI to the host, CPU port
wired to eth0:
};
};
};
+
+Ethernet switch connected via I2C to the host, CPU port wired to eth0:
+
+ eth0: ethernet@10001000 {
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+
+ i2c0: i2c@f8008000 {
+ switch: ksz9897@5f {
+ compatible =
"microchip,ksz9897";
+ reg = <5f>;
+
+ ports {
+
#address-cells = <1>;
+
#size-cells = <0>;
+
port@0 {
+
reg = <0>;
+
label = "lan1";
+ };
+
port@1 {
+
reg = <1>;
+
label = "lan2";
+ };
+
port@2 {
+
reg = <2>;
+
label = "lan3";
+ };
+
port@3 {
+
reg = <3>;
+
label = "lan4";
+ };
+
port@4 {
+
reg = <4>;
+
label = "lan5";
+ };
+
port@6 {
+
reg = <6>;
+
label = "cpu";
+
ethernet = <>;
+
fixed-link {
+
speed = <1000>;
+
full-duplex;
+
};
+ };
+ };
+ };
+ };
--
2.19.1
Re: [PATCH] Drivers: hv: vmbus: Return -EINVAL for the sys files for unopened channels
On Mon, 17 Dec 2018 18:44:12 + Dexuan Cui wrote: > > From: devel On Behalf Of > > Dexuan Cui > > Sent: Monday, December 17, 2018 10:31 AM > > > From: Stephen Hemminger > > > > > > The old code was risky because it would silently return stack garbage. > > > Having an error check in get_debuginfo would eliminate that. > > > > OK, then let me make another patch based on the latest char-misc-linus. > > > > -- Dexuan > > Hi Stephen, your patch can apply cleanly. Let me rebase your patch to > char-misc-linus, do a test, and then post it with your Signed-off-by and > mine: > I assume you're Ok with this. Please let me know in case it's not. :-) > > Thanks, > -- Dexuan Sure. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/2] net: dsa: ksz9477: add I2C managed mode support
In this mode the switch device and the internal phys will be managed via
I2C interface.
Signed-off-by: Sergio Paracuellos
---
Changes v3:
- return -EINVAL if 'ksz_i2c_write' is called with len 0 to
avoid a possible memory corruption.
Changes v2:
- Use dev->txbuf as transmition buffer which is allocated using
kernel allocators avoiding some possible DMA issues using the
stack.
drivers/net/dsa/microchip/Kconfig | 6 +
drivers/net/dsa/microchip/Makefile | 1 +
drivers/net/dsa/microchip/ksz9477_i2c.c | 272
3 files changed, 279 insertions(+)
create mode 100644 drivers/net/dsa/microchip/ksz9477_i2c.c
diff --git a/drivers/net/dsa/microchip/Kconfig
b/drivers/net/dsa/microchip/Kconfig
index a8caf9249d50..162fec43d204 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -14,3 +14,9 @@ config NET_DSA_MICROCHIP_KSZ9477_SPI
depends on NET_DSA_MICROCHIP_KSZ9477 && SPI
help
Select to enable support for registering switches configured through
SPI.
+
+config NET_DSA_MICROCHIP_KSZ9477_I2C
+ tristate "KSZ series I2C connected switch driver"
+ depends on NET_DSA_MICROCHIP_KSZ9477 && I2C
+ help
+ Select to enable support for registering switches configured through
I2C.
diff --git a/drivers/net/dsa/microchip/Makefile
b/drivers/net/dsa/microchip/Makefile
index 3142c18b8f57..eb9b768face2 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON) += ksz_common.o
obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477)+= ksz9477.o
obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_SPI)+= ksz9477_spi.o
+obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_I2C)+= ksz9477_i2c.o
diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c
b/drivers/net/dsa/microchip/ksz9477_i2c.c
new file mode 100644
index ..52f63c0d35e7
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz9477_i2c.c
@@ -0,0 +1,272 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microchip KSZ series register access through I2C
+ *
+ * Author: Sergio Paracuellos
+ */
+
+#include
+#include
+#include
+
+#include "ksz_priv.h"
+
+/* Enough to read all switch port registers. */
+#define I2C_TX_BUF_LEN 0x100
+
+/**
+ * ksz_i2c_read_reg - issue read register command
+ * @client: i2c client structure
+ * @reg: The register address.
+ * @val: The buffer to return the result into.
+ * @len: The length of data expected.
+ *
+ * This is the low level read call that issues the necessary i2c message(s)
+ * to read data from the register specified in @reg.
+ */
+static int ksz_i2c_read_reg(struct i2c_client *client, u32 reg, u8 *val,
+ unsigned int len)
+{
+ struct i2c_adapter *adapter = client->adapter;
+ struct i2c_msg msg[2];
+ int ret;
+
+ msg[0].addr = client->addr;
+ msg[0].flags = 0;
+ msg[0].len = 2;
+ msg[0].buf = val;
+
+ msg[1].addr = client->addr;
+ msg[1].flags = I2C_M_RD;
+ msg[1].len = len;
+ msg[1].buf = val;
+
+ ret = i2c_transfer(adapter, msg, 2);
+ return (ret != 2) ? -EREMOTEIO : 0;
+}
+
+static int ksz_i2c_read(struct ksz_device *dev, u32 reg, u8 *data,
+ unsigned int len)
+{
+ struct i2c_client *client = dev->priv;
+ int ret;
+
+ len = (len > I2C_TX_BUF_LEN) ? I2C_TX_BUF_LEN : len;
+ dev->txbuf[0] = (u8)(reg >> 8);
+ dev->txbuf[1] = (u8)reg;
+
+ ret = ksz_i2c_read_reg(client, reg, dev->txbuf, len);
+ if (!ret)
+ memcpy(data, dev->txbuf, len);
+
+ return ret;
+}
+
+static int ksz_i2c_read8(struct ksz_device *dev, u32 reg, u8 *val)
+{
+ return ksz_i2c_read(dev, reg, val, 1);
+}
+
+static int ksz_i2c_read16(struct ksz_device *dev, u32 reg, u16 *val)
+{
+ int ret = ksz_i2c_read(dev, reg, (u8 *)val, 2);
+
+ if (!ret)
+ *val = be16_to_cpu(*val);
+
+ return ret;
+}
+
+static int ksz_i2c_read24(struct ksz_device *dev, u32 reg, u32 *val)
+{
+ int ret;
+
+ *val = 0;
+ ret = ksz_i2c_read(dev, reg, (u8 *)val, 3);
+
+ if (!ret) {
+ *val = be32_to_cpu(*val);
+ /* convert to 24 bit */
+ *val >>= 8;
+ }
+
+ return ret;
+}
+
+static int ksz_i2c_read32(struct ksz_device *dev, u32 reg, u32 *val)
+{
+ int ret = ksz_i2c_read(dev, reg, (u8 *)val, 4);
+
+ if (!ret)
+ *val = be32_to_cpu(*val);
+
+ return ret;
+}
+
+/**
+ * ksz_i2c_write_reg - issue write register command
+ * @client: i2c client structure
+ * @reg: The register address.
+ * @val: value to write
+ * @len: The length of data
+ *
+ * This is the low level write call that issues the necessary i2c message(s)
+ * to write data to the register specified in @reg.
+ */
+static int ksz_i2c_write_reg(struct i2c_client *client, u32 reg, u8
[PATCH] Drivers: hv: vmbus: Check for ring when getting debug info
fc96df16a1ce is good and can already fix the "return stack garbage" issue,
but let's also improve hv_ringbuffer_get_debuginfo(), which would silently
return stack garbage, if people forget to check channel->state or
ring_info->ring_buffer, when using the function in the future.
Having an error check in the function would eliminate the potential risk.
Add a Fixes tag to indicate the patch depdendency.
Fixes: fc96df16a1ce ("Drivers: hv: vmbus: Return -EINVAL for the sys files for
unopened channels")
Cc: sta...@vger.kernel.org
Cc: K. Y. Srinivasan
Cc: Haiyang Zhang
Signed-off-by: Stephen Hemminger
Signed-off-by: Dexuan Cui
---
*NOTE*: the patch is based on char-misc's char-misc-linus branch.
drivers/hv/ring_buffer.c | 31 -
drivers/hv/vmbus_drv.c | 91
include/linux/hyperv.h | 5 +--
3 files changed, 79 insertions(+), 48 deletions(-)
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 64d0c85..1f1a55e 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -164,26 +164,25 @@ hv_get_ringbuffer_availbytes(const struct
hv_ring_buffer_info *rbi,
}
/* Get various debug metrics for the specified ring buffer. */
-void hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,
-struct hv_ring_buffer_debug_info *debug_info)
+int hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,
+ struct hv_ring_buffer_debug_info *debug_info)
{
u32 bytes_avail_towrite;
u32 bytes_avail_toread;
- if (ring_info->ring_buffer) {
- hv_get_ringbuffer_availbytes(ring_info,
- _avail_toread,
- _avail_towrite);
-
- debug_info->bytes_avail_toread = bytes_avail_toread;
- debug_info->bytes_avail_towrite = bytes_avail_towrite;
- debug_info->current_read_index =
- ring_info->ring_buffer->read_index;
- debug_info->current_write_index =
- ring_info->ring_buffer->write_index;
- debug_info->current_interrupt_mask =
- ring_info->ring_buffer->interrupt_mask;
- }
+ if (!ring_info->ring_buffer)
+ return -EINVAL;
+
+ hv_get_ringbuffer_availbytes(ring_info,
+_avail_toread,
+_avail_towrite);
+ debug_info->bytes_avail_toread = bytes_avail_toread;
+ debug_info->bytes_avail_towrite = bytes_avail_towrite;
+ debug_info->current_read_index = ring_info->ring_buffer->read_index;
+ debug_info->current_write_index = ring_info->ring_buffer->write_index;
+ debug_info->current_interrupt_mask
+ = ring_info->ring_buffer->interrupt_mask;
+ return 0;
}
EXPORT_SYMBOL_GPL(hv_ringbuffer_get_debuginfo);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index d0ff656..403fee0 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -313,12 +313,16 @@ static ssize_t out_intr_mask_show(struct device *dev,
{
struct hv_device *hv_dev = device_to_hv_device(dev);
struct hv_ring_buffer_debug_info outbound;
+ int ret;
if (!hv_dev->channel)
return -ENODEV;
- if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
- return -EINVAL;
- hv_ringbuffer_get_debuginfo(_dev->channel->outbound, );
+
+ ret = hv_ringbuffer_get_debuginfo(_dev->channel->outbound,
+ );
+ if (ret < 0)
+ return ret;
+
return sprintf(buf, "%d\n", outbound.current_interrupt_mask);
}
static DEVICE_ATTR_RO(out_intr_mask);
@@ -328,12 +332,15 @@ static ssize_t out_read_index_show(struct device *dev,
{
struct hv_device *hv_dev = device_to_hv_device(dev);
struct hv_ring_buffer_debug_info outbound;
+ int ret;
if (!hv_dev->channel)
return -ENODEV;
- if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
- return -EINVAL;
- hv_ringbuffer_get_debuginfo(_dev->channel->outbound, );
+
+ ret = hv_ringbuffer_get_debuginfo(_dev->channel->outbound,
+ );
+ if (ret < 0)
+ return ret;
return sprintf(buf, "%d\n", outbound.current_read_index);
}
static DEVICE_ATTR_RO(out_read_index);
@@ -344,12 +351,15 @@ static ssize_t out_write_index_show(struct device *dev,
{
struct hv_device *hv_dev = device_to_hv_device(dev);
struct hv_ring_buffer_debug_info outbound;
+ int ret;
if (!hv_dev->channel)
return -ENODEV;
- if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
- return -EINVAL;
- hv_ringbuffer_get_debuginfo(_dev->channel->outbound, );
+
+
Re: [PATCH 2/2] Staging: nvec: nvec: fixed check style issues
Hi,
On Mon, 17 Dec 2018, Greg KH wrote:
On Sun, Dec 16, 2018 at 08:57:43AM -0800, Amir Mahdi Ghorbanian wrote:
@@ -626,7 +628,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
break;
case 2: /* first byte after command */
if (status == (I2C_SL_IRQ | RNW | RCVD)) {
- udelay(33);
+ usleep_range(0, 33);
Why is this a valid range to sleep for for this device? Have you been
able to verify/test this?
oh no, not again. Why does this come up again every half year? This udelay
is a workaround for a hw bug which only seldom triggers (if it triggers at
all). Secondly, this is in interrupt context, so *sleep timers are no go,
afaik.
Marc
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] net: dsa: ksz9477: add I2C managed mode support
On Mon, Dec 17, 2018 at 07:22:51PM +0100, Sergio Paracuellos wrote:
> > > +static int ksz_i2c_write(struct ksz_device *dev, u32 reg, u8 *val,
> > > + unsigned int len)
> > > +{
> > > + struct i2c_client *client = dev->priv;
> > > + unsigned int cnt = len;
> > > + int i = 0;
> > > + u8 txb[4];
> > > +
> > > + do {
> > > + txb[i++] = (u8)(*val >> (8 * (cnt - 1)));
> > ^^^
> >
> > Can "cnt" be zero from ksz_i2c_set()? If so this loop will corrupt
> > memory.
>
> This get and set interface seems to be introduced recently but it is
> not being used yet, so
> in this moment the answer is not. For me, there is no sense in call
> 'set' with no len. Should
> I add a check for zero len and return EINVAL just in case?
>
Yes, please.
> > > +static int ksz_i2c_probe(struct i2c_client *client,
> > > + const struct i2c_device_id *id)
> > > +{
> > > + struct ksz_device *dev;
> > > + int ret;
> > > +
> > > + dev = ksz_switch_alloc(>dev, _i2c_ops, client);
> > > + if (!dev)
> > > + return -ENOMEM;
> > > +
> > > + if (client->dev.platform_data)
> > > + dev->pdata = client->dev.platform_data;
> > > +
> > > + i2c_set_clientdata(client, dev);
> > > +
> > > + ret = ksz9477_switch_register(dev);
> > > + if (ret) {
> > > + dev_err(>dev, "registering switch (ret: %d)\n",
> > > ret);
> >
> >
> > free dev on this error path?
>
> Internally all of this are using devm_* funcions, so I think is there
> is no need to free anything. Also, the
> spi managed driver for this does nothing also about this.
You're right. My bad. I should have looked at this.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Drivers: hv: vmbus: Return -EINVAL for the sys files for unopened channels
> From: devel On Behalf Of > Dexuan Cui > Sent: Monday, December 17, 2018 10:31 AM > > From: Stephen Hemminger > > > > The old code was risky because it would silently return stack garbage. > > Having an error check in get_debuginfo would eliminate that. > > OK, then let me make another patch based on the latest char-misc-linus. > > -- Dexuan Hi Stephen, your patch can apply cleanly. Let me rebase your patch to char-misc-linus, do a test, and then post it with your Signed-off-by and mine: I assume you're Ok with this. Please let me know in case it's not. :-) Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: android: ion: Add implementation of dma_buf_vmap and dma_buf_vunmap
On Sun, 16 Dec 2018, Alexey Skidanov wrote: > > > On 12/16/18 7:20 AM, Liam Mark wrote: > > On Tue, 6 Feb 2018, Alexey Skidanov wrote: > > > >> > >> > >> On 02/07/2018 01:56 AM, Laura Abbott wrote: > >>> On 01/31/2018 10:10 PM, Alexey Skidanov wrote: > > On 01/31/2018 03:00 PM, Greg KH wrote: > > On Wed, Jan 31, 2018 at 02:03:42PM +0200, Alexey Skidanov wrote: > >> Any driver may access shared buffers, created by ion, using > >> dma_buf_vmap and > >> dma_buf_vunmap dma-buf API that maps/unmaps previosuly allocated > >> buffers into > >> the kernel virtual address space. The implementation of these API is > >> missing in > >> the current ion implementation. > >> > >> Signed-off-by: Alexey Skidanov > >> --- > > > > No review from any other Intel developers? :( > Will add. > > > > Anyway, what in-tree driver needs access to these functions? > I'm not sure that there are the in-tree drivers using these functions > and ion as> buffer exporter because they are not implemented in ion :) > But there are some in-tre> drivers using these APIs (gpu drivers) with > other buffer exporters. > >>> > >>> It's still not clear why you need to implement these APIs. > >> How the importing kernel module may access the content of the buffer? :) > >> With the current ion implementation it's only possible by dma_buf_kmap, > >> mapping one page at a time. For pretty large buffers, it might have some > >> performance impact. > >> (Probably, the page by page mapping is the only way to access large > >> buffers on 32 bit systems, where the vmalloc range is very small. By the > >> way, the current ion dma_map_kmap doesn't really map only 1 page at a > >> time - it uses the result of vmap() that might fail on 32 bit systems.) > >> > >>> Are you planning to use Ion with GPU drivers? I'm especially > >>> interested in this if you have a non-Android use case. > >> Yes, my use case is the non-Android one. But not with GPU drivers. > >>> > >>> Thanks, > >>> Laura > >> > >> Thanks, > >> Alexey > > > > I was wondering if we could re-open the discussion on adding support to > > ION for dma_buf_vmap. > > It seems like the patch was not taken as the reviewers wanted more > > evidence of an upstream use case. > > > > Here would be my upstream usage argument for including dma_buf_vmap > > support in ION. > > > > Currently all calls to ion_dma_buf_begin_cpu_access result in the creation > > of a kernel mapping for the buffer, unfortunately the resulting call to > > alloc_vmap_area can be quite expensive and this has caused a performance > > regression for certain clients when they have moved to the new version of > > ION. > > > > The kernel mapping is not actually needed in ion_dma_buf_begin_cpu_access, > > and generally isn't needed by clients. So if we remove the creation of the > > kernel mapping in ion_dma_buf_begin_cpu_access and only create it when > > needed we can speed up the calls to ion_dma_buf_begin_cpu_access. > > > > An additional benefit of removing the creation of kernel mappings from > > ion_dma_buf_begin_cpu_access is that it makes the ION code more secure. > > Currently a malicious client could call the DMA_BUF_IOCTL_SYNC IOCTL with > > flags DMA_BUF_SYNC_END multiple times to cause the ION buffer kmap_cnt to > > go negative which could lead to undesired behavior. > > > > One disadvantage of the above change is that a kernel mapping is not > > already created when a client calls dma_buf_kmap. So the following > > dma_buf_kmap contract can't be satisfied. > > > > /** > > * dma_buf_kmap - Map a page of the buffer object into kernel address > > space. The > > * same restrictions as for kmap and friends apply. > > * @dmabuf: [in]buffer to map page from. > > * @page_num:[in]page in PAGE_SIZE units to map. > > * > > * This call must always succeed, any necessary preparations that might > > fail > > * need to be done in begin_cpu_access. > > */ > > > > But hopefully we can work around this by moving clients to dma_buf_vmap. > I think the problem is with the contract. We can't ensure that the call > is always succeeds regardless the implementation - any mapping might > fail. Probably this is why *all* clients of dma_buf_kmap() check the > return value (so it's safe to return NULL in case of failure). > I think currently the call to dma_buf_kmap will always succeed since the DMA-Buf contract requires that the client first successfully call dma_buf_begin_cpu_access(), and if dma_buf_begin_cpu_access() succeeds then dma_buf_kmap will succeed. > I would suggest to fix the contract and to keep the dma_buf_kmap() > support in ION. I will leave it to the DMA-Buf maintainers as to whether they want to change their contract. Liam Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ devel
RE: [PATCH] Drivers: hv: vmbus: Return -EINVAL for the sys files for unopened channels
> From: Stephen Hemminger > Sent: Monday, December 17, 2018 10:17 AM > To: Dexuan Cui > > On Mon, 17 Dec 2018 18:00:29 + > Dexuan Cui wrote: > > > > From: Stephen Hemminger > > > On Thu, 13 Dec 2018 16:35:43 + > > > Dexuan Cui wrote: > > > > > > > Before 98f4c651762c, we returned zeros for unopened channels. > > > > With 98f4c651762c, we started to return random on-stack values. > > > > > > > > We'd better return -EINVAL instead. > > > > > > The concept looks fine, but maybe it would be simpler to move it into > > > hv_ringbuffer_get_debuginfo and have it return an error code. > > > > > > Since so much of the code is repeated, I would probably make a > > > macro which generates the code as well. > > > > > > Something like this: > > > > Thanks, Stephen! Now the patch has been in char-misc's char-misc-linus > > branch, so IMO we may as well leave it as is (considering the code here is > > unlikely to be frqeuencly changed), and we have a smaller patch this way. > > :-) > > > > But, yes, I agree with you that generally we should make a common > > function to avoid duplicate code. > > > > Thanks, > > -- Dexuan > > The old code was risky because it would silently return stack garbage. > Having an error check in get_debuginfo would eliminate that. OK, then let me make another patch based on the latest char-misc-linus. Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] net: dsa: ksz9477: add I2C managed mode support
On Mon, Dec 17, 2018 at 8:38 AM Andrew Lunn wrote:
>
> On Sun, Dec 16, 2018 at 10:49:52AM +0100, Sergio Paracuellos wrote:
> > In this mode the switch device and the internal phys will be managed via
> > I2C interface.
> >
> > Signed-off-by: Sergio Paracuellos
> > ---
> > Changes v2:
> > - Use dev->txbuf as transmition buffer which is allocated using
> > kernel allocators avoiding some possible DMA issues using the
> > stack.
> > drivers/net/dsa/microchip/Kconfig | 6 +
> > drivers/net/dsa/microchip/Makefile | 1 +
> > drivers/net/dsa/microchip/ksz9477_i2c.c | 269
> > 3 files changed, 276 insertions(+)
> > create mode 100644 drivers/net/dsa/microchip/ksz9477_i2c.c
> >
> > diff --git a/drivers/net/dsa/microchip/Kconfig
> > b/drivers/net/dsa/microchip/Kconfig
> > index a8caf9249d50..162fec43d204 100644
> > --- a/drivers/net/dsa/microchip/Kconfig
> > +++ b/drivers/net/dsa/microchip/Kconfig
> > @@ -14,3 +14,9 @@ config NET_DSA_MICROCHIP_KSZ9477_SPI
> > depends on NET_DSA_MICROCHIP_KSZ9477 && SPI
> > help
> > Select to enable support for registering switches configured
> > through SPI.
> > +
> > +config NET_DSA_MICROCHIP_KSZ9477_I2C
> > + tristate "KSZ series I2C connected switch driver"
> > + depends on NET_DSA_MICROCHIP_KSZ9477 && I2C
> > + help
> > + Select to enable support for registering switches configured
> > through I2C.
> > diff --git a/drivers/net/dsa/microchip/Makefile
> > b/drivers/net/dsa/microchip/Makefile
> > index 3142c18b8f57..eb9b768face2 100644
> > --- a/drivers/net/dsa/microchip/Makefile
> > +++ b/drivers/net/dsa/microchip/Makefile
> > @@ -1,3 +1,4 @@
> > obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON) += ksz_common.o
> > obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477) += ksz9477.o
> > obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_SPI) += ksz9477_spi.o
> > +obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_I2C) += ksz9477_i2c.o
> > diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c
> > b/drivers/net/dsa/microchip/ksz9477_i2c.c
> > new file mode 100644
> > index ..3c9cba6e254b
> > --- /dev/null
> > +++ b/drivers/net/dsa/microchip/ksz9477_i2c.c
> > @@ -0,0 +1,269 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Microchip KSZ series register access through I2C
> > + *
> > + * Author: Sergio Paracuellos
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +
> > +#include "ksz_priv.h"
> > +
> > +/* Enough to read all switch port registers. */
> > +#define I2C_TX_BUF_LEN 0x100
> > +
> > +/**
> > + * ksz_i2c_read_reg - issue read register command
> > + * @client: i2c client structure
> > + * @reg: The register address.
> > + * @val: The buffer to return the result into.
> > + * @len: The length of data expected.
> > + *
> > + * This is the low level read call that issues the necessary i2c message(s)
> > + * to read data from the register specified in @reg.
> > + */
> > +static int ksz_i2c_read_reg(struct i2c_client *client, u32 reg, u8 *val,
> > + unsigned int len)
> > +{
> > + struct i2c_adapter *adapter = client->adapter;
> > + struct i2c_msg msg[2];
> > + int ret;
> > +
> > + msg[0].addr = client->addr;
> > + msg[0].flags = 0;
> > + msg[0].len = 2;
> > + msg[0].buf = val;
> > +
> > + msg[1].addr = client->addr;
> > + msg[1].flags = I2C_M_RD;
> > + msg[1].len = len;
> > + msg[1].buf = val;
> > +
> > + ret = i2c_transfer(adapter, msg, 2);
> > + return (ret != 2) ? -EREMOTEIO : 0;
> > +}
> > +
> > +static int ksz_i2c_read(struct ksz_device *dev, u32 reg, u8 *data,
> > + unsigned int len)
> > +{
> > + struct i2c_client *client = dev->priv;
> > + int ret;
> > +
> > + len = (len > I2C_TX_BUF_LEN) ? I2C_TX_BUF_LEN : len;
> > + dev->txbuf[0] = (u8)(reg >> 8);
> > + dev->txbuf[1] = (u8)reg;
> > +
> > + ret = ksz_i2c_read_reg(client, reg, dev->txbuf, len);
> > + if (!ret)
> > + memcpy(data, dev->txbuf, len);
> > +
> > + return ret;
> > +}
> > +
> > +static int ksz_i2c_read8(struct ksz_device *dev, u32 reg, u8 *val)
> > +{
> > + return ksz_i2c_read(dev, reg, val, 1);
> > +}
> > +
> > +static int ksz_i2c_read16(struct ksz_device *dev, u32 reg, u16 *val)
> > +{
> > + int ret = ksz_i2c_read(dev, reg, (u8 *)val, 2);
> > +
> > + if (!ret)
> > + *val = be16_to_cpu(*val);
> > +
> > + return ret;
> > +}
> > +
> > +static int ksz_i2c_read24(struct ksz_device *dev, u32 reg, u32 *val)
> > +{
> > + int ret;
> > +
> > + *val = 0;
> > + ret = ksz_i2c_read(dev, reg, (u8 *)val, 3);
> > +
> > + if (!ret) {
> > + *val = be32_to_cpu(*val);
> > + /* convert to 24 bit */
> > + *val >>= 8;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int ksz_i2c_read32(struct ksz_device *dev, u32 reg, u32 *val)
> > +{
> > + int ret = ksz_i2c_read(dev, reg,
Re: [PATCH 1/2] net: dsa: ksz9477: add I2C managed mode support
Hi Dan,
Thanks for the feedback.
On Mon, Dec 17, 2018 at 7:55 AM Dan Carpenter wrote:
>
> On Sun, Dec 16, 2018 at 08:57:40AM +0100, Sergio Paracuellos wrote:
> > +static int ksz_i2c_write(struct ksz_device *dev, u32 reg, u8 *val,
> > + unsigned int len)
> > +{
> > + struct i2c_client *client = dev->priv;
> > + unsigned int cnt = len;
> > + int i = 0;
> > + u8 txb[4];
> > +
> > + do {
> > + txb[i++] = (u8)(*val >> (8 * (cnt - 1)));
> ^^^
>
> Can "cnt" be zero from ksz_i2c_set()? If so this loop will corrupt
> memory.
This get and set interface seems to be introduced recently but it is
not being used yet, so
in this moment the answer is not. For me, there is no sense in call
'set' with no len. Should
I add a check for zero len and return EINVAL just in case?
>
> > + cnt--;
> > + } while (cnt);
> > +
> > + return ksz_i2c_write_reg(client, reg, txb, len);
> > +}
> > +
> > +static int ksz_i2c_write8(struct ksz_device *dev, u32 reg, u8 value)
> > +{
> > + return ksz_i2c_write(dev, reg, , 1);
> > +}
> > +
> > +static int ksz_i2c_write16(struct ksz_device *dev, u32 reg, u16 value)
> > +{
> > + value = cpu_to_be16(value);
> > + return ksz_i2c_write(dev, reg, (u8 *), 2);
> > +}
> > +
> > +static int ksz_i2c_write24(struct ksz_device *dev, u32 reg, u32 value)
> > +{
> > + /* make it to big endian 24bit from MSB */
> > + value <<= 8;
> > + value = cpu_to_be32(value);
> > +
> > + return ksz_i2c_write(dev, reg, (u8 *), 3);
> > +}
> > +
> > +static int ksz_i2c_write32(struct ksz_device *dev, u32 reg, u32 value)
> > +{
> > + value = cpu_to_be32(value);
> > + return ksz_i2c_write(dev, reg, (u8 *), 4);
> > +}
> > +
> > +static int ksz_i2c_get(struct ksz_device *dev, u32 reg, void *data, size_t
> > len)
> > +{
> > + return ksz_i2c_read(dev, reg, data, len);
> > +}
> > +
> > +static int ksz_i2c_set(struct ksz_device *dev, u32 reg, void *data, size_t
> > len)
> > +{
> > + return ksz_i2c_write(dev, reg, data, len);
> > +}
> > +
> > +static const struct ksz_io_ops ksz_i2c_ops = {
> > + .read8 = ksz_i2c_read8,
> > + .read16 = ksz_i2c_read16,
> > + .read24 = ksz_i2c_read24,
> > + .read32 = ksz_i2c_read32,
> > + .write8 = ksz_i2c_write8,
> > + .write16 = ksz_i2c_write16,
> > + .write24 = ksz_i2c_write24,
> > + .write32 = ksz_i2c_write32,
> > + .get = ksz_i2c_get,
> > + .set = ksz_i2c_set,
> > +};
> > +
> > +static int ksz_i2c_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct ksz_device *dev;
> > + int ret;
> > +
> > + dev = ksz_switch_alloc(>dev, _i2c_ops, client);
> > + if (!dev)
> > + return -ENOMEM;
> > +
> > + if (client->dev.platform_data)
> > + dev->pdata = client->dev.platform_data;
> > +
> > + i2c_set_clientdata(client, dev);
> > +
> > + ret = ksz9477_switch_register(dev);
> > + if (ret) {
> > + dev_err(>dev, "registering switch (ret: %d)\n", ret);
>
>
> free dev on this error path?
Internally all of this are using devm_* funcions, so I think is there
is no need to free anything. Also, the
spi managed driver for this does nothing also about this.
>
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
>
> regards,
> dan carpenter
Best regards,
Sergio Paracuellos
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Drivers: hv: vmbus: Return -EINVAL for the sys files for unopened channels
On Mon, 17 Dec 2018 18:00:29 + Dexuan Cui wrote: > > From: Stephen Hemminger > > On Thu, 13 Dec 2018 16:35:43 + > > Dexuan Cui wrote: > > > > > Before 98f4c651762c, we returned zeros for unopened channels. > > > With 98f4c651762c, we started to return random on-stack values. > > > > > > We'd better return -EINVAL instead. > > > > The concept looks fine, but maybe it would be simpler to move it into > > hv_ringbuffer_get_debuginfo and have it return an error code. > > > > Since so much of the code is repeated, I would probably make a > > macro which generates the code as well. > > > > Something like this: > > Thanks, Stephen! Now the patch has been in char-misc's char-misc-linus > branch, so IMO we may as well leave it as is (considering the code here is > unlikely to be frqeuencly changed), and we have a smaller patch this way. :-) > > But, yes, I agree with you that generally we should make a common > function to avoid duplicate code. > > Thanks, > -- Dexuan The old code was risky because it would silently return stack garbage. Having an error check in get_debuginfo would eliminate that. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Drivers: hv: vmbus: Return -EINVAL for the sys files for unopened channels
> From: Stephen Hemminger > On Thu, 13 Dec 2018 16:35:43 + > Dexuan Cui wrote: > > > Before 98f4c651762c, we returned zeros for unopened channels. > > With 98f4c651762c, we started to return random on-stack values. > > > > We'd better return -EINVAL instead. > > The concept looks fine, but maybe it would be simpler to move it into > hv_ringbuffer_get_debuginfo and have it return an error code. > > Since so much of the code is repeated, I would probably make a > macro which generates the code as well. > > Something like this: Thanks, Stephen! Now the patch has been in char-misc's char-misc-linus branch, so IMO we may as well leave it as is (considering the code here is unlikely to be frqeuencly changed), and we have a smaller patch this way. :-) But, yes, I agree with you that generally we should make a common function to avoid duplicate code. Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Drivers: hv: vmbus: Return -EINVAL for the sys files for unopened channels
On Thu, 13 Dec 2018 16:35:43 +
Dexuan Cui wrote:
> Before 98f4c651762c, we returned zeros for unopened channels.
> With 98f4c651762c, we started to return random on-stack values.
>
> We'd better return -EINVAL instead.
>
> Fixes: 98f4c651762c ("hv: move ringbuffer bus attributes to dev_groups")
> Cc: sta...@vger.kernel.org
> Cc: K. Y. Srinivasan
> Cc: Haiyang Zhang
> Cc: Stephen Hemminger
> Signed-off-by: Dexuan Cui
The concept looks fine, but maybe it would be simpler to move it into
hv_ringbuffer_get_debuginfo and have it return an error code.
Since so much of the code is repeated, I would probably make a
macro which generates the code as well.
Something like this:
>From c6bbdbcde933c85098f7b3e71650a8479d52810c Mon Sep 17 00:00:00 2001
From: Stephen Hemminger
Date: Mon, 17 Dec 2018 09:13:24 -0800
Subject: [PATCH] hv: vmbus: check for ring in debug info
---
drivers/hv/ring_buffer.c | 31 +-
drivers/hv/vmbus_drv.c | 71 ++--
include/linux/hyperv.h | 5 +--
3 files changed, 79 insertions(+), 28 deletions(-)
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 64d0c85d5161..1f1a55e07733 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -164,26 +164,25 @@ hv_get_ringbuffer_availbytes(const struct
hv_ring_buffer_info *rbi,
}
/* Get various debug metrics for the specified ring buffer. */
-void hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,
-struct hv_ring_buffer_debug_info *debug_info)
+int hv_ringbuffer_get_debuginfo(const struct hv_ring_buffer_info *ring_info,
+ struct hv_ring_buffer_debug_info *debug_info)
{
u32 bytes_avail_towrite;
u32 bytes_avail_toread;
- if (ring_info->ring_buffer) {
- hv_get_ringbuffer_availbytes(ring_info,
- _avail_toread,
- _avail_towrite);
-
- debug_info->bytes_avail_toread = bytes_avail_toread;
- debug_info->bytes_avail_towrite = bytes_avail_towrite;
- debug_info->current_read_index =
- ring_info->ring_buffer->read_index;
- debug_info->current_write_index =
- ring_info->ring_buffer->write_index;
- debug_info->current_interrupt_mask =
- ring_info->ring_buffer->interrupt_mask;
- }
+ if (!ring_info->ring_buffer)
+ return -EINVAL;
+
+ hv_get_ringbuffer_availbytes(ring_info,
+_avail_toread,
+_avail_towrite);
+ debug_info->bytes_avail_toread = bytes_avail_toread;
+ debug_info->bytes_avail_towrite = bytes_avail_towrite;
+ debug_info->current_read_index = ring_info->ring_buffer->read_index;
+ debug_info->current_write_index = ring_info->ring_buffer->write_index;
+ debug_info->current_interrupt_mask
+ = ring_info->ring_buffer->interrupt_mask;
+ return 0;
}
EXPORT_SYMBOL_GPL(hv_ringbuffer_get_debuginfo);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 283d184280af..403fee01572c 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -313,10 +313,16 @@ static ssize_t out_intr_mask_show(struct device *dev,
{
struct hv_device *hv_dev = device_to_hv_device(dev);
struct hv_ring_buffer_debug_info outbound;
+ int ret;
if (!hv_dev->channel)
return -ENODEV;
- hv_ringbuffer_get_debuginfo(_dev->channel->outbound, );
+
+ ret = hv_ringbuffer_get_debuginfo(_dev->channel->outbound,
+ );
+ if (ret < 0)
+ return ret;
+
return sprintf(buf, "%d\n", outbound.current_interrupt_mask);
}
static DEVICE_ATTR_RO(out_intr_mask);
@@ -326,10 +332,15 @@ static ssize_t out_read_index_show(struct device *dev,
{
struct hv_device *hv_dev = device_to_hv_device(dev);
struct hv_ring_buffer_debug_info outbound;
+ int ret;
if (!hv_dev->channel)
return -ENODEV;
- hv_ringbuffer_get_debuginfo(_dev->channel->outbound, );
+
+ ret = hv_ringbuffer_get_debuginfo(_dev->channel->outbound,
+ );
+ if (ret < 0)
+ return ret;
return sprintf(buf, "%d\n", outbound.current_read_index);
}
static DEVICE_ATTR_RO(out_read_index);
@@ -340,10 +351,15 @@ static ssize_t out_write_index_show(struct device *dev,
{
struct hv_device *hv_dev = device_to_hv_device(dev);
struct hv_ring_buffer_debug_info outbound;
+ int ret;
if (!hv_dev->channel)
return -ENODEV;
- hv_ringbuffer_get_debuginfo(_dev->channel->outbound, );
+
+ ret = hv_ringbuffer_get_debuginfo(_dev->channel->outbound,
+
Re: [PATCH v4 0/6] staging: most: sound: change sound card layout
On Mon, 2018-12-17 at 17:16 +0300, Dan Carpenter wrote: > Thanks! > My fault. :) > Reviewed-by: Dan Carpenter > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 0/6] staging: most: sound: change sound card layout
Thanks! Reviewed-by: Dan Carpenter regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 3/6] staging: most: sound: rename variable
Since the channels of a MOST device are now being represented as
individual PCM devices of one sound card, the variable card_name is not
suitable anymore to describe them. Therefore, this patch renames the
variable to device_name.
Signed-off-by: Christian Gromm
---
v2:
Reported-by: Dan Carpenter
- remove call to snprintf to store PCM name
v3:
nothing
v4:
nothing
drivers/staging/most/sound/sound.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/most/sound/sound.c
b/drivers/staging/most/sound/sound.c
index 0e9377f..6a453d7 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -471,14 +471,14 @@ static const struct snd_pcm_ops pcm_ops = {
.page = snd_pcm_lib_get_vmalloc_page,
};
-static int split_arg_list(char *buf, char **card_name, u16 *ch_num,
+static int split_arg_list(char *buf, char **device_name, u16 *ch_num,
char **sample_res, u8 *create)
{
char *num;
int ret;
- *card_name = strsep(, ".");
- if (!*card_name) {
+ *device_name = strsep(, ".");
+ if (!*device_name) {
pr_err("Missing sound card name\n");
return -EIO;
}
@@ -588,7 +588,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
int capture_count = 0;
int ret;
int direction;
- char *card_name;
+ char *device_name;
u16 ch_num;
u8 create = 0;
char *sample_res;
@@ -601,7 +601,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
return -EINVAL;
}
- ret = split_arg_list(arg_list, _name, _num, _res,
+ ret = split_arg_list(arg_list, _name, _num, _res,
);
if (ret < 0)
return ret;
@@ -622,7 +622,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
INIT_LIST_HEAD(>dev_list);
iface->priv = adpt;
list_add_tail(>list, _list);
- ret = snd_card_new(>dev, -1, card_name, THIS_MODULE,
+ ret = snd_card_new(>dev, -1, device_name, THIS_MODULE,
sizeof(*channel), >card);
if (ret < 0)
goto err_free_adpt;
@@ -664,14 +664,14 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
if (ret)
goto err_free_adpt;
- ret = snd_pcm_new(adpt->card, card_name, adpt->pcm_dev_idx,
+ ret = snd_pcm_new(adpt->card, device_name, adpt->pcm_dev_idx,
playback_count, capture_count, );
if (ret < 0)
goto err_free_adpt;
pcm->private_data = channel;
- snprintf(pcm->name, sizeof(pcm->name), card_name);
+ snprintf(pcm->name, sizeof(pcm->name), device_name);
snd_pcm_set_ops(pcm, direction, _ops);
if (create) {
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 4/6] staging: most: sound: use static name for ALSA card
This patch uses a static name for the sound card's short name and long name. Having the card names configurable doesn't make sense anymore, as the card represents the same physical hardware. Signed-off-by: Christian Gromm --- v2: nothing v3: nothing v4: nothing drivers/staging/most/sound/sound.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c index 6a453d7..c698631 100644 --- a/drivers/staging/most/sound/sound.c +++ b/drivers/staging/most/sound/sound.c @@ -622,14 +622,14 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id, INIT_LIST_HEAD(>dev_list); iface->priv = adpt; list_add_tail(>list, _list); - ret = snd_card_new(>dev, -1, device_name, THIS_MODULE, + ret = snd_card_new(>dev, -1, "INIC", THIS_MODULE, sizeof(*channel), >card); if (ret < 0) goto err_free_adpt; snprintf(adpt->card->driver, sizeof(adpt->card->driver), "%s", DRIVER_NAME); snprintf(adpt->card->shortname, sizeof(adpt->card->shortname), -"Microchip MOST:%d", adpt->card->number); +"Microchip INIC"); snprintf(adpt->card->longname, sizeof(adpt->card->longname), "%s at %s, ch %d", adpt->card->shortname, iface->description, channel_id); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 6/6] staging: most: Documentation: add information to driver_usage file
This patch updates driver_usage.txt file to reflect the latest changes that this patch set introduces. Signed-off-by: Christian Gromm --- v2: nothing v3: nothing v4: nothing drivers/staging/most/Documentation/driver_usage.txt | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/staging/most/Documentation/driver_usage.txt b/drivers/staging/most/Documentation/driver_usage.txt index bb9b4e8..da7a8f4 100644 --- a/drivers/staging/most/Documentation/driver_usage.txt +++ b/drivers/staging/most/Documentation/driver_usage.txt @@ -142,8 +142,9 @@ Cdev component example: Sound component example: -The sound component needs an additional parameter to determine the audio -resolution that is going to be used. The following formats are available: +The sound component needs additional parameters to determine the audio +resolution that is going to be used and to trigger the registration of a +sound card with ALSA. The following audio formats are available: - "1x8" (Mono) - "2x16" (16-bit stereo) @@ -151,9 +152,18 @@ resolution that is going to be used. The following formats are available: - "2x32" (32-bit stereo) - "6x16" (16-bit surround 5.1) -$ echo "mdev0:ep_81:sound:most51_playback.6x16" >$(DRV_DIR)/add_link +To make the sound module create a sound card and register it with ALSA the +string "create" needs to be attached to the module parameter section of the +configuration string. To create a sound card with with two playback devices +(linked to channel ep01 and ep02) and one capture device (linked to channel +ep83) the following is written to the add_link file: +$ echo "mdev0:ep01:sound:most51_playback.6x16" >$(DRV_DIR)/add_link +$ echo "mdev0:ep02:sound:most_playback.2x16" >$(DRV_DIR)/add_link +$ echo "mdev0:ep83:sound:most_capture.2x16.create" >$(DRV_DIR)/add_link +The link names (most51_playback, most_playback and most_capture) will +become the names of the PCM devices of the sound card. Section 2.3 USB Padding -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 5/6] staging: most: sound: remove channel number from ALSA card's long name
Adding the channel number to the name of the sound card is wrong,
as the card does not represent a single streaming channel of the
MOST device.
Signed-off-by: Christian Gromm
---
v2:
nothing
v3:
nothing
v4:
nothing
drivers/staging/most/sound/sound.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/most/sound/sound.c
b/drivers/staging/most/sound/sound.c
index c698631..29a4c72 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -631,8 +631,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
snprintf(adpt->card->shortname, sizeof(adpt->card->shortname),
"Microchip INIC");
snprintf(adpt->card->longname, sizeof(adpt->card->longname),
-"%s at %s, ch %d", adpt->card->shortname, iface->description,
-channel_id);
+"%s at %s", adpt->card->shortname, iface->description);
skip_adpt_alloc:
if (get_channel(iface, channel_id)) {
pr_err("channel (%s:%d) is already linked\n",
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 1/6] staging: most: sound: create one sound card w/ multiple PCM devices per MOST device
This patch avoids that a sound card is created and registered with ALSA
every time a channel is being linked. Instead the channels are hooked on
the same card, which is registered not until the final link has been added
to the component. The string provided by user space that used to be the
card name becomes the PCM device name. The user space API to add a link is
being expanded by a "create" flag to trigger the registration.
Signed-off-by: Christian Gromm
---
v2:
- push audio apdaper to list right afer its initialization
Reported-by: Dan Carpenter
- mention user space API change in description
- assign value to variable ret before jumping to error handling
- remove unnecessary variable initializations
- fix if statements when iterating audio adapter list
- use correct variable in sizeof operator
- store PCM device name
v3:
- jump to error handling in case snd_card_new() fails
v4:
- test pointer to sound card before calling snd_card_free()
drivers/staging/most/sound/sound.c | 130 +
1 file changed, 89 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/most/sound/sound.c
b/drivers/staging/most/sound/sound.c
index 89b02fc..9f84808 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -20,7 +21,6 @@
#define DRIVER_NAME "sound"
-static struct list_head dev_list;
static struct core_component comp;
/**
@@ -56,6 +56,17 @@ struct channel {
void (*copy_fn)(void *alsa, void *most, unsigned int bytes);
};
+struct sound_adapter {
+ struct list_head dev_list;
+ struct most_interface *iface;
+ struct snd_card *card;
+ struct list_head list;
+ bool registered;
+ int pcm_dev_idx;
+};
+
+static struct list_head adpt_list;
+
#define MOST_PCM_INFO (SNDRV_PCM_INFO_MMAP | \
SNDRV_PCM_INFO_MMAP_VALID | \
SNDRV_PCM_INFO_BATCH | \
@@ -157,9 +168,10 @@ static void most_to_alsa_copy32(void *alsa, void *most,
unsigned int bytes)
static struct channel *get_channel(struct most_interface *iface,
int channel_id)
{
+ struct sound_adapter *adpt = iface->priv;
struct channel *channel, *tmp;
- list_for_each_entry_safe(channel, tmp, _list, list) {
+ list_for_each_entry_safe(channel, tmp, >dev_list, list) {
if ((channel->iface == iface) && (channel->id == channel_id))
return channel;
}
@@ -460,7 +472,7 @@ static const struct snd_pcm_ops pcm_ops = {
};
static int split_arg_list(char *buf, char **card_name, u16 *ch_num,
- char **sample_res)
+ char **sample_res, u8 *create)
{
char *num;
int ret;
@@ -479,6 +491,9 @@ static int split_arg_list(char *buf, char **card_name, u16
*ch_num,
*sample_res = strsep(, ".\n");
if (!*sample_res)
goto err;
+
+ if (buf && !strcmp(buf, "create"))
+ *create = 1;
return 0;
err:
@@ -536,6 +551,20 @@ static int audio_set_hw_params(struct snd_pcm_hardware
*pcm_hw,
return 0;
}
+static void release_adapter(struct sound_adapter *adpt)
+{
+ struct channel *channel, *tmp;
+
+ list_for_each_entry_safe(channel, tmp, >dev_list, list) {
+ list_del(>list);
+ kfree(channel);
+ }
+ if (adpt->card)
+ snd_card_free(adpt->card);
+ list_del(>list);
+ kfree(adpt);
+}
+
/**
* audio_probe_channel - probe function of the driver module
* @iface: pointer to interface instance
@@ -553,7 +582,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
char *arg_list)
{
struct channel *channel;
- struct snd_card *card;
+ struct sound_adapter *adpt;
struct snd_pcm *pcm;
int playback_count = 0;
int capture_count = 0;
@@ -561,6 +590,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
int direction;
char *card_name;
u16 ch_num;
+ u8 create = 0;
char *sample_res;
if (!iface)
@@ -571,6 +601,39 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
return -EINVAL;
}
+ ret = split_arg_list(arg_list, _name, _num, _res,
+);
+ if (ret < 0)
+ return ret;
+
+ list_for_each_entry(adpt, _list, list) {
+ if (adpt->iface != iface)
+ continue;
+ if (adpt->registered)
+ return -ENOSPC;
+ adpt->pcm_dev_idx++;
+ goto skip_adpt_alloc;
+ }
+ adpt = kzalloc(sizeof(*adpt),
[PATCH v4 2/6] staging: most: sound: correct label name
This patch fixes the lable name that is used to jump to error
handling section of function audio_probe_channel() in case
something went wrong.
Signed-off-by: Christian Gromm
---
v2:
nothing
v3:
- rename label name of added goto instruction
v4:
nothing
drivers/staging/most/sound/sound.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/most/sound/sound.c
b/drivers/staging/most/sound/sound.c
index 9f84808..0e9377f 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -625,7 +625,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
ret = snd_card_new(>dev, -1, card_name, THIS_MODULE,
sizeof(*channel), >card);
if (ret < 0)
- goto err_free_card;
+ goto err_free_adpt;
snprintf(adpt->card->driver, sizeof(adpt->card->driver),
"%s", DRIVER_NAME);
snprintf(adpt->card->shortname, sizeof(adpt->card->shortname),
@@ -650,7 +650,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
channel = kzalloc(sizeof(*channel), GFP_KERNEL);
if (!channel) {
ret = -ENOMEM;
- goto err_free_card;
+ goto err_free_adpt;
}
channel->card = adpt->card;
channel->cfg = cfg;
@@ -662,13 +662,13 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
ret = audio_set_hw_params(>pcm_hardware, ch_num, sample_res,
cfg);
if (ret)
- goto err_free_card;
+ goto err_free_adpt;
ret = snd_pcm_new(adpt->card, card_name, adpt->pcm_dev_idx,
playback_count, capture_count, );
if (ret < 0)
- goto err_free_card;
+ goto err_free_adpt;
pcm->private_data = channel;
snprintf(pcm->name, sizeof(pcm->name), card_name);
@@ -677,12 +677,12 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
if (create) {
ret = snd_card_register(adpt->card);
if (ret < 0)
- goto err_free_card;
+ goto err_free_adpt;
adpt->registered = true;
}
return 0;
-err_free_card:
+err_free_adpt:
release_adapter(adpt);
return ret;
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 0/6] staging: most: sound: change sound card layout
Currently, for every synchronous channel allocated on MOST an extra sound card is being created and registered with ALSA. From a logical point of view this fails to reflect the actual condition, as all channels originate in the same physical device. This patch series introduces a way to map this layout and creates only one sound card per registered MOST device that has multiple PCM devices to access the individual MOST channels. To achieve this a change in the user space API is added that allows to signal the driver that the configuration is complete and that the new sound card is ready to be registered with ALSA. v2: - mention API change in description v3: nothing v4: nothing Christian Gromm (6): staging: most: sound: create one sound card w/ multiple PCM devices per MOST device staging: most: sound: correct label name staging: most: sound: rename variable staging: most: sound: use static name for ALSA card staging: most: sound: remove channel number from ALSA card's long name staging: most: Documentation: add information to driver_usage file .../staging/most/Documentation/driver_usage.txt| 16 ++- drivers/staging/most/sound/sound.c | 143 ++--- 2 files changed, 108 insertions(+), 51 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RESEND] staging: rtlwifi: convert to DEFINE_SHOW_ATTRIBUTE
On Sat, Dec 15, 2018 at 03:56:35AM -0500, Yangtao Li wrote: > Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code. > > Signed-off-by: Yangtao Li > --- > drivers/staging/rtlwifi/debug.c | 23 ++- > 1 file changed, 6 insertions(+), 17 deletions(-) Always test build your patches so you do not get grumpy maintainers telling you to not break the build :( Please fix up. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/9] staging: rtl8188eu: convert unsigned char arrays to u8
On Sat, Dec 15, 2018 at 05:46:11PM +0100, Michael Straube wrote: > Covert unsigned char arrays to u8 and make them static and/or const > where possible. You are also moving things around here from different .h files in different places. Always say exactly what you are doing. And this too needs to be broken up into smaller pieces, don't make things static/const when you change the type. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/9] staging: rtl8188eu: cleanup declarations in rtw_mlme_ext.c
On Sat, Dec 15, 2018 at 05:46:08PM +0100, Michael Straube wrote: > Replace tabs with spaces and/or remove spaces in declarations. > Remove unused/commented declarations, remove unnecessary comment, > remove blank lines between declarations and add missing lines after > declarations. Also clears some line over 80 characters checkpatch > warnings. That is a lot of different things to do all in one patch :( Please break this up into "one logical thing per patch" please, so it can be reviewed easier. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 01/33] staging: vc04_services: Remove DUMP_CONTEXT_T typedef
On Fri, Dec 14, 2018 at 01:04:38PM +0100, Dominic Braun wrote:
> Typedefing structs is not encouraged in the kernel.
>
> Signed-off-by: Dominic Braun
> Signed-off-by: Tobias Büttner
> ---
> .../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 2833f93bbc74..fa15033daf5f 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -154,12 +154,12 @@ struct vchiq_instance_struct {
> VCHIQ_DEBUGFS_NODE_T debugfs_node;
> };
>
> -typedef struct dump_context_struct {
> +struct dump_context {
> char __user *buf;
> size_t actual;
> size_t space;
> loff_t offset;
> -} DUMP_CONTEXT_T;
> +};
>
> static struct cdevvchiq_cdev;
> static dev_t vchiq_devid;
> @@ -2113,7 +2113,7 @@ static int vchiq_release(struct inode *inode, struct
> file *file)
> void
> vchiq_dump(void *dump_context, const char *str, int len)
> {
> - DUMP_CONTEXT_T *context = (DUMP_CONTEXT_T *)dump_context;
> + struct dump_context *context = (struct dump_context *)dump_context;
Stuff to clean up in the future. No need to cast this, in fact, the
function should be taking a real 'struct dump_context' pointer here, and
everywhere else up the call chain as it all trickles down to this
structure:
> vchiq_read(struct file *file, char __user *buf,
> size_t count, loff_t *ppos)
> {
> - DUMP_CONTEXT_T context;
> + struct dump_context context;
which is the correct type, so no casting or void * messing with needed
anywhere.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: comedi: 8255: fixed an SPDX License Tag coding style issue
On Thu, Dec 13, 2018 at 08:53:17AM -0800, Amir Mahdi Ghorbanian wrote: > From: Amir Mahdi Ghorbanian > > Fixed a coding style issue. > > Signed-off-by: Amir Mahdi Ghorbanian > --- > drivers/staging/comedi/drivers/8255.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/comedi/drivers/8255.h > b/drivers/staging/comedi/drivers/8255.h > index 6cd1339..ceae3ca 100644 > --- a/drivers/staging/comedi/drivers/8255.h > +++ b/drivers/staging/comedi/drivers/8255.h > @@ -1,4 +1,4 @@ > -// SPDX-License-Identifier: GPL-2.0+ > +/* SPDX-License-Identifier: GPL-2.0+ */ > /* > * module/8255.h > * Header file for 8255 > -- > 2.7.4 This patch does not apply to my tree at all. Always work against linux-next so you do not duplicate work others have already done. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] Staging: comedi: cb_pcidas: fixed a spelling mistake coding style issue
On Sun, Dec 16, 2018 at 08:57:25AM -0800, Amir Mahdi Ghorbanian wrote: > Fixed a coding style issue. > > Signed-off-by: Amir Mahdi Ghorbanian > --- > drivers/staging/comedi/drivers/cb_pcidas.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) You already sent this patch, right? What changed from the previous one? confused, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/6] staging: iio: adc: ad7606: Move out of staging
Move ad7606 ADC driver out of staging and into the mainline.
Signed-off-by: Stefan Popa
---
MAINTAINERS | 7 +
drivers/iio/adc/Kconfig | 27 ++
drivers/iio/adc/Makefile | 3 +
drivers/iio/adc/ad7606.c | 583 +++
drivers/iio/adc/ad7606.h | 99 ++
drivers/iio/adc/ad7606_par.c | 105 +++
drivers/iio/adc/ad7606_spi.c | 82 +
drivers/staging/iio/adc/Kconfig | 27 --
drivers/staging/iio/adc/Makefile | 4 -
drivers/staging/iio/adc/ad7606.c | 583 ---
drivers/staging/iio/adc/ad7606.h | 99 --
drivers/staging/iio/adc/ad7606_par.c | 105 ---
drivers/staging/iio/adc/ad7606_spi.c | 82 -
13 files changed, 906 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 db9fcf2..bc9f816 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -854,6 +854,13 @@ S: Supported
F: drivers/iio/adc/ad7124.c
F: Documentation/devicetree/bindings/iio/adc/adi,ad7124.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 7a3ca4e..f3cc7a3 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -69,6 +69,33 @@ config AD7476
To compile this driver as a module, choose M here: the
module will be called ad7476.
+config AD7606
+ tristate
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+
+config AD7606_IFACE_PARALLEL
+ tristate "Analog Devices AD7606 ADC driver with parallel interface
support"
+ depends on HAS_IOMEM
+ 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 07df37f..ea50313 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -11,6 +11,9 @@ obj-$(CONFIG_AD7291) += ad7291.o
obj-$(CONFIG_AD7298) += ad7298.o
obj-$(CONFIG_AD7923) += ad7923.o
obj-$(CONFIG_AD7476) += ad7476.o
+obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
+obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
+obj-$(CONFIG_AD7606) += ad7606.o
obj-$(CONFIG_AD7766) += ad7766.o
obj-$(CONFIG_AD7791) += ad7791.o
obj-$(CONFIG_AD7793) += ad7793.o
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
new file mode 100644
index 000..32854f1
--- /dev/null
+++ b/drivers/iio/adc/ad7606.c
@@ -0,0 +1,583 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AD7606 SPI ADC driver
+ *
+ * Copyright 2011 Analog Devices Inc.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "ad7606.h"
+
+/*
+ * Scales are computed as 5000/32768 and 1/32768 respectively,
+ * so that when applied to the raw values they provide mV values
+ */
+static const unsigned int scale_avail[2] = {
+ 152588, 305176
+};
+
+static const unsigned int ad7606_oversampling_avail[7] = {
+ 1, 2, 4, 8, 16, 32, 64,
+};
+
+static int ad7606_reset(struct ad7606_state *st)
+{
+ if (st->gpio_reset) {
+ gpiod_set_value(st->gpio_reset, 1);
+ ndelay(100); /* t_reset >= 100ns */
+ gpiod_set_value(st->gpio_reset, 0);
+ return 0;
+ }
+
+ return -ENODEV;
+}
+
+static int ad7606_read_samples(struct ad7606_state *st)
+{
+ unsigned int num = st->chip_info->num_channels;
+ u16 *data =
[PATCH v2 2/6] staging: iio: adc: ad7606: Use SPDX identifier
This patch replaces the license text at the top of ad7606 driver files
and instead adds SPDX GPL-2.0 license identifier.
Signed-off-by: Stefan Popa
---
drivers/staging/iio/adc/ad7606.c | 5 ++---
drivers/staging/iio/adc/ad7606.h | 3 +--
drivers/staging/iio/adc/ad7606_par.c | 5 ++---
drivers/staging/iio/adc/ad7606_spi.c | 5 ++---
4 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index e1d85b7..97b4a83 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* AD7606 SPI ADC driver
*
* Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
*/
#include
@@ -531,4 +530,4 @@ EXPORT_SYMBOL_GPL(ad7606_pm_ops);
MODULE_AUTHOR("Michael Hennerich ");
MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 510a93d..3e12fff 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -1,9 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* AD7606 ADC driver
*
* Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
*/
#ifndef IIO_ADC_AD7606_H_
diff --git a/drivers/staging/iio/adc/ad7606_par.c
b/drivers/staging/iio/adc/ad7606_par.c
index da26742..2d137b1 100644
--- a/drivers/staging/iio/adc/ad7606_par.c
+++ b/drivers/staging/iio/adc/ad7606_par.c
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* AD7606 Parallel Interface ADC driver
*
* Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
*/
#include
@@ -114,4 +113,4 @@ module_platform_driver(ad7606_driver);
MODULE_AUTHOR("Michael Hennerich ");
MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/staging/iio/adc/ad7606_spi.c
b/drivers/staging/iio/adc/ad7606_spi.c
index e533917..f38f0d6 100644
--- a/drivers/staging/iio/adc/ad7606_spi.c
+++ b/drivers/staging/iio/adc/ad7606_spi.c
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* AD7606 SPI ADC driver
*
* Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
*/
#include
@@ -80,4 +79,4 @@ module_spi_driver(ad7606_driver);
MODULE_AUTHOR("Michael Hennerich ");
MODULE_DESCRIPTION("Analog Devices AD7606 ADC");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/6] dt-bindings: iio: adc: Add docs for AD7606 ADC
Document support for AD7606 Analog to Digital Converter.
Signed-off-by: Stefan Popa
Reviewed-by: Rob Herring
---
.../devicetree/bindings/iio/adc/adi,ad7606.txt | 65 ++
MAINTAINERS| 1 +
2 files changed, 66 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
new file mode 100644
index 000..d7b6241
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
@@ -0,0 +1,65 @@
+Analog Devices AD7606 Simultaneous Sampling ADC
+
+Required properties for the AD7606:
+
+- compatible: Must be one of
+ * "adi,ad7605-4"
+ * "adi,ad7606-8"
+ * "adi,ad7606-6"
+ * "adi,ad7606-4"
+- reg: SPI chip select number for the device
+- spi-max-frequency: Max SPI frequency to use
+ see: Documentation/devicetree/bindings/spi/spi-bus.txt
+- spi-cpha: See Documentation/devicetree/bindings/spi/spi-bus.txt
+- avcc-supply: phandle to the Avcc power supply
+- interrupts: IRQ line for the ADC
+ see:
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+- adi,conversion-start-gpios: must be the device tree identifier of the CONVST
pin.
+ This logic input is used to initiate conversions on the analog
+ input channels. As the line is active high, it should be
marked
+ GPIO_ACTIVE_HIGH.
+
+Optional properties:
+
+- reset-gpios: must be the device tree identifier of the RESET pin. If
specified,
+ it will be asserted during driver probe. As the line is active
high,
+ it should be marked GPIO_ACTIVE_HIGH.
+- standby-gpios: must be the device tree identifier of the STBY pin. This pin
is used
+ to place the AD7606 into one of two power-down modes, Standby
mode or
+ Shutdown mode. As the line is active low, it should be marked
+ GPIO_ACTIVE_LOW.
+- adi,first-data-gpios: must be the device tree identifier of the FRSTDATA pin.
+ The FRSTDATA output indicates when the first channel, V1, is
+ being read back on either the parallel, byte or serial
interface.
+ As the line is active high, it should be marked
GPIO_ACTIVE_HIGH.
+- adi,range-gpios: must be the device tree identifier of the RANGE pin. The
polarity on
+ this pin determines the input range of the analog input channels.
If
+ this pin is tied to a logic high, the analog input range is ±10V
for
+ all channels. If this pin is tied to a logic low, the analog
input range
+ is ±5V for all channels. As the line is active high, it should be
marked
+ GPIO_ACTIVE_HIGH.
+- adi,oversampling-ratio-gpios: must be the device tree identifier of the
over-sampling
+ mode pins. As the line is active high, it
should be marked
+ GPIO_ACTIVE_HIGH.
+
+Example:
+
+ adc@0 {
+ compatible = "adi,ad7606-8";
+ reg = <0>;
+ spi-max-frequency = <100>;
+ spi-cpol;
+
+ avcc-supply = <_vref>;
+
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <>;
+
+ adi,conversion-start-gpios = < 17 GPIO_ACTIVE_HIGH>;
+ reset-gpios = < 27 GPIO_ACTIVE_HIGH>;
+ adi,first-data-gpios = < 22 GPIO_ACTIVE_HIGH>;
+ adi,oversampling-ratio-gpios = < 18 GPIO_ACTIVE_HIGH
+23 GPIO_ACTIVE_HIGH
+26 GPIO_ACTIVE_HIGH>;
+ standby-gpios = < 24 GPIO_ACTIVE_LOW>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index bc9f816..d039f66 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -860,6 +860,7 @@ L: linux-...@vger.kernel.org
W: http://ez.analog.com/community/linux-device-drivers
S: Supported
F: drivers/iio/adc/ad7606.c
+F: Documentation/devicetree/bindings/iio/adc/ad7606.txt
ANALOG DEVICES INC AD9389B DRIVER
M: Hans Verkuil
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/6] staging: iio: adc: ad7606: Misc style fixes (no functional change)
* Placed includes in alphabetical order
* Added brackets around num and mask through out for AD760X_CHANNEL
* Used single line comments where needed
* Removed extra lines and spaces
Signed-off-by: Stefan Popa
---
drivers/staging/iio/adc/ad7606.c | 27 ---
drivers/staging/iio/adc/ad7606.h | 1 -
drivers/staging/iio/adc/ad7606_par.c | 27 ---
drivers/staging/iio/adc/ad7606_spi.c | 16
4 files changed, 28 insertions(+), 43 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 5f0712c..32854f1 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -5,25 +5,25 @@
* Copyright 2011 Analog Devices Inc.
*/
-#include
+#include
#include
-#include
-#include
-#include
-#include
#include
#include
-#include
-#include
+#include
+#include
#include
+#include
+#include
+#include
+#include
#include
#include
-#include
#include
-#include
+#include
#include
#include
+#include
#include "ad7606.h"
@@ -249,8 +249,7 @@ static const struct attribute_group
ad7606_attribute_group_range = {
.attrs = ad7606_attributes_range,
};
-#define AD760X_CHANNEL(num, mask) \
- { \
+#define AD760X_CHANNEL(num, mask) {\
.type = IIO_VOLTAGE,\
.indexed = 1, \
.channel = num, \
@@ -265,7 +264,7 @@ static const struct attribute_group
ad7606_attribute_group_range = {
.storagebits = 16, \
.endianness = IIO_CPU, \
}, \
- }
+}
#define AD7605_CHANNEL(num)\
AD760X_CHANNEL(num, 0)
@@ -294,9 +293,7 @@ static const struct iio_chan_spec ad7606_channels[] = {
};
static const struct ad7606_chip_info ad7606_chip_info_tbl[] = {
- /*
-* More devices added in future
-*/
+ /* More devices added in future */
[ID_AD7605_4] = {
.channels = ad7605_channels,
.num_channels = 5,
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 9a832d2..5d12410 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,7 +14,6 @@
* @num_channels: number of channels
* @has_oversampling: whether the device has oversampling support
*/
-
struct ad7606_chip_info {
const struct iio_chan_spec *channels;
unsigned intnum_channels;
diff --git a/drivers/staging/iio/adc/ad7606_par.c
b/drivers/staging/iio/adc/ad7606_par.c
index 2d137b1..df607c7 100644
--- a/drivers/staging/iio/adc/ad7606_par.c
+++ b/drivers/staging/iio/adc/ad7606_par.c
@@ -26,7 +26,7 @@ static int ad7606_par16_read_block(struct device *dev,
}
static const struct ad7606_bus_ops ad7606_par16_bops = {
- .read_block = ad7606_par16_read_block,
+ .read_block = ad7606_par16_read_block,
};
static int ad7606_par8_read_block(struct device *dev,
@@ -41,7 +41,7 @@ static int ad7606_par8_read_block(struct device *dev,
}
static const struct ad7606_bus_ops ad7606_par8_bops = {
- .read_block = ad7606_par8_read_block,
+ .read_block = ad7606_par8_read_block,
};
static int ad7606_par_probe(struct platform_device *pdev)
@@ -72,22 +72,12 @@ static int ad7606_par_probe(struct platform_device *pdev)
}
static const struct platform_device_id ad7606_driver_ids[] = {
- {
- .name = "ad7605-4",
- .driver_data= ID_AD7605_4,
- }, {
- .name = "ad7606-8",
- .driver_data= ID_AD7606_8,
- }, {
- .name = "ad7606-6",
- .driver_data= ID_AD7606_6,
- }, {
- .name = "ad7606-4",
- .driver_data= ID_AD7606_4,
- },
+ { .name = "ad7605-4", .driver_data = ID_AD7605_4, },
+ { .name = "ad7606-4", .driver_data = ID_AD7606_4, },
+ { .name = "ad7606-6", .driver_data = ID_AD7606_6, },
+ { .name = "ad7606-8", .driver_data = ID_AD7606_8, },
{ }
};
-
MODULE_DEVICE_TABLE(platform, ad7606_driver_ids);
static const struct of_device_id ad7606_of_match[] = {
@@ -103,12 +93,11 @@ static struct platform_driver ad7606_driver = {
.probe = ad7606_par_probe,
.id_table = ad7606_driver_ids,
.driver = {
- .name= "ad7606",
- .pm = AD7606_PM_OPS,
+ .name = "ad7606",
+ .pm = AD7606_PM_OPS,
.of_match_table = ad7606_of_match,
},
};
-
module_platform_driver(ad7606_driver);
[PATCH v2 3/6] staging: iio: adc: ad7606: Add support for threaded irq
This patch replaces the use of a polling ring buffer with a threaded
interrupt.
Enabling the buffer sets the CONVST signal to high. When the rising edge
of the CONVST is applied, BUSY signal goes logic high and transitions low
at the end of the entire conversion process. The falling edge of the BUSY
signal triggers the interrupt.
ad7606_trigger_handler() is used as bottom half of the poll function.
It reads data from the device and stores it in the internal buffer.
Signed-off-by: Stefan Popa
---
drivers/staging/iio/adc/ad7606.c | 111 +--
drivers/staging/iio/adc/ad7606.h | 6 +--
2 files changed, 84 insertions(+), 33 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 97b4a83..5f0712c 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include "ad7606.h"
@@ -86,36 +87,24 @@ static int ad7606_read_samples(struct ad7606_state *st)
static irqreturn_t ad7606_trigger_handler(int irq, void *p)
{
struct iio_poll_func *pf = p;
- struct ad7606_state *st = iio_priv(pf->indio_dev);
-
- gpiod_set_value(st->gpio_convst, 1);
-
- return IRQ_HANDLED;
-}
-
-/**
- * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring buffer
- * @work_s:the work struct through which this was scheduled
- *
- * Currently there is no option in this driver to disable the saving of
- * timestamps within the ring.
- * I think the one copy of this at a time was to avoid problems if the
- * trigger was set far too high and the reads then locked up the computer.
- **/
-static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
-{
- struct ad7606_state *st = container_of(work_s, struct ad7606_state,
- poll_work);
- struct iio_dev *indio_dev = iio_priv_to_dev(st);
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct ad7606_state *st = iio_priv(indio_dev);
int ret;
+ mutex_lock(>lock);
+
ret = ad7606_read_samples(st);
if (ret == 0)
iio_push_to_buffers_with_timestamp(indio_dev, st->data,
iio_get_time_ns(indio_dev));
- gpiod_set_value(st->gpio_convst, 0);
iio_trigger_notify_done(indio_dev->trig);
+ /* The rising edge of the CONVST signal starts a new conversion. */
+ gpiod_set_value(st->gpio_convst, 1);
+
+ mutex_unlock(>lock);
+
+ return IRQ_HANDLED;
}
static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
@@ -366,8 +355,11 @@ static int ad7606_request_gpios(struct ad7606_state *st)
return PTR_ERR_OR_ZERO(st->gpio_os);
}
-/**
- * Interrupt handler
+/*
+ * The BUSY signal indicates when conversions are in progress, so when a rising
+ * edge of CONVST is applied, BUSY goes logic high and transitions low at the
+ * end of the entire conversion process. The falling edge of the BUSY signal
+ * triggers this interrupt.
*/
static irqreturn_t ad7606_interrupt(int irq, void *dev_id)
{
@@ -375,7 +367,8 @@ static irqreturn_t ad7606_interrupt(int irq, void *dev_id)
struct ad7606_state *st = iio_priv(indio_dev);
if (iio_buffer_enabled(indio_dev)) {
- schedule_work(>poll_work);
+ gpiod_set_value(st->gpio_convst, 0);
+ iio_trigger_poll_chained(st->trig);
} else {
complete(>completion);
}
@@ -383,26 +376,69 @@ static irqreturn_t ad7606_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
};
+static int ad7606_validate_trigger(struct iio_dev *indio_dev,
+ struct iio_trigger *trig)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ if (st->trig != trig)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int ad7606_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ iio_triggered_buffer_postenable(indio_dev);
+ gpiod_set_value(st->gpio_convst, 1);
+
+ return 0;
+}
+
+static int ad7606_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct ad7606_state *st = iio_priv(indio_dev);
+
+ gpiod_set_value(st->gpio_convst, 0);
+
+ return iio_triggered_buffer_predisable(indio_dev);
+}
+
+static const struct iio_buffer_setup_ops ad7606_buffer_ops = {
+ .postenable = _buffer_postenable,
+ .predisable = _buffer_predisable,
+};
+
static const struct iio_info ad7606_info_no_os_or_range = {
.read_raw = _read_raw,
+ .validate_trigger = _validate_trigger,
};
static const struct iio_info ad7606_info_os_and_range = {
.read_raw = _read_raw,
.write_raw = _write_raw,
.attrs = _attribute_group_os_and_range,
+ .validate_trigger = _validate_trigger,
};
static const struct iio_info
[PATCH v2 0/6] staging: iio: ad7606: Move out of staging
Changes in v2: Patch 1: - Moved the HAS_IOMEM under AD7606_IFACE_PARALLEL and dropped GPIOLIB. Patch 2: - Used SPDX GPL-2.0 license identifier instead of GPL-2.0+. Patch 3: - Before disabling the buffer, there is no need to trigger a conversion. Patches 4, 5, 6: - Nothing changed. Stefan Popa (6): staging: iio: adc: ad7606: Simplify the Kconfing menu staging: iio: adc: ad7606: Use SPDX identifier staging: iio: adc: ad7606: Add support for threaded irq staging: iio: adc: ad7606: Misc style fixes (no functional change) staging: iio: adc: ad7606: Move out of staging dt-bindings: iio: adc: Add docs for AD7606 ADC .../devicetree/bindings/iio/adc/adi,ad7606.txt | 65 +++ MAINTAINERS| 8 + drivers/iio/adc/Kconfig| 27 + drivers/iio/adc/Makefile | 3 + drivers/iio/adc/ad7606.c | 583 + drivers/iio/adc/ad7606.h | 99 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 | 4 - drivers/staging/iio/adc/ad7606.c | 534 --- drivers/staging/iio/adc/ad7606.h | 103 drivers/staging/iio/adc/ad7606_par.c | 117 - drivers/staging/iio/adc/ad7606_spi.c | 83 --- 14 files changed, 972 insertions(+), 875 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt 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 -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/6] staging: iio: adc: ad7606: Simplify the Kconfing menu
There is no point in having three menu entries that can be selected individually. Instead, the SPI and parallel interfaces should select AD7606. Signed-off-by: Stefan Popa --- drivers/staging/iio/adc/Kconfig | 27 ++- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig index fc23059..302639a 100644 --- a/drivers/staging/iio/adc/Kconfig +++ b/drivers/staging/iio/adc/Kconfig @@ -4,35 +4,28 @@ menu "Analog to digital converters" config AD7606 - tristate "Analog Devices AD7606 ADC driver" - depends on GPIOLIB || COMPILE_TEST - depends on HAS_IOMEM + tristate select IIO_BUFFER select IIO_TRIGGERED_BUFFER - help - Say yes here to build 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. config AD7606_IFACE_PARALLEL - tristate "parallel interface support" - depends on AD7606 + tristate "Analog Devices AD7606 ADC driver with parallel interface support" + depends on HAS_IOMEM + select AD7606 help - Say yes here to include parallel interface support on the AD7606 - ADC driver. + 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 "spi interface support" - depends on AD7606 + tristate "Analog Devices AD7606 ADC driver with spi interface support" depends on SPI + select AD7606 help - Say yes here to include parallel interface support on the AD7606 - ADC driver. + 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. -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/11] staging: iio: adc: ad7606: Add support for threaded irq
On Du, 2018-12-16 at 13:49 +, Jonathan Cameron wrote:
> On Thu, 13 Dec 2018 14:46:17 +0200
> Stefan Popa wrote:
>
> >
> > This patch replaces the use of a polling ring buffer with a threaded
> > interrupt.
> >
> > Enabling the buffer sets the CONVST signal to high. When the rising
> > edge
> > of the CONVST is applied, BUSY signal goes logic high and transitions
> > low
> > at the end of the entire conversion process. The falling edge of the
> > BUSY
> > signal triggers the interrupt.
> >
> > ad7606_trigger_handler() is used as bottom half of the poll function.
> > It reads data from the device and stores it in the internal buffer.
> >
> > Signed-off-by: Stefan Popa
> I'd like a little more info as comments in this one. See below.
> Which is another way of saying I have no idea what is going on :)
>
> Thanks,
>
> Jonathan.
>
Hi Jonathan,
Thank you for the review. It turns out that there is no reason to trigger a
conversion before disabling the buffer. I will remove it in v2.
Thank you!
-Stefan
> >
> > ---
> > drivers/staging/iio/adc/ad7606.c | 116 +
> > --
> > drivers/staging/iio/adc/ad7606.h | 6 +-
> > 2 files changed, 89 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7606.c
> > b/drivers/staging/iio/adc/ad7606.c
> > index 7191d51..13aeeec 100644
> > --- a/drivers/staging/iio/adc/ad7606.c
> > +++ b/drivers/staging/iio/adc/ad7606.c
> > @@ -21,6 +21,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include "ad7606.h"
> > @@ -81,36 +82,24 @@ static int ad7606_read_samples(struct ad7606_state
> > *st)
> > static irqreturn_t ad7606_trigger_handler(int irq, void *p)
> > {
> > struct iio_poll_func *pf = p;
> > - struct ad7606_state *st = iio_priv(pf->indio_dev);
> > -
> > - gpiod_set_value(st->gpio_convst, 1);
> > -
> > - return IRQ_HANDLED;
> > -}
> > -
> > -/**
> > - * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring
> > buffer
> > - * @work_s:the work struct through which this was scheduled
> > - *
> > - * Currently there is no option in this driver to disable the saving
> > of
> > - * timestamps within the ring.
> > - * I think the one copy of this at a time was to avoid problems if the
> > - * trigger was set far too high and the reads then locked up the
> > computer.
> > - **/
> > -static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
> > -{
> > - struct ad7606_state *st = container_of(work_s, struct
> > ad7606_state,
> > - poll_work);
> > - struct iio_dev *indio_dev = iio_priv_to_dev(st);
> > + struct iio_dev *indio_dev = pf->indio_dev;
> > + struct ad7606_state *st = iio_priv(indio_dev);
> > int ret;
> >
> > + mutex_lock(>lock);
> > +
> > ret = ad7606_read_samples(st);
> > if (ret == 0)
> > iio_push_to_buffers_with_timestamp(indio_dev, st-
> > >data,
> > iio_get_time_ns(ind
> > io_dev));
> >
> > - gpiod_set_value(st->gpio_convst, 0);
> > iio_trigger_notify_done(indio_dev->trig);
> > + /* The rising edge of the CONVST signal starts a new
> > conversion. */
> > + gpiod_set_value(st->gpio_convst, 1);
> > +
> > + mutex_unlock(>lock);
> > +
> > + return IRQ_HANDLED;
> > }
> >
> > static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int
> > ch)
> > @@ -378,8 +367,11 @@ static int ad7606_request_gpios(struct
> > ad7606_state *st)
> > return PTR_ERR_OR_ZERO(st->gpio_os);
> > }
> >
> > -/**
> > - * Interrupt handler
> > +/*
> > + * The BUSY signal indicates when conversions are in progress, so when
> > a rising
> > + * edge of CONVST is applied, BUSY goes logic high and transitions low
> > at the
> > + * end of the entire conversion process. The falling edge of the BUSY
> > signal
> > + * triggers this interrupt.
> > */
> > static irqreturn_t ad7606_interrupt(int irq, void *dev_id)
> > {
> > @@ -387,7 +379,8 @@ static irqreturn_t ad7606_interrupt(int irq, void
> > *dev_id)
> > struct ad7606_state *st = iio_priv(indio_dev);
> >
> > if (iio_buffer_enabled(indio_dev)) {
> > - schedule_work(>poll_work);
> > + gpiod_set_value(st->gpio_convst, 0);
> > + iio_trigger_poll_chained(st->trig);
> > } else {
> > complete(>completion);
> > }
> > @@ -395,26 +388,74 @@ static irqreturn_t ad7606_interrupt(int irq, void
> > *dev_id)
> > return IRQ_HANDLED;
> > };
> >
> > +static int ad7606_validate_trigger(struct iio_dev *indio_dev,
> > + struct iio_trigger *trig)
> > +{
> > + struct ad7606_state *st = iio_priv(indio_dev);
> > +
> > + if (st->trig != trig)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +static int ad7606_buffer_postenable(struct iio_dev *indio_dev)
> > +{
> > + struct ad7606_state *st = iio_priv(indio_dev);
> > +
> > +
Re: [PATCH v3 1/6] staging: most: sound: create one sound card w/ multiple PCM devices per MOST device
On Mon, Dec 17, 2018 at 12:33:49PM +0100, Christian Gromm wrote:
> +static void release_adapter(struct sound_adapter *adpt)
> +{
> + struct channel *channel, *tmp;
> +
> + list_for_each_entry_safe(channel, tmp, >dev_list, list) {
> + list_del(>list);
> + kfree(channel);
> + }
> + snd_card_free(adpt->card);
I'm sorry for drawing this out. I should have seen this problem in the
v2 patch. snd_card_free() does not take NULL pointers. We need to
say:
if (adpt->card)
snd_card_free(adpt->card);
> + list_del(>list);
> + kfree(adpt);
> +}
> +
> /**
> * audio_probe_channel - probe function of the driver module
> * @iface: pointer to interface instance
> @@ -553,7 +581,7 @@ static int audio_probe_channel(struct most_interface
> *iface, int channel_id,
> char *arg_list)
> {
> struct channel *channel;
> - struct snd_card *card;
> + struct sound_adapter *adpt;
> struct snd_pcm *pcm;
> int playback_count = 0;
> int capture_count = 0;
> @@ -561,6 +589,7 @@ static int audio_probe_channel(struct most_interface
> *iface, int channel_id,
> int direction;
> char *card_name;
> u16 ch_num;
> + u8 create = 0;
> char *sample_res;
>
> if (!iface)
> @@ -571,6 +600,39 @@ static int audio_probe_channel(struct most_interface
> *iface, int channel_id,
> return -EINVAL;
> }
>
> + ret = split_arg_list(arg_list, _name, _num, _res,
> + );
> + if (ret < 0)
> + return ret;
> +
> + list_for_each_entry(adpt, _list, list) {
> + if (adpt->iface != iface)
> + continue;
> + if (adpt->registered)
> + return -ENOSPC;
> + adpt->pcm_dev_idx++;
> + goto skip_adpt_alloc;
> + }
> + adpt = kzalloc(sizeof(*adpt), GFP_KERNEL);
> + if (!adpt)
> + return -ENOMEM;
> +
> + adpt->iface = iface;
> + INIT_LIST_HEAD(>dev_list);
> + iface->priv = adpt;
> + list_add_tail(>list, _list);
> + ret = snd_card_new(>dev, -1, card_name, THIS_MODULE,
> +sizeof(*channel), >card);
> + if (ret < 0)
> + goto err_free_card;
Otherwise this error path will oops.
[ Snip ]
> err_free_card:
> - snd_card_free(card);
> + release_adapter(adpt);
> return ret;
> }
I feel quite bad for making you redo this over and over. :(
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 4/6] staging: most: sound: use static name for ALSA card
This patch uses a static name for the sound card's short name and long name. Having the card names configurable doesn't make sense anymore, as the card represents the same physical hardware. Signed-off-by: Christian Gromm --- v2: nothing v3: nothing drivers/staging/most/sound/sound.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c index 8d5211d..9a851e5 100644 --- a/drivers/staging/most/sound/sound.c +++ b/drivers/staging/most/sound/sound.c @@ -621,14 +621,14 @@ static int audio_probe_channel(struct most_interface *iface, int channel_id, INIT_LIST_HEAD(>dev_list); iface->priv = adpt; list_add_tail(>list, _list); - ret = snd_card_new(>dev, -1, device_name, THIS_MODULE, + ret = snd_card_new(>dev, -1, "INIC", THIS_MODULE, sizeof(*channel), >card); if (ret < 0) goto err_free_adpt; snprintf(adpt->card->driver, sizeof(adpt->card->driver), "%s", DRIVER_NAME); snprintf(adpt->card->shortname, sizeof(adpt->card->shortname), -"Microchip MOST:%d", adpt->card->number); +"Microchip INIC"); snprintf(adpt->card->longname, sizeof(adpt->card->longname), "%s at %s, ch %d", adpt->card->shortname, iface->description, channel_id); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/6] staging: most: sound: create one sound card w/ multiple PCM devices per MOST device
This patch avoids that a sound card is created and registered with ALSA
every time a channel is being linked. Instead the channels are hooked on
the same card, which is registered not until the final link has been added
to the component. The string provided by user space that used to be the
card name becomes the PCM device name. The user space API to add a link is
being expanded by a "create" flag to trigger the registration.
Signed-off-by: Christian Gromm
---
v2:
- push audio apdaper to list right afer its initialization
Reported-by: Dan Carpenter
- mention user space API change in description
- assign value to variable ret before jumping to error handling
- remove unnecessary variable initializations
- fix if statements when iterating audio adapter list
- use correct variable in sizeof operator
- store PCM device name
v3:
- jump to error handling in case snd_card_new() fails
drivers/staging/most/sound/sound.c | 129 +
1 file changed, 88 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/most/sound/sound.c
b/drivers/staging/most/sound/sound.c
index 89b02fc..cb02590 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -20,7 +21,6 @@
#define DRIVER_NAME "sound"
-static struct list_head dev_list;
static struct core_component comp;
/**
@@ -56,6 +56,17 @@ struct channel {
void (*copy_fn)(void *alsa, void *most, unsigned int bytes);
};
+struct sound_adapter {
+ struct list_head dev_list;
+ struct most_interface *iface;
+ struct snd_card *card;
+ struct list_head list;
+ bool registered;
+ int pcm_dev_idx;
+};
+
+static struct list_head adpt_list;
+
#define MOST_PCM_INFO (SNDRV_PCM_INFO_MMAP | \
SNDRV_PCM_INFO_MMAP_VALID | \
SNDRV_PCM_INFO_BATCH | \
@@ -157,9 +168,10 @@ static void most_to_alsa_copy32(void *alsa, void *most,
unsigned int bytes)
static struct channel *get_channel(struct most_interface *iface,
int channel_id)
{
+ struct sound_adapter *adpt = iface->priv;
struct channel *channel, *tmp;
- list_for_each_entry_safe(channel, tmp, _list, list) {
+ list_for_each_entry_safe(channel, tmp, >dev_list, list) {
if ((channel->iface == iface) && (channel->id == channel_id))
return channel;
}
@@ -460,7 +472,7 @@ static const struct snd_pcm_ops pcm_ops = {
};
static int split_arg_list(char *buf, char **card_name, u16 *ch_num,
- char **sample_res)
+ char **sample_res, u8 *create)
{
char *num;
int ret;
@@ -479,6 +491,9 @@ static int split_arg_list(char *buf, char **card_name, u16
*ch_num,
*sample_res = strsep(, ".\n");
if (!*sample_res)
goto err;
+
+ if (buf && !strcmp(buf, "create"))
+ *create = 1;
return 0;
err:
@@ -536,6 +551,19 @@ static int audio_set_hw_params(struct snd_pcm_hardware
*pcm_hw,
return 0;
}
+static void release_adapter(struct sound_adapter *adpt)
+{
+ struct channel *channel, *tmp;
+
+ list_for_each_entry_safe(channel, tmp, >dev_list, list) {
+ list_del(>list);
+ kfree(channel);
+ }
+ snd_card_free(adpt->card);
+ list_del(>list);
+ kfree(adpt);
+}
+
/**
* audio_probe_channel - probe function of the driver module
* @iface: pointer to interface instance
@@ -553,7 +581,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
char *arg_list)
{
struct channel *channel;
- struct snd_card *card;
+ struct sound_adapter *adpt;
struct snd_pcm *pcm;
int playback_count = 0;
int capture_count = 0;
@@ -561,6 +589,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
int direction;
char *card_name;
u16 ch_num;
+ u8 create = 0;
char *sample_res;
if (!iface)
@@ -571,6 +600,39 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
return -EINVAL;
}
+ ret = split_arg_list(arg_list, _name, _num, _res,
+);
+ if (ret < 0)
+ return ret;
+
+ list_for_each_entry(adpt, _list, list) {
+ if (adpt->iface != iface)
+ continue;
+ if (adpt->registered)
+ return -ENOSPC;
+ adpt->pcm_dev_idx++;
+ goto skip_adpt_alloc;
+ }
+ adpt = kzalloc(sizeof(*adpt), GFP_KERNEL);
+ if (!adpt)
+ return -ENOMEM;
+
+ adpt->iface = iface;
+
[PATCH v3 5/6] staging: most: sound: remove channel number from ALSA card's long name
Adding the channel number to the name of the sound card is wrong,
as the card does not represent a single streaming channel of the
MOST device.
Signed-off-by: Christian Gromm
---
v2:
nothing
v3:
nothing
drivers/staging/most/sound/sound.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/most/sound/sound.c
b/drivers/staging/most/sound/sound.c
index 9a851e5..9cac58e 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -630,8 +630,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
snprintf(adpt->card->shortname, sizeof(adpt->card->shortname),
"Microchip INIC");
snprintf(adpt->card->longname, sizeof(adpt->card->longname),
-"%s at %s, ch %d", adpt->card->shortname, iface->description,
-channel_id);
+"%s at %s", adpt->card->shortname, iface->description);
skip_adpt_alloc:
if (get_channel(iface, channel_id)) {
pr_err("channel (%s:%d) is already linked\n",
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 6/6] staging: most: Documentation: add information to driver_usage file
This patch updates driver_usage.txt file to reflect the latest changes that this patch set introduces. Signed-off-by: Christian Gromm --- v2: nothing v3: nothing drivers/staging/most/Documentation/driver_usage.txt | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/staging/most/Documentation/driver_usage.txt b/drivers/staging/most/Documentation/driver_usage.txt index bb9b4e8..da7a8f4 100644 --- a/drivers/staging/most/Documentation/driver_usage.txt +++ b/drivers/staging/most/Documentation/driver_usage.txt @@ -142,8 +142,9 @@ Cdev component example: Sound component example: -The sound component needs an additional parameter to determine the audio -resolution that is going to be used. The following formats are available: +The sound component needs additional parameters to determine the audio +resolution that is going to be used and to trigger the registration of a +sound card with ALSA. The following audio formats are available: - "1x8" (Mono) - "2x16" (16-bit stereo) @@ -151,9 +152,18 @@ resolution that is going to be used. The following formats are available: - "2x32" (32-bit stereo) - "6x16" (16-bit surround 5.1) -$ echo "mdev0:ep_81:sound:most51_playback.6x16" >$(DRV_DIR)/add_link +To make the sound module create a sound card and register it with ALSA the +string "create" needs to be attached to the module parameter section of the +configuration string. To create a sound card with with two playback devices +(linked to channel ep01 and ep02) and one capture device (linked to channel +ep83) the following is written to the add_link file: +$ echo "mdev0:ep01:sound:most51_playback.6x16" >$(DRV_DIR)/add_link +$ echo "mdev0:ep02:sound:most_playback.2x16" >$(DRV_DIR)/add_link +$ echo "mdev0:ep83:sound:most_capture.2x16.create" >$(DRV_DIR)/add_link +The link names (most51_playback, most_playback and most_capture) will +become the names of the PCM devices of the sound card. Section 2.3 USB Padding -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/6] staging: most: sound: rename variable
Since the channels of a MOST device are now being represented as
individual PCM devices of one sound card, the variable card_name is not
suitable anymore to describe them. Therefore, this patch renames the
variable to device_name.
Signed-off-by: Christian Gromm
---
v2:
Reported-by: Dan Carpenter
- remove call to snprintf to store PCM name
v3:
nothing
drivers/staging/most/sound/sound.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/most/sound/sound.c
b/drivers/staging/most/sound/sound.c
index 22e106f..8d5211d 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -471,14 +471,14 @@ static const struct snd_pcm_ops pcm_ops = {
.page = snd_pcm_lib_get_vmalloc_page,
};
-static int split_arg_list(char *buf, char **card_name, u16 *ch_num,
+static int split_arg_list(char *buf, char **device_name, u16 *ch_num,
char **sample_res, u8 *create)
{
char *num;
int ret;
- *card_name = strsep(, ".");
- if (!*card_name) {
+ *device_name = strsep(, ".");
+ if (!*device_name) {
pr_err("Missing sound card name\n");
return -EIO;
}
@@ -587,7 +587,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
int capture_count = 0;
int ret;
int direction;
- char *card_name;
+ char *device_name;
u16 ch_num;
u8 create = 0;
char *sample_res;
@@ -600,7 +600,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
return -EINVAL;
}
- ret = split_arg_list(arg_list, _name, _num, _res,
+ ret = split_arg_list(arg_list, _name, _num, _res,
);
if (ret < 0)
return ret;
@@ -621,7 +621,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
INIT_LIST_HEAD(>dev_list);
iface->priv = adpt;
list_add_tail(>list, _list);
- ret = snd_card_new(>dev, -1, card_name, THIS_MODULE,
+ ret = snd_card_new(>dev, -1, device_name, THIS_MODULE,
sizeof(*channel), >card);
if (ret < 0)
goto err_free_adpt;
@@ -663,14 +663,14 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
if (ret)
goto err_free_adpt;
- ret = snd_pcm_new(adpt->card, card_name, adpt->pcm_dev_idx,
+ ret = snd_pcm_new(adpt->card, device_name, adpt->pcm_dev_idx,
playback_count, capture_count, );
if (ret < 0)
goto err_free_adpt;
pcm->private_data = channel;
- snprintf(pcm->name, sizeof(pcm->name), card_name);
+ snprintf(pcm->name, sizeof(pcm->name), device_name);
snd_pcm_set_ops(pcm, direction, _ops);
if (create) {
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/6] staging: most: sound: change sound card layout
Currently, for every synchronous channel allocated on MOST an extra sound card is being created and registered with ALSA. From a logical point of view this fails to reflect the actual condition, as all channels originate in the same physical device. This patch series introduces a way to map this layout and creates only one sound card per registered MOST device that has multiple PCM devices to access the individual MOST channels. To achieve this a change in the user space API is added that allows to signal the driver that the configuration is complete and that the new sound card is ready to be registered with ALSA. v2: - mention API change in description v3: nothing Christian Gromm (6): staging: most: sound: create one sound card w/ multiple PCM devices per MOST device staging: most: sound: correct label name staging: most: sound: rename variable staging: most: sound: use static name for ALSA card staging: most: sound: remove channel number from ALSA card's long name staging: most: Documentation: add information to driver_usage file .../staging/most/Documentation/driver_usage.txt| 16 ++- drivers/staging/most/sound/sound.c | 142 ++--- 2 files changed, 107 insertions(+), 51 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/6] staging: most: sound: correct label name
This patch fixes the lable name that is used to jump to error
handling section of function audio_probe_channel() in case
something went wrong.
Signed-off-by: Christian Gromm
---
v2:
nothing
v3:
- rename label name of added goto instruction
drivers/staging/most/sound/sound.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/most/sound/sound.c
b/drivers/staging/most/sound/sound.c
index cb02590..22e106f 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -624,7 +624,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
ret = snd_card_new(>dev, -1, card_name, THIS_MODULE,
sizeof(*channel), >card);
if (ret < 0)
- goto err_free_card;
+ goto err_free_adpt;
snprintf(adpt->card->driver, sizeof(adpt->card->driver),
"%s", DRIVER_NAME);
snprintf(adpt->card->shortname, sizeof(adpt->card->shortname),
@@ -649,7 +649,7 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
channel = kzalloc(sizeof(*channel), GFP_KERNEL);
if (!channel) {
ret = -ENOMEM;
- goto err_free_card;
+ goto err_free_adpt;
}
channel->card = adpt->card;
channel->cfg = cfg;
@@ -661,13 +661,13 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
ret = audio_set_hw_params(>pcm_hardware, ch_num, sample_res,
cfg);
if (ret)
- goto err_free_card;
+ goto err_free_adpt;
ret = snd_pcm_new(adpt->card, card_name, adpt->pcm_dev_idx,
playback_count, capture_count, );
if (ret < 0)
- goto err_free_card;
+ goto err_free_adpt;
pcm->private_data = channel;
snprintf(pcm->name, sizeof(pcm->name), card_name);
@@ -676,12 +676,12 @@ static int audio_probe_channel(struct most_interface
*iface, int channel_id,
if (create) {
ret = snd_card_register(adpt->card);
if (ret < 0)
- goto err_free_card;
+ goto err_free_adpt;
adpt->registered = true;
}
return 0;
-err_free_card:
+err_free_adpt:
release_adapter(adpt);
return ret;
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: bcm2835-audio: double free in init error path
On Mon, 17 Dec 2018 08:08:54 +0100,
Dan Carpenter wrote:
>
> We free instance here and in the caller. It should be only the caller
> which handles it.
>
> Fixes: d7ca3a71545b ("staging: bcm2835-audio: Operate non-atomic PCM ops")
> Signed-off-by: Dan Carpenter
Reviewed-by: Takashi Iwai
thanks,
Takashi
> ---
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> index 0db412fd7c55..c0debdbce26c 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> @@ -138,7 +138,6 @@ vc_vchi_audio_init(VCHI_INSTANCE_T vchi_instance,
> dev_err(instance->dev,
> "failed to open VCHI service connection (status=%d)\n",
> status);
> - kfree(instance);
> return -EPERM;
> }
>
> --
> 2.17.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] uio_hv_generic: set callbacks on open
On Mon, 2018-12-10 at 10:18 -0800, Stephen Hemminger wrote:
> This fixes the problem where uio application was unable to
> use multple queues on restart. The root cause is that the callbacks
> are cleared on disconnect. Change to setting up callbacks
> everytime in open.
>
> Fixes: cdfa835c6e5e ("uio_hv_generic: defer opening vmbus until first
> use")
> Reported-by: Mohammed Gamal
> Signed-off-by: Stephen Hemminger
> ---
> drivers/uio/uio_hv_generic.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/uio/uio_hv_generic.c
> b/drivers/uio/uio_hv_generic.c
> index c2493d011225..3c5169eb23f5 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -204,9 +204,11 @@ hv_uio_open(struct uio_info *info, struct inode
> *inode)
> if (atomic_inc_return(>refcnt) != 1)
> return 0;
>
> + vmbus_set_chn_rescind_callback(dev->channel,
> hv_uio_rescind);
> + vmbus_set_sc_create_callback(dev->channel,
> hv_uio_new_channel);
> +
> ret = vmbus_connect_ring(dev->channel,
> hv_uio_channel_cb, dev->channel);
> -
> if (ret == 0)
> dev->channel->inbound.ring_buffer->interrupt_mask =
> 1;
> else
> @@ -334,9 +336,6 @@ hv_uio_probe(struct hv_device *dev,
> goto fail_close;
> }
>
> - vmbus_set_chn_rescind_callback(channel, hv_uio_rescind);
> - vmbus_set_sc_create_callback(channel, hv_uio_new_channel);
> -
> ret = sysfs_create_bin_file(>kobj,
> _buffer_bin_attr);
> if (ret)
> dev_notice(>device,
Tested-by: Mohammed Gamal
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] Staging: nvec: nvec: fixed check style issues
On Sun, Dec 16, 2018 at 08:57:43AM -0800, Amir Mahdi Ghorbanian wrote:
> @@ -626,7 +628,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
> break;
> case 2: /* first byte after command */
> if (status == (I2C_SL_IRQ | RNW | RCVD)) {
> - udelay(33);
> + usleep_range(0, 33);
Why is this a valid range to sleep for for this device? Have you been
able to verify/test this?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
