Re: [PATCH 6/6] staging: most: Documentation: add information to driver_usage file
On Mi, 2018-12-12 at 17:31 +0300, Dan Carpenter wrote: > On Wed, Dec 12, 2018 at 01:15:31PM +0100, Christian Gromm wrote: > > > > 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. > So this patchset does break userspace... Which is allowed sometimes > in > staging, but it's better to point it out in the original commit which > causes the breakage. > The sound card layout that we have in mind is, from a system architecture point of view, the better one. Unfortunately, it does come with the cost of changing the configuration in one way or the other. Which is not really great, I agree on that. What I don't see is, why enhancing the way how user space interferes wi th the driver is such a terrible thing to do in staging. This should be the place to be to develop and change things (unless I totally misunderstood the concept behind it). > But really I don't like this API at all... It feels like something Fair enough. Is it just latest change that made you feel this way, or didn't you like the process in the first place? > from decades ago. There has to be a better way than this. > What if I would find a better way and change the API into something you like. Wouldn't that break user space too? Is this a dead end? How am I supposed to go on? > Unfortunately, I'm not clever enough to give you useful > suggestions... > thanks, Chris ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Obědové karty
Dobrý den, moderní příspěvek na stravování v podobě obědové karty, kterou lze použít ve více než 15 000 restauracích a gastronomických zařízení po celé zemi, je nástroj, který Vám pomůže poskytnout svým zaměstnancům nejžádanější benefit v ČR. Výzkum Světové zdravotnické organizace WHO naznačuje, že zaměstnanec, který dostává od svého zaměstnavatele příspěvek na jídlo, je efektivnější o 20 % a počet absencí klesá o 27 %. Výběrem našich obědových karet jako formy výhod zaměstnavatel získá nejen produktivnější a k práci více motivované zaměstnance, ale také finanční výhody, protože hodnota prostředků na kartě znamená úsporu 34 % při odvodech na sociálním a zdravotním pojištění zaměstnance a na dani z příjmu. Navíc je příspěvek na obědovou kartu zaměstnanci o 48 % výhodnější než příspěvek do mzdy. Mohli bych Vám poskytnout více informací o naší obědové kartě? Martin Kaspar Project Manager www.obedovekarty.cz ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Obědové karty
Dobrý den, moderní příspěvek na stravování v podobě obědové karty, kterou lze použít ve více než 15 000 restauracích a gastronomických zařízení po celé zemi, je nástroj, který Vám pomůže poskytnout svým zaměstnancům nejžádanější benefit v ČR. Výzkum Světové zdravotnické organizace WHO naznačuje, že zaměstnanec, který dostává od svého zaměstnavatele příspěvek na jídlo, je efektivnější o 20 % a počet absencí klesá o 27 %. Výběrem našich obědových karet jako formy výhod zaměstnavatel získá nejen produktivnější a k práci více motivované zaměstnance, ale také finanční výhody, protože hodnota prostředků na kartě znamená úsporu 34 % při odvodech na sociálním a zdravotním pojištění zaměstnance a na dani z příjmu. Navíc je příspěvek na obědovou kartu zaměstnanci o 48 % výhodnější než příspěvek do mzdy. Mohli bych Vám poskytnout více informací o naší obědové kartě? Martin Kaspar Project Manager www.obedovekarty.cz ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Obědové karty
Dobrý den, moderní příspěvek na stravování v podobě obědové karty, kterou lze použít ve více než 15 000 restauracích a gastronomických zařízení po celé zemi, je nástroj, který Vám pomůže poskytnout svým zaměstnancům nejžádanější benefit v ČR. Výzkum Světové zdravotnické organizace WHO naznačuje, že zaměstnanec, který dostává od svého zaměstnavatele příspěvek na jídlo, je efektivnější o 20 % a počet absencí klesá o 27 %. Výběrem našich obědových karet jako formy výhod zaměstnavatel získá nejen produktivnější a k práci více motivované zaměstnance, ale také finanční výhody, protože hodnota prostředků na kartě znamená úsporu 34 % při odvodech na sociálním a zdravotním pojištění zaměstnance a na dani z příjmu. Navíc je příspěvek na obědovou kartu zaměstnanci o 48 % výhodnější než příspěvek do mzdy. Mohli bych Vám poskytnout více informací o naší obědové kartě? Martin Kaspar Project Manager www.obedovekarty.cz ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: erofs: Add a blank line after declarations
On 2018/12/13 9:51, Gao Xiang wrote:
> Hi Sungkyung,
>
> On 2018/12/12 23:50, Sungkyung Kim wrote:
>> Fix a warning from checkpathch.pl: 'Missing a blank line after
>> declarations'
>>
>> Signed-off-by: Sungkyung Kim
>> ---
>> drivers/staging/erofs/inode.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
>> index 04c61a9d7b76..82b6ad5badff 100644
>> --- a/drivers/staging/erofs/inode.c
>> +++ b/drivers/staging/erofs/inode.c
>> @@ -278,6 +278,7 @@ struct inode *erofs_iget(struct super_block *sb,
>> if (inode->i_state & I_NEW) {
>> int err;
>> struct erofs_vnode *vi = EROFS_V(inode);
>> +
>
> Thanks for your patch, is there a only one 'Missing a blank line after
> declarations' in erofs?
>
> If not, could you have time check the other files? That is my personal
> thought, and
> I cc-ed the staging mailing list as well.
Agreed, it needs to check all files, and fix similar coding style issues in
one patch.
Thanks,
>
> Thanks,
> Gao Xiang
>
>> vi->nid = nid;
>>
>> err = fill_inode(inode, isdir);
>>
>
> .
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] arm64: hyperv: Add support for Hyper-V as a hypervisor
Hi Michael,
On 12/12/2018 05:00, Michael Kelley wrote:
> From: Marc Zyngier Sent: Friday, December 7, 2018
> 6:43 AM
>
>>> Add ARM64-specific code to enable Hyper-V. This code includes:
>>> * Detecting Hyper-V and initializing the guest/Hyper-V interface
>>> * Setting up Hyper-V's synthetic clocks
>>> * Making hypercalls using the HVC instruction
>>> * Setting up VMbus and stimer0 interrupts
>>> * Setting up kexec and crash handlers
>>
>> This commit message is a clear indication that this should be split in
>> at least 5 different patches.
>
> OK, I'll work on separating into multiple layered patches in the next
> version.
Thanks. This will definitely help the review process.
>>> +/*
>>> + * This variant of HVC invocation is for hv_get_vpreg and
>>> + * hv_get_vpreg_128. The input parameters are passed in registers
>>> + * along with a pointer in x4 to where the output result should
>>> + * be stored. The output is returned in x15 and x16. x18 is used as
>>> + * scratch space to avoid buildng a stack frame, as Hyper-V does
>>> + * not preserve registers x0-x17.
>>> + */
>>> +ENTRY(hv_do_hvc_fast_get)
>>> + mov x18, x4
>>> + hvc #1
>>> + str x15,[x18]
>>> + str x16,[x18,#8]
>>> + ret
>>> +ENDPROC(hv_do_hvc_fast_get)
>>
>> As Will said, this isn't a viable option. Please follow SMCCC 1.1.
>
> I'll have to start a conversation with the Hyper-V team about this.
> I don't know why they chose to use HVC #1 or this register scheme
> for output values. It may be tough to change at this point because
> there are Windows guests on Hyper-V for ARM64 that are already
> using this approach.
I appreciate you already have stuff in the wild, but there is definitely
a case to be made for supporting architecturally specified mechanisms in
a hypervisor, and SMCCC is definitely part of it (I'm certainly curious
of how you support the Spectre mitigation otherwise).
>
>>
>>> diff --git a/arch/arm64/hyperv/hv_init.c b/arch/arm64/hyperv/hv_init.c
>>> new file mode 100644
>>> index ..aa1a8c09d989
>>> --- /dev/null
>>> +++ b/arch/arm64/hyperv/hv_init.c
>>> @@ -0,0 +1,441 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +/*
>>> + * Initialization of the interface with Microsoft's Hyper-V hypervisor,
>>> + * and various low level utility routines for interacting with Hyper-V.
>>> + *
>>> + * Copyright (C) 2018, Microsoft, Inc.
>>> + *
>>> + * Author : Michael Kelley
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License version 2 as published
>>> + * by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful, but
>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
>>> + * NON INFRINGEMENT. See the GNU General Public License for more
>>> + * details.
>>> + */
>>> +
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +static boolhyperv_initialized;
>>> +struct ms_hyperv_info ms_hyperv;
>>> +EXPORT_SYMBOL_GPL(ms_hyperv);
>>
>> Who are the users of this structure? Should they go via accessors instead?
>
> The structure is an aggregation of several flags fields that describe a myriad
> of features and hints that may or may not be present on any particular version
> of Hyper-V, plus the max virtual processor ID values. Everything is read-only
> after initialization.
nit: please consider using a __ro_after_init annotation in that case.
> Most of the references are to test one of the flags. It's
> a judgment call, but there are a lot of different flags with long names, and
> writing accessors for each one doesn't seem to me to add any clarity.
Looking at that structure, it doesn't seem so bad, and you could easily
have generic accessors that take a flag as a parameter. Your call.
[...]
>>> +static struct clocksource hyperv_cs_msr = {
>>> + .name = "hyperv_clocksource_msr",
>>> + .rating = 400,
>>> + .read = read_hv_clock_msr,
>>> + .mask = CLOCKSOURCE_MASK(64),
>>> + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
>>> +};
>>> +
>>> +struct clocksource *hyperv_cs;
>>> +EXPORT_SYMBOL_GPL(hyperv_cs);
>>
>> Why? Who needs to poke this?
>
> It's referenced in the architecture independent driver code
> for the Hyper-V clocksource in drivers/hv/hv.c, and in the
> code to sync the time with the Hyper-V host in
> drivers/hv/hv_util.c.
Fair enough.
>>> +static int hv_cpu_init(unsigned int cpu)
>>> +{
>>> + u64 msr_vp_index;
>>> +
>>> + hv_get_vp_index(msr_vp_index);
>>> +
>>> + hv_vp_index[smp_processor_id()] = msr_vp_index;
>>> +
>>> + if (msr_vp_
Re: [PATCH 6/6] staging: most: Documentation: add information to driver_usage file
I'm not really complaining about breaking userspace, I'm complaining that I had to discover it by reading the code. It should have been mentioned in the commit message. We should probably just fold patch 1 & 6 together. I'm also not really complaining about this patch in particular when it comes to the API. The whole thing is a bit weird to me. I wish we could re-think the configuration from square one... It could be done in a later patch. I'm not going to NAK this patch if you resend it with the other small issues fixed. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: most: Documentation: add information to driver_usage file
On Wed, Dec 12, 2018 at 01:15:31PM +0100, Christian Gromm wrote: > This patch updates driver_usage.txt file to reflect the latest changes > that this patch set introduces. > > Signed-off-by: Christian Gromm > --- > 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 I would maybe prefer if the "create" command were on a separate line. Something like: +$ 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" >$(DRV_DIR)/add_link +$ echo "mdev0:ep83:sound:create" >$(DRV_DIR)/sound_card It's sort of a separate command in a way? I'm not sure... regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: most: sound: create one sound card w/ multiple PCM devices per MOST device
On Wed, Dec 12, 2018 at 03:31:13PM +, christian.gr...@microchip.com wrote:
> On Mi, 2018-12-12 at 17:21 +0300, Dan Carpenter wrote:
> > On Wed, Dec 12, 2018 at 01:15:26PM +0100, Christian Gromm wrote:
> > >
> > > 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.
> > >
> > > Signed-off-by: Christian Gromm
> > > ---
> > > drivers/staging/most/sound/sound.c | 127
> > > +
> > > 1 file changed, 87 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/drivers/staging/most/sound/sound.c
> > > b/drivers/staging/most/sound/sound.c
> > > index 89b02fc..41bcd2c 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, &dev_list, list) {
> > > + list_for_each_entry_safe(channel, tmp, &adpt->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(&buf, ".\n");
> > > if (!*sample_res)
> > > goto err;
> > > +
> > > + if (buf && !strcmp(buf, "create"))
> > > + *create = 1;
> > This comes from userspace, right? So we're adding a new API?
> >
>
> An additional field is added to the configuration parameter,
> which is provided by user space.
> This seemed to be less painful than adding a new sysfs
> file and make the configuration even more complicated.
Using sysfs to configure it might actually be the right thing...
Four commands: Add link, Add link, Add link, create.
> > We add new allocations to the &adpt_list, but then if
> > audio_probe_channel()
> > fails, then we free "adpt" in the error handling. But we don't
> > remove
> > the adpt from the list so now if we iterate through the list again
> > it's
> > a use after free.
> >
>
> The only location where "adpt" is being freed is inside of function
> release_adapter(). And there it gets removed from the list right before
> the call to kfree. Am I missing something?
My bad, I didn't read the code properly.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: most: sound: create one sound card w/ multiple PCM devices per MOST device
On Wed, Dec 12, 2018 at 01:15:26PM +0100, Christian Gromm wrote:
> @@ -571,6 +600,40 @@ static int audio_probe_channel(struct most_interface
> *iface, int channel_id,
> return -EINVAL;
> }
>
> + ret = split_arg_list(arg_list, &card_name, &ch_num, &sample_res,
> + &create);
> + if (ret < 0)
> + return ret;
> +
> + list_for_each_entry(adpt, &adpt_list, list) {
> + if (adpt->iface == iface && adpt->registered)
> + return -ENOSPC;
> + if (!adpt->registered) {
> + adpt->pcm_dev_idx++;
> + goto skip_adpt_alloc;
We haven't ensured the adpt->iface == iface.
> + }
> + }
Probably you want to say:
list_for_each_entry(adpt, &adpt_list, list) {
if (adpt->iface != iface)
continue;
if (adpt->registered)
return -ENOSPC;
adpt->pcm_dev_idx++;
goto skip_adpt_alloc;
}
But here again, I think I might prefer if allocating a new "adpt" were
and explicit command from user space as opposed to just a side effect of
registering a new iface.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: most: Documentation: add information to driver_usage file
On Thu, Dec 13, 2018 at 02:58:00PM +0300, Dan Carpenter wrote: > On Wed, Dec 12, 2018 at 01:15:31PM +0100, Christian Gromm wrote: > > This patch updates driver_usage.txt file to reflect the latest changes > > that this patch set introduces. > > > > Signed-off-by: Christian Gromm > > --- > > 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 > > I would maybe prefer if the "create" command were on a separate line. > Something like: > > +$ 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" >$(DRV_DIR)/add_link > +$ echo "mdev0:ep83:sound:create" >$(DRV_DIR)/sound_card > > It's sort of a separate command in a way? Why is sysfs being used for configuring things? Isn't that what configfs was created for? :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: most: sound: create one sound card w/ multiple PCM devices per MOST device
On Wed, Dec 12, 2018 at 03:31:13PM +, christian.gr...@microchip.com wrote: > An additional field is added to the configuration parameter, > which is provided by user space. > This seemed to be less painful than adding a new sysfs > file and make the configuration even more complicated. I think that adding a bunch of new sysfs files *and directories* is the way to go actually. echo "mdev0:ep01:sound:most51_playback.6x16" >$(DRV_DIR)/add_link I've never used the most driver but my guess is that $DRV_DIR is /sys/module/most/. In theory, you're supposed to write one word or number to sysfs files so this is sort of a misuse of the API. There should be an mdev0/ep01/sound/most51_playback/ directory. Maybe every ep01 channel will have a sound/ subdirectory or maybe we have to create it manually, I don't know. There should be a command to create an new adpt, and another command to add some commponents to it and a last command to register it with the kernel. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: most: sound: create one sound card w/ multiple PCM devices per MOST device
On Thu, Dec 13, 2018 at 03:38:03PM +0300, Dan Carpenter wrote: > On Wed, Dec 12, 2018 at 03:31:13PM +, christian.gr...@microchip.com wrote: > > An additional field is added to the configuration parameter, > > which is provided by user space. > > This seemed to be less painful than adding a new sysfs > > file and make the configuration even more complicated. > > I think that adding a bunch of new sysfs files *and directories* is the > way to go actually. Or configfs. But I think generally my email still applies in that we should break things up into separate create, add link, and register commands. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/11] 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 7308fa8..aa5ab1e 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
@@ -560,4 +559,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 8618805..e365fa0 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 8bd86e7..db2fede46 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
@@ -110,4 +109,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 b76ca5a..b6553ce 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
@@ -76,4 +75,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 08/11] staging: iio: adc: ad7606: Add OF device ID table
The driver does not have a struct of_device_id table, but supported
devices are registered via Device Trees. This patch adds and OF device
ID table.
Signed-off-by: Stefan Popa
---
drivers/staging/iio/adc/ad7606_par.c | 10 ++
drivers/staging/iio/adc/ad7606_spi.c | 10 ++
2 files changed, 20 insertions(+)
diff --git a/drivers/staging/iio/adc/ad7606_par.c
b/drivers/staging/iio/adc/ad7606_par.c
index 6269ee7..ac0c7b0 100644
--- a/drivers/staging/iio/adc/ad7606_par.c
+++ b/drivers/staging/iio/adc/ad7606_par.c
@@ -90,12 +90,22 @@ static const struct platform_device_id ad7606_driver_ids[]
= {
MODULE_DEVICE_TABLE(platform, ad7606_driver_ids);
+static const struct of_device_id ad7606_of_match[] = {
+ { .compatible = "adi,ad7605-4" },
+ { .compatible = "adi,ad7606-4" },
+ { .compatible = "adi,ad7606-6" },
+ { .compatible = "adi,ad7606-8" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, ad7606_of_match);
+
static struct platform_driver ad7606_driver = {
.probe = ad7606_par_probe,
.id_table = ad7606_driver_ids,
.driver = {
.name= "ad7606",
.pm = AD7606_PM_OPS,
+ .of_match_table = ad7606_of_match,
},
};
diff --git a/drivers/staging/iio/adc/ad7606_spi.c
b/drivers/staging/iio/adc/ad7606_spi.c
index 9291598..2608d34 100644
--- a/drivers/staging/iio/adc/ad7606_spi.c
+++ b/drivers/staging/iio/adc/ad7606_spi.c
@@ -57,9 +57,19 @@ static const struct spi_device_id ad7606_id[] = {
};
MODULE_DEVICE_TABLE(spi, ad7606_id);
+static const struct of_device_id ad7606_of_match[] = {
+ { .compatible = "adi,ad7605-4" },
+ { .compatible = "adi,ad7606-4" },
+ { .compatible = "adi,ad7606-6" },
+ { .compatible = "adi,ad7606-8" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, ad7606_of_match);
+
static struct spi_driver ad7606_driver = {
.driver = {
.name = "ad7606",
+ .of_match_table = ad7606_of_match,
.pm = AD7606_PM_OPS,
},
.probe = ad7606_spi_probe,
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/11] staging: iio: adc: ad7606: Use wait-for-completion handler
This patch replaces the use of wait_event_interruptible() with
wait_for_completion_timeout() when reading the result of a single
conversion. In this way, if the interrupt never occurs, the program will
not remain blocked.
Signed-off-by: Stefan Popa
---
drivers/staging/iio/adc/ad7606.c | 14 +++---
drivers/staging/iio/adc/ad7606.h | 6 ++
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index aa5ab1e..4b1bc20 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -118,12 +118,13 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev,
unsigned int ch)
struct ad7606_state *st = iio_priv(indio_dev);
int ret;
- st->done = false;
gpiod_set_value(st->gpio_convst, 1);
-
- ret = wait_event_interruptible(st->wq_data_avail, st->done);
- if (ret)
+ ret = wait_for_completion_timeout(&st->completion,
+ msecs_to_jiffies(1000));
+ if (!ret) {
+ ret = -ETIMEDOUT;
goto error_ret;
+ }
ret = ad7606_read_samples(st);
if (ret == 0)
@@ -388,8 +389,7 @@ static irqreturn_t ad7606_interrupt(int irq, void *dev_id)
if (iio_buffer_enabled(indio_dev)) {
schedule_work(&st->poll_work);
} else {
- st->done = true;
- wake_up_interruptible(&st->wq_data_avail);
+ complete(&st->completion);
}
return IRQ_HANDLED;
@@ -473,7 +473,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem
*base_address,
indio_dev->channels = st->chip_info->channels;
indio_dev->num_channels = st->chip_info->num_channels;
- init_waitqueue_head(&st->wq_data_avail);
+ init_completion(&st->completion);
ret = ad7606_reset(st);
if (ret)
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index e365fa0..cf20ca2 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -28,11 +28,9 @@ struct ad7606_chip_info {
* @regregulator info for the the power supply of the device
* @poll_work work struct for continuously reading data from the
device
* into an IIO triggered buffer
- * @wq_data_avail wait queue struct for buffer mode
* @bops bus operations (SPI or parallel)
* @range voltage range selection, selects which scale to apply
* @oversampling oversampling selection
- * @done marks whether reading data is done
* @base_address address from where to read data in parallel operation
* @lock protect sensor state from concurrent accesses to GPIOs
* @gpio_convstGPIO descriptor for conversion start signal (CONVST)
@@ -43,6 +41,7 @@ struct ad7606_chip_info {
* @gpio_frstdata GPIO descriptor for reading from device when data
* is being read on the first channel
* @gpio_osGPIO descriptors to control oversampling on the device
+ * @complete completion to indicate end of conversion
* @data buffer for reading data from the device
*/
@@ -51,11 +50,9 @@ struct ad7606_state {
const struct ad7606_chip_info *chip_info;
struct regulator*reg;
struct work_struct poll_work;
- wait_queue_head_t wq_data_avail;
const struct ad7606_bus_ops *bops;
unsigned intrange;
unsigned intoversampling;
- booldone;
void __iomem*base_address;
struct mutexlock; /* protect sensor state */
@@ -65,6 +62,7 @@ struct ad7606_state {
struct gpio_desc*gpio_standby;
struct gpio_desc*gpio_frstdata;
struct gpio_descs *gpio_os;
+ struct completion completion;
/*
* DMA (thus cache coherency maintenance) requires the
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Example code to access IRQ 5 on ISA card (using uio_pdrv_genirq ?)
Hi I would like to make use of an ISA card which generates IRQ 5 preferably using user-space code only, if possible. Does anybody know of any example code using uio_pdrv_genirq which could be adapted for this purpose? Or, does anyone have a simple example kernel module? Thanks! Andrew ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/11] staging: iio: adc: ad7606: Use devm functions in probe
Switch to devm version of request_irq, iio_triggered_buffer_setup,
iio_device_register. To avoid potential ordering issues in probe,
devm_add_action_or_reset() is used for the regulator_disable(). This
simplifies the code and decreases the chance of bugs.
Signed-off-by: Stefan Popa
---
drivers/staging/iio/adc/ad7606.c | 59 +---
drivers/staging/iio/adc/ad7606.h | 1 -
drivers/staging/iio/adc/ad7606_par.c | 6
drivers/staging/iio/adc/ad7606_spi.c | 6
4 files changed, 21 insertions(+), 51 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 4b1bc20..7191d51 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -417,6 +417,13 @@ static const struct iio_info ad7606_info_range = {
.attrs = &ad7606_attribute_group_range,
};
+static void ad7606_regulator_disable(void *data)
+{
+ struct ad7606_state *st = data;
+
+ regulator_disable(st->reg);
+}
+
int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
const char *name, unsigned int id,
const struct ad7606_bus_ops *bops)
@@ -430,6 +437,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem
*base_address,
return -ENOMEM;
st = iio_priv(indio_dev);
+ dev_set_drvdata(dev, indio_dev);
st->dev = dev;
mutex_init(&st->lock);
@@ -450,11 +458,15 @@ int ad7606_probe(struct device *dev, int irq, void
__iomem *base_address,
return ret;
}
+ ret = devm_add_action_or_reset(dev, ad7606_regulator_disable, st);
+ if (ret)
+ return ret;
+
st->chip_info = &ad7606_chip_info_tbl[id];
ret = ad7606_request_gpios(st);
if (ret)
- goto error_disable_reg;
+ return ret;
indio_dev->dev.parent = dev;
if (st->gpio_os) {
@@ -479,50 +491,21 @@ int ad7606_probe(struct device *dev, int irq, void
__iomem *base_address,
if (ret)
dev_warn(st->dev, "failed to RESET: no RESET GPIO specified\n");
- ret = request_irq(irq, ad7606_interrupt, IRQF_TRIGGER_FALLING, name,
- indio_dev);
- if (ret)
- goto error_disable_reg;
-
- ret = iio_triggered_buffer_setup(indio_dev, &ad7606_trigger_handler,
-NULL, NULL);
+ ret = devm_request_irq(dev, irq, ad7606_interrupt, IRQF_TRIGGER_FALLING,
+ name, indio_dev);
if (ret)
- goto error_free_irq;
+ return ret;
- ret = iio_device_register(indio_dev);
+ ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+ &ad7606_trigger_handler,
+ NULL, NULL);
if (ret)
- goto error_unregister_ring;
-
- dev_set_drvdata(dev, indio_dev);
-
- return 0;
-error_unregister_ring:
- iio_triggered_buffer_cleanup(indio_dev);
-
-error_free_irq:
- free_irq(irq, indio_dev);
+ return ret;
-error_disable_reg:
- regulator_disable(st->reg);
- return ret;
+ return devm_iio_device_register(dev, indio_dev);
}
EXPORT_SYMBOL_GPL(ad7606_probe);
-int ad7606_remove(struct device *dev, int irq)
-{
- struct iio_dev *indio_dev = dev_get_drvdata(dev);
- struct ad7606_state *st = iio_priv(indio_dev);
-
- iio_device_unregister(indio_dev);
- iio_triggered_buffer_cleanup(indio_dev);
-
- free_irq(irq, indio_dev);
- regulator_disable(st->reg);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(ad7606_remove);
-
#ifdef CONFIG_PM_SLEEP
static int ad7606_suspend(struct device *dev)
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index cf20ca2..70486ef 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -84,7 +84,6 @@ struct ad7606_bus_ops {
int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
const char *name, unsigned int id,
const struct ad7606_bus_ops *bops);
-int ad7606_remove(struct device *dev, int irq);
enum ad7606_supported_device_ids {
ID_AD7605_4,
diff --git a/drivers/staging/iio/adc/ad7606_par.c
b/drivers/staging/iio/adc/ad7606_par.c
index db2fede46..6269ee7 100644
--- a/drivers/staging/iio/adc/ad7606_par.c
+++ b/drivers/staging/iio/adc/ad7606_par.c
@@ -71,11 +71,6 @@ static int ad7606_par_probe(struct platform_device *pdev)
&ad7606_par8_bops);
}
-static int ad7606_par_remove(struct platform_device *pdev)
-{
- return ad7606_remove(&pdev->dev, platform_get_irq(pdev, 0));
-}
-
static const struct platform_device_id ad7606_driver_ids[] = {
{
.name = "ad7605-4",
@@ -97,7 +92,6 @@ MODULE_DEVICE_TABLE(platform, ad7606_driver
[PATCH 07/11] staging: iio: adc: ad7606: Use vendor prefix for DT properties
The 'adi' vendor prefix needs to be added to conversion-start, range,
first-data and oversampling-ratio properties.
Signed-off-by: Stefan Popa
---
drivers/staging/iio/adc/ad7606.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 0925379..3355301 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -322,7 +322,7 @@ static int ad7606_request_gpios(struct ad7606_state *st)
{
struct device *dev = st->dev;
- st->gpio_convst = devm_gpiod_get(dev, "conversion-start",
+ st->gpio_convst = devm_gpiod_get(dev, "adi,conversion-start",
GPIOD_OUT_LOW);
if (IS_ERR(st->gpio_convst))
return PTR_ERR(st->gpio_convst);
@@ -331,7 +331,8 @@ static int ad7606_request_gpios(struct ad7606_state *st)
if (IS_ERR(st->gpio_reset))
return PTR_ERR(st->gpio_reset);
- st->gpio_range = devm_gpiod_get_optional(dev, "range", GPIOD_OUT_LOW);
+ st->gpio_range = devm_gpiod_get_optional(dev, "adi,range",
+GPIOD_OUT_LOW);
if (IS_ERR(st->gpio_range))
return PTR_ERR(st->gpio_range);
@@ -340,7 +341,7 @@ static int ad7606_request_gpios(struct ad7606_state *st)
if (IS_ERR(st->gpio_standby))
return PTR_ERR(st->gpio_standby);
- st->gpio_frstdata = devm_gpiod_get_optional(dev, "first-data",
+ st->gpio_frstdata = devm_gpiod_get_optional(dev, "adi,first-data",
GPIOD_IN);
if (IS_ERR(st->gpio_frstdata))
return PTR_ERR(st->gpio_frstdata);
@@ -348,7 +349,8 @@ static int ad7606_request_gpios(struct ad7606_state *st)
if (!st->chip_info->has_oversampling)
return 0;
- st->gpio_os = devm_gpiod_get_array_optional(dev, "oversampling-ratio",
+ st->gpio_os = devm_gpiod_get_array_optional(dev,
+ "adi,oversampling-ratio",
GPIOD_OUT_LOW);
return PTR_ERR_OR_ZERO(st->gpio_os);
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/11] 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 | 24 +--- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig index fc23059..af1bad8 100644 --- a/drivers/staging/iio/adc/Kconfig +++ b/drivers/staging/iio/adc/Kconfig @@ -4,35 +4,29 @@ menu "Analog to digital converters" config AD7606 - tristate "Analog Devices AD7606 ADC driver" + tristate depends on GPIOLIB || COMPILE_TEST depends on HAS_IOMEM 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" + 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
[PATCH 06/11] staging: iio: adc: ad7606: Use find_closest() macro
When looking for the available scale or oversampling ratio, it is better
to use the find_closest() macro. This simplifies the code and also does
not require an exact value to be entered from the user space.
Signed-off-by: Stefan Popa
---
drivers/staging/iio/adc/ad7606.c | 58 +++-
1 file changed, 22 insertions(+), 36 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 13aeeec..0925379 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
@@ -30,8 +31,12 @@
* 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][2] = {
- {0, 152588}, {0, 305176}
+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)
@@ -148,8 +153,8 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
*val = (short)ret;
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
- *val = scale_avail[st->range][0];
- *val2 = scale_avail[st->range][1];
+ *val = 0;
+ *val2 = scale_avail[st->range];
return IIO_VAL_INT_PLUS_MICRO;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
*val = st->oversampling;
@@ -165,8 +170,8 @@ static ssize_t in_voltage_scale_available_show(struct
device *dev,
int i, len = 0;
for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
- len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%06u ",
-scale_avail[i][0], scale_avail[i][1]);
+ len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ",
+scale_avail[i]);
buf[len - 1] = '\n';
@@ -175,18 +180,6 @@ static ssize_t in_voltage_scale_available_show(struct
device *dev,
static IIO_DEVICE_ATTR_RO(in_voltage_scale_available, 0);
-static int ad7606_oversampling_get_index(unsigned int val)
-{
- unsigned char supported[] = {1, 2, 4, 8, 16, 32, 64};
- int i;
-
- for (i = 0; i < ARRAY_SIZE(supported); i++)
- if (val == supported[i])
- return i;
-
- return -EINVAL;
-}
-
static int ad7606_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int val,
@@ -195,36 +188,29 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
{
struct ad7606_state *st = iio_priv(indio_dev);
DECLARE_BITMAP(values, 3);
- int ret, i;
+ int i;
switch (mask) {
case IIO_CHAN_INFO_SCALE:
- ret = -EINVAL;
mutex_lock(&st->lock);
- for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
- if (val2 == scale_avail[i][1]) {
- gpiod_set_value(st->gpio_range, i);
- st->range = i;
-
- ret = 0;
- break;
- }
+ i = find_closest(val2, scale_avail, ARRAY_SIZE(scale_avail));
+ gpiod_set_value(st->gpio_range, i);
+ st->range = i;
mutex_unlock(&st->lock);
- return ret;
+ return 0;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
if (val2)
return -EINVAL;
- ret = ad7606_oversampling_get_index(val);
- if (ret < 0)
- return ret;
+ i = find_closest(val, ad7606_oversampling_avail,
+ARRAY_SIZE(ad7606_oversampling_avail));
- values[0] = ret;
+ values[0] = i;
mutex_lock(&st->lock);
- gpiod_set_array_value(3, st->gpio_os->desc, st->gpio_os->info,
- values);
- st->oversampling = val;
+ gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
+ st->gpio_os->info, values);
+ st->oversampling = ad7606_oversampling_avail[i];
mutex_unlock(&st->lock);
return 0;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/11] 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 | 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(&st->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(&st->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(&st->poll_work);
+ gpiod_set_value(st->gpio_convst, 0);
+ iio_trigger_poll_chained(st->trig);
} else {
complete(&st->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);
+
+ 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);
+ int ret;
+
+ reinit_completion(&st->completion);
+ gpiod_set_value(st->gpio_convst, 1);
+ ret = wait_for_completion_timeout(&st->completion,
+ msecs_to_jiffies(1000));
+ 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 = &ad7606_buffer_postenable,
+ .predisable = &ad7606_buffer_predisable,
+};
+
static const struct iio_info ad7606_info_no_os_or_range = {
.read_raw = &ad7606_read_raw,
+ .validate_trigger = &ad7606_validate_tri
[PATCH 00/11] staging: iio: ad7606: Move out of staging
This series attempts to clean up the driver according to the feedback received during review and finally moves it out of staging. Stefan Popa (11): staging: iio: adc: ad7606: Simplify the Kconfing menu staging: iio: adc: ad7606: Use SPDX identifier staging: iio: adc: ad7606: Use wait-for-completion handler staging: iio: adc: ad7606: Use devm functions in probe staging: iio: adc: ad7606: Add support for threaded irq staging: iio: adc: ad7606: Use find_closest() macro staging: iio: adc: ad7606: Use vendor prefix for DT properties staging: iio: adc: ad7606: Add OF device ID table 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| 28 + drivers/iio/adc/Makefile | 3 + drivers/iio/adc/ad7606.c | 588 + 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 | 563 drivers/staging/iio/adc/ad7606.h | 106 drivers/staging/iio/adc/ad7606_par.c | 113 drivers/staging/iio/adc/ad7606_spi.c | 79 --- 14 files changed, 978 insertions(+), 899 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 09/11] 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 3355301..5733760 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 b238e9b..40433af 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 ac0c7b0..32c7069 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);
MODULE_AUTHOR("Mic
Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
Hi Steve,
On Tue, 2018-10-16 at 17:00 -0700, Steve Longerbeam wrote:
> The function ipu_csi_init_interface() was inverting the F-bit for
> NTSC case, in the CCIR_CODE_1/2 registers. The result being that
> for NTSC bottom-top field order, the CSI would swap fields and
> capture in top-bottom order.
>
> Instead, base field swap on the field order of the input to the CSI,
> and the field order of the requested output. If the input/output
> fields are sequential but different, swap fields, otherwise do
> not swap. This requires passing both the input and output mbus
> frame formats to ipu_csi_init_interface().
>
> Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
> that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
> possibly interlaced BT.1120 in the future).
>
> When detecting input video standard from the input frame width/height,
> make sure to double height if input field type is alternate, since
> in that case input height only includes lines for one field.
>
> Signed-off-by: Steve Longerbeam
> ---
> Changes since v4:
> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
> by Philipp Zabel.
> - Fixed a regression in csi_setup(), caught by Philipp.
> ---
> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++---
> drivers/staging/media/imx/imx-media-csi.c | 17 +---
> include/video/imx-ipu-v3.h| 3 +-
> 3 files changed, 88 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index aa0e30a2ba18..4a15e513fa05 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct
> ipu_csi_bus_config *cfg, u32 mbus_code,
> return 0;
> }
>
> +/* translate alternate field mode based on given standard */
> +static inline enum v4l2_field
> +ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
> +{
> + return (field != V4L2_FIELD_ALTERNATE) ? field :
> + ((std & V4L2_STD_525_60) ?
> + V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
> +}
> +
> /*
> * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
> */
> @@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config
> *csicfg,
> return 0;
> }
>
> +static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
> +struct v4l2_mbus_framefmt *infmt,
> +struct v4l2_mbus_framefmt *outfmt,
infmt and outfmt parameters could be const.
> +v4l2_std_id std)
> +{
> + enum v4l2_field infield, outfield;
> + bool swap_fields;
> +
> + /* get translated field type of input and output */
> + infield = ipu_csi_translate_field(infmt->field, std);
> + outfield = ipu_csi_translate_field(outfmt->field, std);
> +
> + /*
> + * Write the H-V-F codes the CSI will match against the
> + * incoming data for start/end of active and blanking
> + * field intervals. If input and output field types are
> + * sequential but not the same (one is SEQ_BT and the other
> + * is SEQ_TB), swap the F-bit so that the CSI will capture
> + * field 1 lines before field 0 lines.
> + */
> + swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
> +V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
> +infield != outfield);
> +
> + if (!swap_fields) {
> + /*
> + * Field0BlankEnd = 110, Field0BlankStart = 010
> + * Field0ActiveEnd = 100, Field0ActiveStart = 000
> + * Field1BlankEnd = 111, Field1BlankStart = 011
> + * Field1ActiveEnd = 101, Field1ActiveStart = 001
> + */
> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> + } else {
> + dev_dbg(csi->ipu->dev, "capture field swap\n");
> +
> + /* same as above but with F-bit inverted */
> + ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> + }
> +
> + ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3);
> +
> + return 0;
> +}
> +
> +
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> -struct v4l2_mbus_framefmt *mbus_fmt)
> +struct v4l2_mbus_framefmt *infmt,
> +struct v4l2_mbus_framefmt *outfmt)
> {
> struct ipu_csi_bus_config cfg;
> unsigned long flags;
> u32 width, height, data = 0;
> + v4l2_std_id std;
> int ret;
>
> - ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
> + ret = fill_csi_bus_cfg(&cfg, mbus_cfg
[PATCH 10/11] 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 | 28 ++
drivers/iio/adc/Makefile | 3 +
drivers/iio/adc/ad7606.c | 588 +++
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 | 28 --
drivers/staging/iio/adc/Makefile | 4 -
drivers/staging/iio/adc/ad7606.c | 588 ---
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, 912 insertions(+), 906 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 d904229..7256ce6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -853,6 +853,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 da9644b..9c0b50b 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -69,6 +69,34 @@ config AD7476
To compile this driver as a module, choose M here: the
module will be called ad7476.
+config AD7606
+ tristate
+ depends on GPIOLIB || COMPILE_TEST
+ depends on HAS_IOMEM
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+
+config AD7606_IFACE_PARALLEL
+ tristate "Analog Devices AD7606 ADC driver with parallel interface
support"
+ select AD7606
+ help
+ Say yes here to build parallel interface support for Analog Devices:
+ ad7605-4, ad7606, ad7606-6, ad7606-4 analog to digital converters
(ADC).
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad7606_parallel.
+
+config AD7606_IFACE_SPI
+ tristate "Analog Devices AD7606 ADC driver with spi interface support"
+ depends on SPI
+ select AD7606
+ help
+ Say yes here to build spi interface support for Analog Devices:
+ ad7605-4, ad7606, ad7606-6, ad7606-4 analog to digital converters
(ADC).
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad7606_spi.
+
config AD7766
tristate "Analog Devices AD7766/AD7767 ADC driver"
depends on SPI_MASTER
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 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..5733760
--- /dev/null
+++ b/drivers/iio/adc/ad7606.c
@@ -0,0 +1,588 @@
+// 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_
[PATCH 11/11] 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 = <&adc_vref>;
+
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <&gpio>;
+
+ adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>;
+ adi,first-data-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
+ adi,oversampling-ratio-gpios = <&gpio 18 GPIO_ACTIVE_HIGH
+ &gpio 23 GPIO_ACTIVE_HIGH
+ &gpio 26 GPIO_ACTIVE_HIGH>;
+ standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 7256ce6..798e9a2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -859,6 +859,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
Re: [PATCH 02/11] staging: iio: adc: ad7606: Use SPDX identifier
On Thu, Dec 13, 2018 at 02:46:14PM +0200, Stefan Popa wrote: > 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 7308fa8..aa5ab1e 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. You've changed the license from 2 to 2+. That's literally breaking the law. As in country laws with lawsuits and fines and what not. Please be very careful with this stuff. regards, dan carpenter ___ 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 Thu, Dec 13, 2018 at 02:46:17PM +0200, Stefan Popa wrote:
> +static int ad7606_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct ad7606_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + reinit_completion(&st->completion);
> + gpiod_set_value(st->gpio_convst, 1);
> + ret = wait_for_completion_timeout(&st->completion,
> + msecs_to_jiffies(1000));
We don't use this "ret". It will introduce a set but not used warning
on new GCCs or static checkers or something.
> + gpiod_set_value(st->gpio_convst, 0);
> +
> + return iio_triggered_buffer_predisable(indio_dev);
> +}
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Hello
Hello, Linux Driver Project. My name is Andrii. I`m interesting in wireless driver projects i.e 802.11n/ac/ maybe ax. If there are any projects, related with it (new drivers, legacy support), I will be happy to join. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: most: sound: create one sound card w/ multiple PCM devices per MOST device
On Do, 2018-12-13 at 15:16 +0300, Dan Carpenter wrote:
> On Wed, Dec 12, 2018 at 01:15:26PM +0100, Christian Gromm wrote:
> >
> > @@ -571,6 +600,40 @@ static int audio_probe_channel(struct
> > most_interface *iface, int channel_id,
> > return -EINVAL;
> > }
> >
> > + ret = split_arg_list(arg_list, &card_name, &ch_num,
> > &sample_res,
> > + &create);
> > + if (ret < 0)
> > + return ret;
> > +
> > + list_for_each_entry(adpt, &adpt_list, list) {
> > + if (adpt->iface == iface && adpt->registered)
> > + return -ENOSPC;
> > + if (!adpt->registered) {
> > + adpt->pcm_dev_idx++;
> > + goto skip_adpt_alloc;
> We haven't ensured the adpt->iface == iface.
>
> >
> > + }
> > + }
> Probably you want to say:
>
> list_for_each_entry(adpt, &adpt_list, list) {
> if (adpt->iface != iface)
> continue;
> if (adpt->registered)
> return -ENOSPC;
> adpt->pcm_dev_idx++;
> goto skip_adpt_alloc;
> }
>
I do.
> But here again, I think I might prefer if allocating a new "adpt"
> were
> and explicit command from user space as opposed to just a side effect
> of
> registering a new iface.
>
Sounds reasonable. But, problem here is that we want the
process of how channels are linked to be independent from the
component that is being used. That's when things start to
become complicated.
In other words, I don't want the sound module to be configured
in a different way than the cdev module, networking module or the
v4l2 module is. The goal was to have only sysfs files that
are generic (->default attrs of most_core mod) and apply to all
components.
The latest change of the sound module creates the need of
signaling that the user is done with the config of the ALSA card.
The quickest way to achieve this, was to expand the "module
parameter" field (which we already have) with the "create" flag
that is passed to the module when a channel is being probed as a
trigger.
Because no other module needs such a trigger, creating a dedicated
sysfs file for this purpose seemed kind of weird, especially when
the sound module isn't loaded at all.
Maybe the sound module should create its own file, but since it
has no struct device embedded, I'm not sure how to achieve that :/
thanks,
Chris
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] mt7621-mmc/sd.c: Fix some coding style issues
Fix some "bigger" issues reported by checkpatch.pl:
- Block comments use * on subsequent lines
- Blank lines aren't necessary before a close brace
- Block comments use a trailing */ on a separate line
- Alignment should match open parenthesis
- void function return statements are not generally useful
- char * array declaration might be better as static const
- printk() should include KERN_ facility level:
replace printk() with pr_debug() to include facility level
- Some spelling mistakes
Signed-off-by: Jona Crasselt
Signed-off-by: Felix Windsheimer
---
drivers/staging/mt7621-mmc/sd.c | 62 -
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 7b66f9b0a094..e137ce2c9f44 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -78,7 +78,8 @@
#define CMD_TIMEOUT (HZ / 10) /* 100ms */
#define DAT_TIMEOUT (HZ / 2 * 5) /* 500ms x5 */
-#define MAX_DMA_CNT (64 * 1024 - 512) /* a single transaction for
WIFI may be 50K*/
+/* a single transaction for WIFI may be 50K*/
+#define MAX_DMA_CNT (64 * 1024 - 512)
#define MAX_GPD_NUM (1 + 1) /* one null gpd */
#define MAX_BD_NUM (1024)
@@ -318,9 +319,9 @@ static void msdc_abort_data(struct msdc_host *host)
#ifdef CONFIG_PM
/*
- register as callback function of WIFI(combo_sdio_register_pm) .
- can called by msdc_drv_suspend/resume too.
-*/
+ * register as callback function of WIFI(combo_sdio_register_pm) .
+ * can called by msdc_drv_suspend/resume too.
+ */
static void msdc_pm(pm_message_t state, void *data)
{
struct msdc_host *host = (struct msdc_host *)data;
@@ -351,7 +352,6 @@ static void msdc_pm(pm_message_t state, void *data)
host->suspend = 0;
host->pm_state = state;
-
}
}
#endif
@@ -685,7 +685,7 @@ static int msdc_do_request(struct mmc_host *mmc, struct
mmc_request *mrq)
if (read) {
if ((host->timeout_ns != data->timeout_ns) ||
- (host->timeout_clks != data->timeout_clks)) {
+ (host->timeout_clks != data->timeout_clks)) {
msdc_set_timeout(host, data->timeout_ns,
data->timeout_clks);
}
}
@@ -711,7 +711,8 @@ static int msdc_do_request(struct mmc_host *mmc, struct
mmc_request *mrq)
goto done;
/* for read, the data coming too fast, then CRC error
- start DMA no business with CRC. */
+* start DMA no business with CRC.
+*/
//init_completion(&host->xfer_done);
msdc_dma_start(host);
@@ -794,9 +795,10 @@ static int msdc_tune_cmdrsp(struct msdc_host *host, struct
mmc_command *cmd)
u32 skip = 1;
/* don't support 3.0 now
- 1: R_SMPL[1]
- 2: PAD_CMD_RESP_RXDLY[26:22]
- ==*/
+* 1: R_SMPL[1]
+* 2: PAD_CMD_RESP_RXDLY[26:22]
+* ==
+*/
// save the previous tune result
sdr_get_field(host->base + MSDC_IOCON, MSDC_IOCON_RSPL, &orig_rsmpl);
@@ -1188,8 +1190,6 @@ static void msdc_ops_request(struct mmc_host *mmc, struct
mmc_request *mrq)
spin_unlock(&host->lock);
mmc_request_done(mmc, mrq);
-
- return;
}
/* called by ops.set_ios */
@@ -1223,26 +1223,26 @@ static void msdc_ops_set_ios(struct mmc_host *mmc,
struct mmc_ios *ios)
u32 ddr = 0;
#ifdef MT6575_SD_DEBUG
- static char *vdd[] = {
+ static const char * const vdd[] = {
"1.50v", "1.55v", "1.60v", "1.65v", "1.70v", "1.80v", "1.90v",
"2.00v", "2.10v", "2.20v", "2.30v", "2.40v", "2.50v", "2.60v",
"2.70v", "2.80v", "2.90v", "3.00v", "3.10v", "3.20v", "3.30v",
"3.40v", "3.50v", "3.60v"
};
- static char *power_mode[] = {
+ static const char * const power_mode[] = {
"OFF", "UP", "ON"
};
- static char *bus_mode[] = {
+ static const char * const bus_mode[] = {
"UNKNOWN", "OPENDRAIN", "PUSHPULL"
};
- static char *timing[] = {
+ static const char * const timing[] = {
"LEGACY", "MMC_HS", "SD_HS"
};
- printk("SET_IOS: CLK(%dkHz), BUS(%s), BW(%u), PWR(%s), VDD(%s),
TIMING(%s)",
- ios->clock / 1000, bus_mode[ios->bus_mode],
- (ios->bus_width == MMC_BUS_WIDTH_4) ? 4 : 1,
- power_mode[ios->power_mode], vdd[ios->vdd],
timing[ios->timing]);
+ pr_debug("SET_IOS: CLK(%dkHz), BUS(%s), BW(%u), PWR(%s), VDD(%s),
TIMING(%s)",
+ios->clock / 1000, bus_mode[ios->bus_mode],
+(ios->bus_width == MMC_BUS_WIDTH_4) ? 4 : 1,
+
Re: [PATCH 1/6] staging: most: sound: create one sound card w/ multiple PCM devices per MOST device
On Do, 2018-12-13 at 15:38 +0300, Dan Carpenter wrote: > On Wed, Dec 12, 2018 at 03:31:13PM +, Christian.Gromm@microchip.c > om wrote: > > > > An additional field is added to the configuration parameter, > > which is provided by user space. > > This seemed to be less painful than adding a new sysfs > > file and make the configuration even more complicated. > I think that adding a bunch of new sysfs files *and directories* is > the > way to go actually. > > echo "mdev0:ep01:sound:most51_playback.6x16" >$(DRV_DIR)/add_link > > I've never used the most driver but my guess is that $DRV_DIR is > /sys/module/most/. In theory, you're supposed to write one word or > number to sysfs files so this is sort of a misuse of the API. > Well, you have to have a MOST or INICnet Infotainment network on your desk to use the driver. Actually, the $DRV_DIR is /sys/bus/most/drivers/most_core. And if the driver is loaded and a MOST device is attached that features two channels we get: /sys/bus/most/ ├── devices │ └── mdev0 -> ../../../devices/most_bus/mdev0 ├── drivers │ └── most_core │ ├── add_link │ ├── bind │ ├── components │ ├── links │ ├── mdev0 -> ../../../../devices/most_bus/mdev0 │ ├── remove_link │ ├── uevent │ └── unbind ├── drivers_autoprobe ├── drivers_probe └── uevent Having a new default attr under $DRV_DIR that is exclusively used for the sound module is not really nice, unless it appears only in case the sound module itself is loaded. But as I said, I'm not sure if we can establish that with the current struct core_component as it lacks the struct device. thanks, Chris ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/11] staging: iio: adc: ad7606: Simplify the Kconfing menu
On Thu, Dec 13, 2018 at 8:18 AM Stefan Popa wrote: > > 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 | 24 +--- > 1 file changed, 9 insertions(+), 15 deletions(-) > > diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig > index fc23059..af1bad8 100644 > --- a/drivers/staging/iio/adc/Kconfig > +++ b/drivers/staging/iio/adc/Kconfig > @@ -4,35 +4,29 @@ > menu "Analog to digital converters" > > config AD7606 > - tristate "Analog Devices AD7606 ADC driver" > + tristate > depends on GPIOLIB || COMPILE_TEST > depends on HAS_IOMEM > 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" > + select AD7606 I don't think this works because the depends on for AD7606 may not be satisfied. Moving the HAS_IOMEM here would help and either drop GPIOLIB or move that too. Rob ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: most: Documentation: add information to driver_usage file
On Do, 2018-12-13 at 14:54 +0300, Dan Carpenter wrote: > I'm not really complaining about breaking userspace, I'm complaining > that I had to discover it by reading the code. It should have been > mentioned in the commit message. We should probably just fold > patch 1 & 6 together. > Ok, I probably should have mentioned this in the cover letter. Thought the usage file addition is enough. Sorry for the inconvenience. > I'm also not really complaining about this patch in particular when > it > comes to the API. The whole thing is a bit weird to me. I wish we > could re-think the configuration from square one... > > It could be done in a later patch. I'm not going to NAK this patch > if > you resend it with the other small issues fixed. > Greg put configfs on the table. But this is like a total redesign of the driver interface which will take some time. thanks, Chris ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: most: Documentation: add information to driver_usage file
On Do, 2018-12-13 at 13:32 +0100, Greg KH wrote: > On Thu, Dec 13, 2018 at 02:58:00PM +0300, Dan Carpenter wrote: > > > > On Wed, Dec 12, 2018 at 01:15:31PM +0100, Christian Gromm wrote: > > > > > > This patch updates driver_usage.txt file to reflect the latest > > > changes > > > that this patch set introduces. > > > > > > Signed-off-by: Christian Gromm > > > --- > > > 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 > > I would maybe prefer if the "create" command were on a separate > > line. > > Something like: > > > > +$ 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" > > >$(DRV_DIR)/add_link > > +$ echo "mdev0:ep83:sound:create" >$(DRV_DIR)/sound_card > > > > It's sort of a separate command in a way? > Why is sysfs being used for configuring things? Isn't that what > configfs was created for? :) > Omg, that would be one API change! I need to dig a little deeper into configfs to recognize its beauty and the possible benefit for our driver. Do you see this as a prerequisite? thanks, Chris ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Drivers: hv: vmbus: Return -EINVAL for the sys files for unopened channels
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
---
drivers/hv/vmbus_drv.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 283d184..d0ff656 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -316,6 +316,8 @@ static ssize_t out_intr_mask_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+ if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
+ return -EINVAL;
hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound, &outbound);
return sprintf(buf, "%d\n", outbound.current_interrupt_mask);
}
@@ -329,6 +331,8 @@ static ssize_t out_read_index_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+ if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
+ return -EINVAL;
hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound, &outbound);
return sprintf(buf, "%d\n", outbound.current_read_index);
}
@@ -343,6 +347,8 @@ static ssize_t out_write_index_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+ if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
+ return -EINVAL;
hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound, &outbound);
return sprintf(buf, "%d\n", outbound.current_write_index);
}
@@ -357,6 +363,8 @@ static ssize_t out_read_bytes_avail_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+ if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
+ return -EINVAL;
hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound, &outbound);
return sprintf(buf, "%d\n", outbound.bytes_avail_toread);
}
@@ -371,6 +379,8 @@ static ssize_t out_write_bytes_avail_show(struct device
*dev,
if (!hv_dev->channel)
return -ENODEV;
+ if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
+ return -EINVAL;
hv_ringbuffer_get_debuginfo(&hv_dev->channel->outbound, &outbound);
return sprintf(buf, "%d\n", outbound.bytes_avail_towrite);
}
@@ -384,6 +394,8 @@ static ssize_t in_intr_mask_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+ if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
+ return -EINVAL;
hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);
return sprintf(buf, "%d\n", inbound.current_interrupt_mask);
}
@@ -397,6 +409,8 @@ static ssize_t in_read_index_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+ if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
+ return -EINVAL;
hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);
return sprintf(buf, "%d\n", inbound.current_read_index);
}
@@ -410,6 +424,8 @@ static ssize_t in_write_index_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+ if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
+ return -EINVAL;
hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);
return sprintf(buf, "%d\n", inbound.current_write_index);
}
@@ -424,6 +440,8 @@ static ssize_t in_read_bytes_avail_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+ if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
+ return -EINVAL;
hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);
return sprintf(buf, "%d\n", inbound.bytes_avail_toread);
}
@@ -438,6 +456,8 @@ static ssize_t in_write_bytes_avail_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+ if (hv_dev->channel->state != CHANNEL_OPENED_STATE)
+ return -EINVAL;
hv_ringbuffer_get_debuginfo(&hv_dev->channel->inbound, &inbound);
return sprintf(buf, "%d\n", inbound.bytes_avail_towrite);
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1] binder: implement binderfs
Hi Christian,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v4.20-rc6 next-20181212]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Christian-Brauner/binder-implement-binderfs/20181210-003538
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
include/linux/slab.h:332:43: warning: dubious: x & !y
include/linux/slab.h:332:43: warning: dubious: x & !y
>> drivers/android/binderfs.c:232:43: warning: incorrect type in initializer
>> (different address spaces)
drivers/android/binderfs.c:232:43:expected struct binderfs_device *device
drivers/android/binderfs.c:232:43:got struct binderfs_device [noderef]
*
>> drivers/android/binderfs.c:238:51: warning: incorrect type in argument 2
>> (different address spaces)
drivers/android/binderfs.c:238:51:expected void const [noderef]
*from
drivers/android/binderfs.c:238:51:got struct binderfs_device *device
drivers/android/binderfs.c:242:60: warning: incorrect type in argument 2
(different address spaces)
drivers/android/binderfs.c:242:60:expected struct binderfs_device
[noderef] *userp
drivers/android/binderfs.c:242:60:got struct binderfs_device *device
include/linux/slab.h:332:43: warning: dubious: x & !y
include/linux/slab.h:332:43: warning: dubious: x & !y
vim +232 drivers/android/binderfs.c
215
216 /**
217 * binderfs_ctl_ioctl - handle binder device node allocation requests
218 *
219 * The request handler for the binder-control device. All requests
operate on
220 * the binderfs mount the binder-control device resides in:
221 * - BINDER_CTL_ADD
222 * Allocate a new binder device.
223 *
224 * Return: 0 on success, negative errno on failure
225 */
226 static long binder_ctl_ioctl(struct file *file, unsigned int cmd,
227 unsigned long arg)
228 {
229 struct binderfs_info *info;
230 int ret = -EINVAL;
231 struct inode *inode = file_inode(file);
> 232 struct binderfs_device *device = (struct binderfs_device __user
> *)arg;
233 struct binderfs_device device_req;
234
235 info = BINDERFS_I(inode);
236 switch (cmd) {
237 case BINDER_CTL_ADD:
> 238 ret = copy_from_user(&device_req, device,
> sizeof(device_req));
239 if (ret)
240 break;
241
242 ret = binderfs_binder_device_create(inode, device,
&device_req);
243 break;
244 default:
245 break;
246 }
247
248 return ret;
249 }
250
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: comedi: 8255: fixed an SPDX License Tag coding style issue
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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: most: Documentation: add information to driver_usage file
On Thu, Dec 13, 2018 at 04:32:25PM +, christian.gr...@microchip.com wrote: > On Do, 2018-12-13 at 13:32 +0100, Greg KH wrote: > > On Thu, Dec 13, 2018 at 02:58:00PM +0300, Dan Carpenter wrote: > > > > > > On Wed, Dec 12, 2018 at 01:15:31PM +0100, Christian Gromm wrote: > > > > > > > > This patch updates driver_usage.txt file to reflect the latest > > > > changes > > > > that this patch set introduces. > > > > > > > > Signed-off-by: Christian Gromm > > > > --- > > > > 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 > > > I would maybe prefer if the "create" command were on a separate > > > line. > > > Something like: > > > > > > +$ 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" > > > >$(DRV_DIR)/add_link > > > +$ echo "mdev0:ep83:sound:create" >$(DRV_DIR)/sound_card > > > > > > It's sort of a separate command in a way? > > Why is sysfs being used for configuring things? Isn't that what > > configfs was created for? :) > > > > Omg, that would be one API change! > > I need to dig a little deeper into configfs to recognize > its beauty and the possible benefit for our driver. > > Do you see this as a prerequisite? Writing random strings to a random sysfs file to configure things is not a "normal" user/kernel api as sysfs files are supposed to just be "one value" and not parsed like what you are doing here. So I would strongly suggest looking at configfs, that is what it was designed for. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1] binder: implement binderfs
On Wed, Dec 12, 2018 at 01:51:27PM +0100, Christian Brauner wrote: > > > Cc: Martijn Coenen > > > Cc: Todd Kjos > > > Cc: Greg Kroah-Hartman > > > Signed-off-by: Christian Brauner > > Do we plan to bring this into mergeable shape before Christmas? I'm > happy to do it. :) I haven't had the chance to give it a good review yet, but you should at least fix up the kbuild warnings that the 0-day bot sent you :) thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: comedi: cb_pcidas: fixed a spelling mistake coding style issue
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(-) diff --git a/drivers/staging/comedi/drivers/cb_pcidas.c b/drivers/staging/comedi/drivers/cb_pcidas.c index 8429d57..02ae00c 100644 --- a/drivers/staging/comedi/drivers/cb_pcidas.c +++ b/drivers/staging/comedi/drivers/cb_pcidas.c @@ -116,7 +116,7 @@ #define PCIDAS_TRIG_SEL_ANALOG PCIDAS_TRIG_SEL(3) /* ext. analog trigger */ #define PCIDAS_TRIG_SEL_MASK PCIDAS_TRIG_SEL(3) /* start trigger mask */ #define PCIDAS_TRIG_POLBIT(2) /* invert trigger (1602 only) */ -#define PCIDAS_TRIG_MODE BIT(3) /* edge/level trigerred (1602 only) */ +#define PCIDAS_TRIG_MODE BIT(3) /* edge/level triggered (1602 only) */ #define PCIDAS_TRIG_EN BIT(4) /* enable external start trigger */ #define PCIDAS_TRIG_BURSTE BIT(5) /* burst mode enable */ #define PCIDAS_TRIG_CLRBIT(7) /* clear external trigger */ -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4] Staging: iio: adt7316: Add all irq related code in adt7316_irq_setup()
ADT7316 driver no more uses platform data and hence use device tree
data instead of platform data for assigning irq_type field and
implement this in adt7316_irq_setup function.
Switch case figures out the type of irq and if it's the default case
then assign the default value to the irq_type i.e.
irq_type = IRQF_TRIGGER_LOW
Move devm_request_threaded_irq() and assignment of chip->config1
into the adt7316_setup_irq() to unclutter the code in probe function.
Signed-off-by: Shreeya Patel
---
Changes in v4
- Merge patches *[1/3 v3], *[2/3 v3] and *[3/3 v3] to make it less
complex to review.
Changes in v3
- Add a new function for having all interrupt related code.
Changes in v2
- Make the commit message of patch *[1/5] appropriate.
drivers/staging/iio/addac/adt7316.c | 52 +
1 file changed, 38 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/iio/addac/adt7316.c
b/drivers/staging/iio/addac/adt7316.c
index 9c72538baf9e..1ca4ee0f30ee 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -1807,6 +1807,43 @@ static irqreturn_t adt7316_event_handler(int irq, void
*private)
return IRQ_HANDLED;
}
+static int adt7316_setup_irq(struct iio_dev *indio_dev)
+{
+ struct adt7316_chip_info *chip = iio_priv(indio_dev);
+ int irq_type, ret;
+
+ irq_type = irqd_get_trigger_type(irq_get_irq_data(chip->bus.irq));
+
+ switch (irq_type) {
+ case IRQF_TRIGGER_HIGH:
+ case IRQF_TRIGGER_RISING:
+ break;
+ case IRQF_TRIGGER_LOW:
+ case IRQF_TRIGGER_FALLING:
+ break;
+ default:
+ dev_info(&indio_dev->dev, "mode %d unsupported, using
IRQF_TRIGGER_LOW\n",
+irq_type);
+ irq_type = IRQF_TRIGGER_LOW;
+ break;
+ }
+
+ ret = devm_request_threaded_irq(&indio_dev->dev, chip->bus.irq,
+ NULL, adt7316_event_handler,
+ irq_type | IRQF_ONESHOT,
+ indio_dev->name, indio_dev);
+ if (ret) {
+ dev_err(&indio_dev->dev, "failed to request irq %d\n",
+ chip->bus.irq);
+ return ret;
+ }
+
+ if (irq_type & IRQF_TRIGGER_HIGH)
+ chip->config1 |= ADT7316_INT_POLARITY;
+
+ return 0;
+}
+
/*
* Show mask of enabled interrupts in Hex.
*/
@@ -2101,8 +2138,6 @@ int adt7316_probe(struct device *dev, struct adt7316_bus
*bus,
{
struct adt7316_chip_info *chip;
struct iio_dev *indio_dev;
- unsigned short *adt7316_platform_data = dev->platform_data;
- int irq_type = IRQF_TRIGGER_LOW;
int ret = 0;
indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
@@ -2146,20 +2181,9 @@ int adt7316_probe(struct device *dev, struct adt7316_bus
*bus,
indio_dev->modes = INDIO_DIRECT_MODE;
if (chip->bus.irq > 0) {
- if (adt7316_platform_data[0])
- irq_type = adt7316_platform_data[0];
-
- ret = devm_request_threaded_irq(dev, chip->bus.irq,
- NULL,
- adt7316_event_handler,
- irq_type | IRQF_ONESHOT,
- indio_dev->name,
- indio_dev);
+ ret = adt7316_setup_irq(indio_dev);
if (ret)
return ret;
-
- if (irq_type & IRQF_TRIGGER_HIGH)
- chip->config1 |= ADT7316_INT_POLARITY;
}
ret = chip->bus.write(chip->bus.client, ADT7316_CONFIG1, chip->config1);
--
2.17.1
___
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, Dec 13, 2018 at 04:35:43PM +, 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
Queued up, thank you.
--
Thanks,
Sasha
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v1] binder: implement binderfs
On Thu, Dec 13, 2018 at 06:56:26PM +0100, Greg Kroah-Hartman wrote: > On Wed, Dec 12, 2018 at 01:51:27PM +0100, Christian Brauner wrote: > > > > Cc: Martijn Coenen > > > > Cc: Todd Kjos > > > > Cc: Greg Kroah-Hartman > > > > Signed-off-by: Christian Brauner > > > > Do we plan to bring this into mergeable shape before Christmas? I'm > > happy to do it. :) > > I haven't had the chance to give it a good review yet, but you should at > least fix up the kbuild warnings that the 0-day bot sent you :) __user is what's confusing kbot here. You want me to wait for your review or send out a v2 right now? :) Thanks! Christian ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] binder: fix use-after-free due to ksys_close() during fdget()
44d8047f1d8 ("binder: use standard functions to allocate fds")
exposed a pre-existing issue in the binder driver.
fdget() is used in ksys_ioctl() as a performance optimization.
One of the rules associated with fdget() is that ksys_close() must
not be called between the fdget() and the fdput(). There is a case
where this requirement is not met in the binder driver which results
in the reference count dropping to 0 when the device is still in
use. This can result in use-after-free or other issues.
If userpace has passed a file-descriptor for the binder driver using
a BINDER_TYPE_FDA object, then kys_close() is called on it when
handling a binder_ioctl(BC_FREE_BUFFER) command. This violates
the assumptions for using fdget().
The problem is fixed by deferring the fd close using task_work_add()
so ksys_close() is called after returning from binder_ioctl().
Fixes: 44d8047f1d87a ("binder: use standard functions to allocate fds")
Suggested-by: Al Viro
Signed-off-by: Todd Kjos
---
drivers/android/binder.c | 91 +++-
1 file changed, 81 insertions(+), 10 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index c525b164d39d2..8f77d6af31209 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -72,6 +72,7 @@
#include
#include
#include
+#include
#include
@@ -560,6 +561,9 @@ enum {
*(protected by @proc->inner_lock)
* @waiting_thread_node: element for @proc->waiting_threads list
*(protected by @proc->inner_lock)
+ * @deferred_close_fd_cb: callback_head for task work
+ * @deferred_close_fds: list of fds to be closed
+ *(only accessed by this thread)
* @pid: PID for this thread
*(invariant after initialization)
* @looper: bitmap of looping state
@@ -592,6 +596,8 @@ struct binder_thread {
struct binder_proc *proc;
struct rb_node rb_node;
struct list_head waiting_thread_node;
+ struct callback_head deferred_close_fd_cb;
+ struct hlist_head deferred_close_fds;
int pid;
int looper; /* only modified by this thread */
bool looper_need_return; /* can be written by other thread */
@@ -2184,7 +2190,64 @@ static bool binder_validate_fixup(struct binder_buffer
*b,
return (fixup_offset >= last_min_offset);
}
+struct binder_task_work {
+ struct hlist_node entry;
+ int fd;
+};
+
+/**
+ * binder_do_fd_close() - close list of file descriptors
+ * @twork: callback head for task work
+ *
+ * It is not safe to call ksys_close() during the binder_ioctl()
+ * function if there is a chance that binder's own file descriptor
+ * might be closed. This is to meet the requirements for using
+ * fdget() (see comments for __fget_light()). Therefore use
+ * task_add_work() to schedule the close operation once we have
+ * returned from binder_ioctl(). This function is a callback
+ * for that mechanism and does the actual ksys_close() on the list
+ * of file descriptors.
+ */
+static void binder_do_fd_close(struct callback_head *twork)
+{
+ struct binder_thread *thread = container_of(twork,
+ struct binder_thread, deferred_close_fd_cb);
+ struct hlist_node *entry, *tmp;
+
+ hlist_for_each_safe(entry, tmp, &thread->deferred_close_fds) {
+ struct binder_task_work *work = container_of(entry,
+ struct binder_task_work, entry);
+
+ ksys_close(work->fd);
+ hlist_del(&work->entry);
+ kfree(work);
+ }
+}
+
+/**
+ * binder_deferred_fd_close() - schedule a close for the given file-descriptor
+ * @thread:struct binder_thread for this task
+ * @fd:file-descriptor to close
+ *
+ * See comments in binder_do_fd_close(). This function is used to schedule
+ * a file-descriptor to be closed after returning from binder_ioctl().
+ */
+static void binder_deferred_fd_close(struct binder_thread *thread, int fd)
+{
+ struct binder_task_work *work;
+
+ work = kzalloc(sizeof(*work), GFP_KERNEL);
+ if (!work)
+ return;
+ work->fd = fd;
+
+ if (hlist_empty(&thread->deferred_close_fds))
+ task_work_add(current, &thread->deferred_close_fd_cb, true);
+ hlist_add_head(&work->entry, &thread->deferred_close_fds);
+}
+
static void binder_transaction_buffer_release(struct binder_proc *proc,
+ struct binder_thread *thread,
struct binder_buffer *buffer,
binder_size_t *failed_at)
{
@@ -2323,7 +2386,8 @@ static void binder_transaction_buffer_release(struct
binder_proc *proc,
}
fd_array = (u32 *)(parent_buffer +
(uintptr_t)fda->parent_offset);
Re: [PATCH v1] binder: implement binderfs
On Thu, Dec 13, 2018 at 09:52:03PM +0100, Christian Brauner wrote: > On Thu, Dec 13, 2018 at 06:56:26PM +0100, Greg Kroah-Hartman wrote: > > On Wed, Dec 12, 2018 at 01:51:27PM +0100, Christian Brauner wrote: > > > > > Cc: Martijn Coenen > > > > > Cc: Todd Kjos > > > > > Cc: Greg Kroah-Hartman > > > > > Signed-off-by: Christian Brauner > > > > > > Do we plan to bring this into mergeable shape before Christmas? I'm > > > happy to do it. :) > > > > I haven't had the chance to give it a good review yet, but you should at > > least fix up the kbuild warnings that the 0-day bot sent you :) > > __user is what's confusing kbot here. You want me to wait for your > review or send out a v2 right now? :) A v2 is good, I'm not going to get to my review today... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: most: sound: create one sound card w/ multiple PCM devices per MOST device
On Thu, Dec 13, 2018 at 02:15:59PM +, christian.gr...@microchip.com wrote: > Sounds reasonable. But, problem here is that we want the > process of how channels are linked to be independent from the > component that is being used. That's when things start to > become complicated. I'm not going NAK the patchset because of the API, but I do think we need to rethink it before we move this out of staging. Greg has suggested configfs, but you might also want to consult with the linux-media or ALSA devs because there might be some subsystem specific patterns to consider. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] binder: implement binderfs
As discussed at Linux Plumbers Conference 2018 in Vancouver [1] this is the
implementation of binderfs.
/* Abstract */
binderfs is a backwards-compatible filesystem for Android's binder ipc
mechanism. Each ipc namespace will mount a new binderfs instance. Mounting
binderfs multiple times at different locations in the same ipc namespace
will not cause a new super block to be allocated and hence it will be the
same filesystem instance.
Each new binderfs mount will have its own set of binder devices only
visible in the ipc namespace it has been mounted in. All devices in a new
binderfs mount will follow the scheme binder%d and numbering will always
start at 0.
/* Backwards compatibility */
Devices requested in the Kconfig via CONFIG_ANDROID_BINDER_DEVICES for the
initial ipc namespace will work as before. They will be registered via
misc_register() and appear in the devtmpfs mount. Specifically, the
standard devices binder, hwbinder, and vndbinder will all appear in their
standard locations in /dev. Mounting or unmounting the binderfs mount in
the initial ipc namespace will have no effect on these devices, i.e. they
will neither show up in the binderfs mount nor will they disappear when the
binderfs mount is gone.
/* binder-control */
Each new binderfs instance comes with a binder-control device. No other
devices will be present at first. The binder-control device can be used to
dynamically allocate binder devices. All requests operate on the binderfs
mount the binder-control device resides in.
Assuming a new instance of binderfs has been mounted at /dev/binderfs
via mount -t binderfs binderfs /dev/binderfs. Then a request to create a
new binder device can be made as illustrated in [2].
Binderfs devices can simply be removed via unlink().
/* Implementation details */
- dynamic major number allocation:
When binderfs is registered as a new filesystem it will dynamically
allocate a new major number. The allocated major number will be returned
in struct binderfs_device when a new binder device is allocated.
- global minor number tracking:
Minor are tracked in a global idr struct that is capped at
BINDERFS_MAX_MINOR. The minor number tracker is protected by a global
mutex. This is the only point of contention between binderfs mounts.
- struct binderfs_info:
Each binderfs super block has its own struct binderfs_info that tracks
specific details about a binderfs instance:
- ipc namespace
- dentry of the binder-control device
- root uid and root gid of the user namespace the binderfs instance
was mounted in
- mountable by user namespace root:
binderfs can be mounted by user namespace root in a non-initial user
namespace. The devices will be owned by user namespace root.
- binderfs binder devices without misc infrastructure:
New binder devices associated with a binderfs mount do not use the
full misc_register() infrastructure.
The misc_register() infrastructure can only create new devices in the
host's devtmpfs mount. binderfs does however only make devices appear
under its own mountpoint and thus allocates new character device nodes
from the inode of the root dentry of the super block. This will have
the side-effect that binderfs specific device nodes do not appear in
sysfs. This behavior is similar to devpts allocated pts devices and
has no effect on the functionality of the ipc mechanism itself.
[1]: https://goo.gl/JL2tfX
[2]: program to allocate a new binderfs binder device:
#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
int main(int argc, char *argv[])
{
int fd, ret, saved_errno;
size_t len;
struct binderfs_device device = { 0 };
if (argc < 2)
exit(EXIT_FAILURE);
len = strlen(argv[1]);
if (len > BINDERFS_MAX_NAME)
exit(EXIT_FAILURE);
memcpy(device.name, argv[1], len);
fd = open("/dev/binderfs/binder-control", O_RDONLY | O_CLOEXEC);
if (fd < 0) {
printf("%s - Failed to open binder-control device\n",
strerror(errno));
exit(EXIT_FAILURE);
}
ret = ioctl(fd, BINDER_CTL_ADD, &device);
saved_errno = errno;
close(fd);
errno = saved_errno;
if (ret < 0) {
printf("%s - Failed to allocate new binder device\n",
strerror(errno));
exit(EXIT_FAILURE);
}
printf("Allocated new binder device with major %d, minor %d, and "
"name %s\n", device.major, device.minor,
device.name);
exit(EXIT_SUCCESS);
}
Cc: Martijn Coenen
Cc: Greg Kroah-Hartman
Signed-off-by: Christian Brauner
Acked-by
Re: [PATCH 04/11] staging: iio: adt7316: fix handling of dac high resolution option
On Wed, Dec 12, 2018 at 11:23:16AM +0300, Dan Carpenter wrote: > On Tue, Dec 11, 2018 at 05:54:56PM -0700, Jeremy Fertic wrote: > > @@ -651,10 +649,12 @@ static ssize_t > > adt7316_store_da_high_resolution(struct device *dev, > > u8 config3; > > int ret; > > > > + if (chip->id == ID_ADT7318 || chip->id == ID_ADT7519) > > + return -EPERM; > > return -EINVAL is more appropriate than -EPERM. > > regards, > dan carpenter > I chose -EPERM because the driver uses it quite a few times in similar circumstances. At least with this driver, -EINVAL is used when the user attempts to write data that would never be valid. -EPERM is used when either the current device settings prevent some functionality from being used, or the device never supports that functionality. This patch is the latter, that these two chip ids never support this function. I'll change to -EINVAL in a v2 series, but I wonder if I should hold off on a separate patch for other instances in this driver since it will be undergoing a substantial refactoring. Is there any rule of thumb about when -EPERM is appropriate for a driver, or is -EINVAL almost always the better option? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/11] staging: iio: adt7316: invert the logic of the check for an ldac pin
On Wed, Dec 12, 2018 at 11:19:49AM +0300, Dan Carpenter wrote:
> On Tue, Dec 11, 2018 at 05:54:54PM -0700, Jeremy Fertic wrote:
> > ADT7316_DA_EN_VIA_DAC_LDCA is set when the dac and ldac registers are being
> > used to update the dacs instead of the ldac pin. ADT7516_SEL_AIN3 is an adc
> > input that shares the ldac pin. Only set these bits if an ldac pin is not
> > being used.
> >
> > Signed-off-by: Jeremy Fertic
>
> Huh... This bug has always been there...
>
> Fixes: 35f6b6b86ede ("staging: iio: new ADT7316/7/8 and ADT7516/7/9 driver")
>
> regards,
> dan carpenter
>
Should I include this Fixes tag in v2? I wasn't sure how important this was
in staging. I think most of the patches in this series fix bugs that date
back to the introduction of the driver.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: iio: adc: ad7192: Add clock for external clock reference
Quoting Jonathan Cameron (2018-12-08 07:29:54) > On Thu, 6 Dec 2018 11:10:51 +0200 > Mircea Caprioru wrote: > > > This patch adds a clock to the state structure of ad7192 for getting the > > external clock frequency. This modifications is in accordance with clock > > framework dt bindings documentation. > > > > Signed-off-by: Mircea Caprioru > > +cc Rob and the clk list for advise on how to do the binding for this one. > > It is basically 2 pins, you can put a clock in on one of them or connect > a crystal across them. The driver has to set a register to say which is > the case. > > Current proposal is two optional clocks (fall back to internal oscillator) > but that doesn't seem to be commonly done, so I'm wondering if there > is a 'standard' way to handle this sort of thing. > I'm not sure I fully understand, but I think perhaps assigned-clock-parents would work? Or does that not work because either way some parent is assigned, either the crystal or the optional clk that isn't a crystal? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Hello...
Hi, How are you? I must confess that you're a nice looking gentle man on your profile.Are you married?, Can we be friends? Susan. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/11] staging: iio: adt7316: invert the logic of the check for an ldac pin
On Thu, Dec 13, 2018 at 03:06:29PM -0700, Jeremy Fertic wrote:
> On Wed, Dec 12, 2018 at 11:19:49AM +0300, Dan Carpenter wrote:
> > On Tue, Dec 11, 2018 at 05:54:54PM -0700, Jeremy Fertic wrote:
> > > ADT7316_DA_EN_VIA_DAC_LDCA is set when the dac and ldac registers are
> > > being
> > > used to update the dacs instead of the ldac pin. ADT7516_SEL_AIN3 is an
> > > adc
> > > input that shares the ldac pin. Only set these bits if an ldac pin is not
> > > being used.
> > >
> > > Signed-off-by: Jeremy Fertic
> >
> > Huh... This bug has always been there...
> >
> > Fixes: 35f6b6b86ede ("staging: iio: new ADT7316/7/8 and ADT7516/7/9 driver")
> >
> > regards,
> > dan carpenter
> >
>
> Should I include this Fixes tag in v2? I wasn't sure how important this was
> in staging. I think most of the patches in this series fix bugs that date
> back to the introduction of the driver.
I was just curious to see if it was a cleanup which introduced the
inverted if statement.
I think the Fixes tag is always useful. For example, it would be
interesting to do some data mining to see how many bugs drivers
normally have when they're first merged.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/11] staging: iio: adt7316: fix handling of dac high resolution option
On Thu, Dec 13, 2018 at 03:01:46PM -0700, Jeremy Fertic wrote: > On Wed, Dec 12, 2018 at 11:23:16AM +0300, Dan Carpenter wrote: > > On Tue, Dec 11, 2018 at 05:54:56PM -0700, Jeremy Fertic wrote: > > > @@ -651,10 +649,12 @@ static ssize_t > > > adt7316_store_da_high_resolution(struct device *dev, > > > u8 config3; > > > int ret; > > > > > > + if (chip->id == ID_ADT7318 || chip->id == ID_ADT7519) > > > + return -EPERM; > > > > return -EINVAL is more appropriate than -EPERM. > > > > regards, > > dan carpenter > > > > I chose -EPERM because the driver uses it quite a few times in similar > circumstances. Yeah. I saw that when I reviewed the later patches in this series. It's really not doing it right. -EPERM means permission checks like access_ok() failed so it's not appropriate. -EINVAL is sort of general purpose for invalid commands so it's probably the correct thing. > At least with this driver, -EINVAL is used when the user > attempts to write data that would never be valid. -EPERM is used when > either the current device settings prevent some functionality from being > used, or the device never supports that functionality. This patch is the > latter, that these two chip ids never support this function. > > I'll change to -EINVAL in a v2 series, but I wonder if I should hold off > on a separate patch for other instances in this driver since it will be > undergoing a substantial refactoring. Generally, you should prefer kernel standards over driver standards and especially for staging. But it doesn't matter. When I reviewed this patch, I hadn't seen that the driver was doing it like this but now I know so it's fine. We can clean it all at once later if you want. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] binder: implement binderfs
On Thu, Dec 13, 2018 at 10:59:11PM +0100, Christian Brauner wrote:
> +/**
> + * binderfs_new_inode - allocate inode from super block of a binderfs mount
> + * @ref_inode: inode from wich the super block will be taken
> + * @userp: buffer to copy information about new device for userspace to
> + * @device:binder device for which the new inode will be allocated
> + * @req: struct binderfs_device as copied from userspace
> + *
> + * This function will allocate a new inode from the super block of the
> + * filesystem mount and attach a dentry to that inode.
> + * Minor numbers are limited and tracked globally in binderfs_minors.
> + * The function will stash a struct binder_device for the specific binder
> + * device in i_private of the inode.
> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +static int binderfs_new_inode(struct inode *ref_inode,
> + struct binder_device *device,
> + struct binderfs_device __user *userp,
> + struct binderfs_device *req)
> +{
> + int minor, ret;
> + struct dentry *dentry, *dup, *root;
> + size_t name_len = BINDERFS_MAX_NAME + 1;
> + char *name = NULL;
> + struct inode *inode = NULL;
> + struct super_block *sb = ref_inode->i_sb;
> + struct binderfs_info *info = sb->s_fs_info;
> +
> + /* Reserve new minor number for the new device. */
> + mutex_lock(&binderfs_minors_mutex);
> + minor = ida_alloc_max(&binderfs_minors, BINDERFS_MAX_MINOR, GFP_KERNEL);
> + mutex_unlock(&binderfs_minors_mutex);
> + if (minor < 0)
> + return minor;
> +
> + ret = -ENOMEM;
> + inode = new_inode(sb);
> + if (!inode)
> + goto err;
> +
> + inode->i_ino = minor + INODE_OFFSET;
> + inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
> + init_special_inode(inode, S_IFCHR | 0600,
> +MKDEV(MAJOR(binderfs_dev), minor));
> + inode->i_fop = &binder_fops;
> + inode->i_uid = info->root_uid;
> + inode->i_gid = info->root_gid;
> + inode->i_private = device;
> +
> + name = kmalloc(name_len, GFP_KERNEL);
> + if (!name)
> + goto err;
> +
> + ret = snprintf(name, name_len, "%s", req->name);
> + if (ret < 0 || (size_t)ret >= name_len) {
kernel snprintf() doesn't return negatives and the cast isn't required
either.
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + device->binderfs_inode = inode;
> + device->context.binder_context_mgr_uid = INVALID_UID;
> + device->context.name = name;
> + device->miscdev.name = name;
> + device->miscdev.minor = minor;
> + mutex_init(&device->context.context_mgr_node_lock);
> +
> + req->major = MAJOR(binderfs_dev);
> + req->minor = minor;
> +
> + ret = copy_to_user(userp, req, sizeof(*req));
> + if (ret)
> + goto err;
copy_to_user() returns the number of bytes remaining.
ret = -EFAULT;
if (copy_to_user(userp, req, sizeof(*req)))
goto err;
Also if this copy_to_user() fails, then does the kfree(name) and the
iput(inode) lead to a double free of name in binderfs_evict_inode()?
> +
> + root = sb->s_root;
> + inode_lock(d_inode(root));
> + dentry = d_alloc_name(root, name);
> + if (!dentry) {
> + inode_unlock(d_inode(root));
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + /* Verify that the name userspace gave us is not already in use. */
> + dup = d_lookup(root, &dentry->d_name);
> + if (dup) {
> + if (d_really_is_positive(dup)) {
> + dput(dup);
> + dput(dentry);
> + inode_unlock(d_inode(root));
> + /*
> + * Prevent double free since iput() calls
> + * binderfs_evict_inode().
> + */
> + inode->i_private = NULL;
> + ret = -EEXIST;
> + goto err;
> + }
> + dput(dup);
> + }
> +
> + d_add(dentry, inode);
> + fsnotify_create(root->d_inode, dentry);
> + inode_unlock(d_inode(root));
> +
> + return 0;
> +
> +err:
> + kfree(name);
> + mutex_lock(&binderfs_minors_mutex);
> + ida_free(&binderfs_minors, minor);
> + mutex_unlock(&binderfs_minors_mutex);
> + iput(inode);
> +
> + return ret;
> +}
> +
> +static int binderfs_binder_device_create(struct inode *inode,
> + struct binderfs_device __user *userp,
> + struct binderfs_device *req)
> +{
> + struct binder_device *device;
> + int ret;
> +
> + device = kzalloc(sizeof(*device), GFP_KERNEL);
> + if (!device)
> + return -ENOMEM;
Just move this allocation into binderfs_new_inode() and get rid of this
function.
> +
