Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
Hi Jonathan, I have attached a CSV version of this table below. Hopefully that will make things easier for you. RegisterIIO Attribute Device Tree or Code Direction type Index ModifierInfo Mask AIGAIN in_current0_phaseA_scalein current 0 phaseA scale AVGAIN in_voltage0_phaseA_scalein voltage 0 phaseA scale BIGAIN in_current0_phaseB_scalein current 0 phaseB scale BVGAIN in_voltage0_phaseB_scalein voltage 0 phaseB scale CIGAIN in_current0_phaseC_scalein current 0 phaseC scale CVGAIN in_voltage0_phaseC_scalein voltage 0 phaseC scale NIGAIN in_current0_neutral_scale in current 0 neutral scale AIRMSOS in_current0_phaseA_rms_offset in current 0 phaseA offset AVRMSOS in_voltage0_phaseA_rms_offset in voltage 0 phaseA offset BIRMSOS in_current0_phaseB_rms_offset in current 0 phaseB offset BVRMSOS in_voltage0_phaseB_rms_offset in voltage 0 phaseB offset CIRMSOS in_current0_phaseC_rms_offset in current 0 phaseC offset CVRMSOS in_voltage0_phaseC_rms_offset in voltage 0 phaseC offset NIRMSOS in_current0_neutral_rms_offset in current 0 neutral offset AVAGAIN in_powerapparent0_phaseA_scale in powerapparent 0 phaseA scale BVAGAIN in_powerapparent0_phaseB_scale in powerapparent 0 phaseB scale CVAGAIN in_powerapparent0_phaseC_scale in powerapparent 0 phaseC scale AWGAIN in_power0_phaseA_scale in power 0 phaseA scale AWATTOS in_power0_phaseA_offset in power 0 phaseA offset BWGAIN in_power0_phaseB_scale in power 0 phaseB scale BWATTOS in_power0_phaseB_offset in power 0 phaseB offset CWGAIN in_power0_PhaseC_scale in power 0 phaseC scale CWATTOS in_power0_phaseC_offset in power 0 phaseC offset AVARGAINin_powerreactive0_phaseA_scale in powerreactive 0 phaseA scale AVAROS in_powerreactive0_phaseA_offset in powerreactive 0 phaseA offset BVARGAINin_powerreactive0_phaseB_scale in powerreactive 0 phaseB scale BVAROS in_powerreactive0_phaseB_offset in powerreactive 0 phaseB offset CVARGAINin_powerreactive0_phaseC_scale in powerreactive 0 phaseC scale CVAROS in_powerreactive0_phaseC_offset in powerreactive 0 phaseC offset AFWGAIN in_power0_phaseA_fundamental_scale in power 0 phaseA_fundamental scale AFWATTOSin_power0_phaseA_fundamental_offset in power 0 phaseA_fundamental offset BFWGAIN in_power0_phaseB_fundamental_scale in power 0 phaseB_fundamental scale BFWATTOSin_power0_phaseB_fundamental_offset in power 0 phaseB_fundamental offset CFWGAIN in_power0_phaseC_fundamental_scale in power 0 phaseC_fundamental scale CFWATTOSin_power0_phaseC_fundamental_offset in power 0 phaseC_fundamental offset AFVARGAIN in_powerreactive0_phaseA_fundamental_scale in powerreactive 0 phaseA_fundamental scale AFVAROS in_powerreactive0_phaseA_fundamental_offset in powerreactive 0 phaseA_fundamental offset BFVARGAIN in_powerreactive0_phaseB_fundamental_scale in powerreactive 0 phaseB_fundamental scale BFVAROS in_powerreactive0_phaseB_fundamental_offset in powerreactive 0 phaseB_fundamental offset CFVARGAIN in_powerreactive0_phaseC_fundamental_scale in powerreactive 0 phaseC_fundamental scale CFVAROS in_powerreactive0_phaseC_fundamental_offset in powerreactive 0 phaseC_fundamental offset VATHR1 VATHR1 DT VATHR0 VATHR0 DT WTHR1 WTHR1 DT WTHR0 WTHR0 DT VARTHR1 VARTHR1 DT VARTHR0 VARTHR0 DT ReservedReserved VANOLOADVANOLOADDT APNOLOADAPNOLOADDT VLEVEL VLEVEL DT DICOEFF DICOEFF DT HPFDIS HPFDIS DT
Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
> On Mar 24, 2018, at 8:18 AM, Jonathan Cameron wrote:
>
> On Mon, 19 Mar 2018 23:28:45 -0700
> John Syne wrote:
>
>> Hi Jonathan,
>>
>> I broke out the {Direction}_{Type}_{Index}_{Modifier}_{Info_Mask} into
>> separate columns to make sure I understand your instructions. Good way to
>> check the results.
>>
>> Probably easier to copy and paste this table into a spreadsheet. Let me know
>> if there is anything I got wrong. Thank you again for all your help.
> Yeah, we need to shrink this if we do it again.
I’ll send an updated copy after this e-mail. Can you accept a spreadsheet
attachment or a CSV file?
>
> Can drop anything not related to ABI (so RW mask address etc and just have the
> register name and the bits around ABI.
Done
>
> Note I mentioned the altvoltage stuff in the previous email, so we need
> to think about whether that is useful to distinguish the instantaneous
> measurements from the multi cycle AC ones.
> Actually I think we are fine here as we explicitly describe all
> alt measurements as mav or rms which going to be handled in our
> new computedvalue description.
Yeah, I don’t see how anyone would use these registers, so I propose
we just drop them.
>
>
>>
>> Address RegisterIIO Attribute Device Tree or Code
>> Direction TypeIndex ModifierInfo Mask R/W Bit
>> Length Bit Length During CommunicationsTypeDefault Value
>> Description
>> 0x4380 AIGAIN in_current0_phaseA_scalein current
>> 0 phaseA scale R/W 24 32 ZPSE S 0x00
>> Phase A current gain adjust.
>> 0x4381 AVGAIN in_voltage0_phaseA_scalein voltage
>> 0 phaseA scale R/W 24 32 ZPSE S 0x00
>> Phase A voltage gain adjust.
>> 0x4382 BIGAIN in_current0_phaseB_scalein current
>> 0 phaseB scale R/W 24 32 ZPSE S 0x00
>> Phase B current gain adjust.
>> 0x4383 BVGAIN in_voltage0_phaseB_scalein voltage
>> 0 phaseB scale R/W 24 32 ZPSE S 0x00
>> Phase B voltage gain adjust.
>> 0x4384 CIGAIN in_current0_phaseC_scalein current
>> 0 phaseC scale R/W 24 32 ZPSE S 0x00
>> Phase C current gain adjust.
>> 0x4385 CVGAIN in_voltage0_phaseC_scalein voltage
>> 0 phaseC scale R/W 24 32 ZPSE S 0x00
>> Phase C voltage gain adjust.
>> 0x4386 NIGAIN in_current0_neutral_scale in current
>> 0 neutral scale R/W 24 32 ZPSE S 0x00
>> Neutral current gain adjust (ADE7868 and ADE7878 only).
>> 0x4387 AIRMSOS in_current0_phaseA_rms_offset in current
>> 0 phaseA offset R/W 24 32 ZPSE S 0x00
>> Phase A current rms offset.
>> 0x4388 AVRMSOS in_voltage0_phaseA_rms_offset in voltage
>> 0 phaseA offset R/W 24 32 ZPSE S 0x00
>> Phase A voltage rms offset.
>> 0x4389 BIRMSOS in_current0_phaseB_rms_offset in current
>> 0 phaseB offset R/W 24 32 ZPSE S 0x00
>> Phase B current rms offset.
>> 0x438A BVRMSOS in_voltage0_phaseB_rms_offset in voltage
>> 0 phaseB offset R/W 24 32 ZPSE S 0x00
>> Phase B voltage rms offset.
>> 0x438B CIRMSOS in_current0_phaseC_rms_offset in current
>> 0 phaseC offset R/W 24 32 ZPSE S 0x00
>> Phase C current rms offset.
>> 0x438C CVRMSOS in_voltage0_phaseC_rms_offset in voltage
>> 0 phaseC offset R/W 24 32 ZPSE S 0x00
>> Phase C voltage rms offset.
>> 0x438D NIRMSOS in_current0_neutral_rms_offset in current
>> 0 neutral offset R/W 24 32 ZPSE S 0x00
>> Neutral current rms offset (ADE7868 and ADE7878 only).
>> 0x438E AVAGAIN in_powerapparent0_phaseA_scale in
>> powerapparent 0 phaseA scale R/W 24 32 ZPSE S
>> 0x00Phase A apparent power gain adjust.
>> 0x438F BVAGAIN in_powerapparent0_phaseB_scale in
>> powerapparent 0 phaseB scale R/W 24 32 ZPSE S
>> 0x00Phase B apparent power gain adjust.
>> 0x4390 CVAGAIN in_powerapparent0_phaseC_scale in
>> powerapparent 0 phaseC scale R/W 24 32 ZPSE S
>> 0x00Phase C apparent power gain adjust.
>> 0x4391 AWGAIN in_power0_phaseA_scale in power 0
>> phaseA scale R/W 24 32 ZPSE S 0x00Phase A
>> total active power gain adjust.
>> 0x4392 AWATTOS in_power0_phaseA_
Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
Hi Jonathan, I was speaking with Rodrigo and here is what I think must be done to move ADE7878 out of staging Here are the steps as I see them: 1) Define the IIO Attributes so they are consistent with the IIO ABI. This should be pretty simple given agreement on the naming convention. 2) Map the ADE7854 interrupt status to IIO events. This requires an interrupt processing section. 3) Add DeviceTree support. 4) Create DeviceTree overlay for the ADE7854. 5) Update ADE7854 probe to read in the DeviceTree register settings. 6) Add support for power modes (PM1, PM2). 7) Not sure if we will support measurement streaming on the ADE7854. The problem is ADE7854 is designed as an SPI master, which means it controls the SPI clock, so the driver must support SPI slave mode. However, the Linux Kernel does not currently support SPI slave mode. We have three choices to make this work and they are all a lot of work: 1) Add support for SPI Slave mode to the kernel, 2) Use hardware to convert SPI signals to I2S signals and with the use of a custom codec, use the ALSA framework to stream the samples (this is an approach I used, but I don’t like it), 3) Move the I2S driver out of the sound subsystem and use it together with DMA to stream samples directly into the ADE7854 driver (my preferred solutions). Perhaps Mark Brown has some ideas on how to make this work. The ADE9000 will be much easier because it uses an SPI Slave interface. I hope I have captured everything, but let me know if I have missed anything. Regards, John ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: bcm2835-audio: Release resources on module_exit()
On Fri, Mar 23, 2018 at 9:22 PM, Kirill Marinushkin wrote: > On 03/23/18 17:23, Andy Shevchenko wrote: >> After addressing above, FWIW, >> >> Reviewed-by: Andy Shevchenko Seems you missed my tag in new version. When someone gives you a tag and you are going to send a new version (w/o drastic changes), it's your responsibility to append it I will send a new mail with it, so, this time no need to resend. Just keep it for the future contributions. -- With Best Regards, Andy Shevchenko ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5] staging: bcm2835-audio: Release resources on module_exit()
On Fri, Mar 23, 2018 at 9:32 PM, Kirill Marinushkin wrote: > In the current implementation, `rmmod snd_bcm2835` does not release > resources properly. It causes an oops when trying to list sound devices. > > This commit fixes it. > > The details WRT allocation / free are described below. > > Device structure WRT allocation: > > pdev > \childdev[] > \card > \chip > \pcm > \ctl > > Allocation / register sequence: > > * childdev: devm_kzalloc - freed during driver detach > * childdev: device_initialize - freed during device_unregister > * pdev: devres_alloc - freed during driver detach > * childdev: device_add- removed during device_unregister > * pdev, childdev: devres_add - freed during driver detach > * card: snd_card_new - freed during snd_card_free > * chip: kzalloc - freed during kfree > * card, chip: snd_device_new - freed during snd_device_free > * chip: new_pcm - TODO: free pcm > * chip: new_ctl - TODO: free ctl > * card: snd_card_register - unregistered during snd_card_free > > Free / unregister sequence: > > * card: snd_card_free > * card, chip: snd_device_free > * childdev: device_unregister > * chip: kfree > > Steps to reproduce the issue before this commit: > > > $ rmmod snd_bcm2835 > $ aplay -L > [ 138.648130] Unable to handle kernel paging request at virtual address > 7f1343c0 > [ 138.660415] pgd = ad8f > [ 138.665567] [7f1343c0] *pgd=3864c811, *pte=, *ppte= > [ 138.674887] Internal error: Oops: 7 [#1] SMP ARM > [ 138.683571] Modules linked in: sha256_generic cfg80211 rfkill snd_pcm > snd_timer > snd fixed uio_pdrv_genirq uio ip_tables x_tables ipv6 [last unloaded: > snd_bcm2835 > ] > [ 138.706594] CPU: 3 PID: 463 Comm: aplay Tainted: GWC > 4.15.0-rc1-v > 7+ #6 > [ 138.719833] Hardware name: BCM2835 > [ 138.726016] task: b877ac00 task.stack: aebec000 > [ 138.733408] PC is at try_module_get+0x38/0x24c > [ 138.740813] LR is at snd_ctl_open+0x58/0x194 [snd] > [ 138.748485] pc : [<801c4d5c>]lr : [<7f0e6b2c>]psr: 2013 > [ 138.757709] sp : aebedd60 ip : aebedd88 fp : aebedd84 > [ 138.765884] r10: r9 : 0004 r8 : 7f0ed440 > [ 138.774040] r7 : b7e469b0 r6 : 7f0e6b2c r5 : afd91900 r4 : 7f1343c0 > [ 138.783571] r3 : aebec000 r2 : 0001 r1 : b877ac00 r0 : 7f1343c0 > [ 138.793084] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment > user > [ 138.803300] Control: 10c5387d Table: 2d8f006a DAC: 0055 > [ 138.812064] Process aplay (pid: 463, stack limit = 0xaebec210) > [ 138.820868] Stack: (0xaebedd60 to 0xaebee000) > [ 138.828207] dd60: b848d000 afd91900 b7e469b0 7f0ed440 > aebedda4 aebedd88 > [ 138.842371] dd80: 7f0e6b2c 801c4d30 afd91900 7f0ea4dc b7e469b0 > aebeddcc aebedda8 > [ 138.856611] dda0: 7f0e250c 7f0e6ae0 7f0e2464 b8478ec0 b7e469b0 afd91900 > 7f0ea388 > [ 138.870864] ddc0: aebeddf4 aebeddd0 802ce590 7f0e2470 8090ab64 afd91900 > afd91900 b7e469b0 > [ 138.885301] dde0: afd91908 802ce4e4 aebede1c aebeddf8 802c57b4 802ce4f0 > afd91900 aebedea8 > [ 138.900110] de00: b7fa4c00 0004 aebede3c aebede20 > 802c6ba8 802c56b4 > [ 138.915260] de20: aebedea8 aebedf5c aebedea4 aebede40 > 802d9a68 802c6b58 > [ 138.930661] de40: b874ddd0 0001 0041 > afd91900 aebede70 > [ 138.946402] de60: 0002 b7e469b0 b8a87610 b8d6ab80 > 801852f8 0008 > [ 138.962314] de80: aebedf5c aebedea8 0001 80108464 aebec000 > aebedf4c aebedea8 > [ 138.978414] dea0: 802dacd4 802d970c b8a87610 b8d6ab80 a7982bc6 0009 > af363019 b9231480 > [ 138.994617] dec0: b8c038a0 b7e469b0 0101 0002 0238 > > [ 139.010823] dee0: aebedee8 0008 000f aebedf3c aebedf00 > 802ed7e4 80843f94 > [ 139.027025] df00: 0003 0008 b9231490 b9231480 0008 > af363000 > [ 139.043229] df20: 0005 0002 ff9c 0008 ff9c > af363000 0003 > [ 139.059430] df40: aebedf94 aebedf50 802c6f70 802dac70 aebec000 > 0001 > [ 139.075629] df60: 0002 0004 0100 0001 7ebe577c 0002e038 > 0005 > [ 139.091828] df80: 80108464 aebec000 aebedfa4 aebedf98 802c7060 802c6e6c > aebedfa8 > [ 139.108025] dfa0: 801082c0 802c7040 7ebe577c 0002e038 7ebe577c 0008 > 0b98 e81c8400 > [ 139.124222] dfc0: 7ebe577c 0002e038 0005 7ebe57e4 00a20af8 > 7ebe57f0 76f87394 > [ 139.140419] dfe0: 7ebe55c4 76ec88e8 76df1d9c 6010 7ebe577c > > [ 139.156715] [<801c4d5c>] (try_module_get) from [<7f0e6b2c>] > (snd_ctl_open+0x58/0x194 [snd]) > [ 139.173222] [<7f0e6b2c>] (snd_ctl_open [snd]) from [<7f0e250c>] > (snd_open+0xa8/0x14c [snd]) > [ 139.189683] [<7f0e250c>] (snd_open [snd]) from [<802ce5
Re: [PATCH v4] staging: bcm2835-audio: Release resources on module_exit()
On 03/25/18 12:33, Andy Shevchenko wrote: > On Fri, Mar 23, 2018 at 9:22 PM, Kirill Marinushkin > wrote: >> On 03/23/18 17:23, Andy Shevchenko wrote: >>> After addressing above, FWIW, >>> >>> Reviewed-by: Andy Shevchenko > Seems you missed my tag in new version. > When someone gives you a tag and you are going to send a new version > (w/o drastic changes), it's your responsibility to append it > > I will send a new mail with it, so, this time no need to resend. Just > keep it for the future contributions. > What is a "tag"? I added in-reply-to this email, I added you as CC. Isn't it enough to keep track of the versioning within a mailing list? Could you please clarify: what "tag" should I usually attach in addition? Best Regards, Kirill ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH][next][V2] staging: r8822be: fix typos in header guard macros
On Fri, Mar 23, 2018 at 06:00:18PM +, Colin King wrote:
> From: Colin Ian King
>
> The macros for __PHYDMKFREE_H__ and __PHYDM_FEATURES_H__ contain
> typos and don't match the #if guard check. Defined them correctly.
>
> Cleans up clang warnings:
> warning: '__PHYDMKFREE_H__' is used as a header guard here, followed
> by #define of a different macro [-Wheader-guard]
> warning: '__PHYDM_FEATURES_H__' is used as a header guard here, followed
> by #define of a different macro [-Wheader-guard]
>
> Fixes: 9ce99b04b5b8 ("staging: r8822be: Add phydm mini driver")
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/rtlwifi/phydm/phydm_features.h | 2 +-
> drivers/staging/rtlwifi/phydm/phydm_kfree.h| 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtlwifi/phydm/phydm_features.h
> b/drivers/staging/rtlwifi/phydm/phydm_features.h
> index 37f6f0cd7235..a12361c6a1a0 100644
> --- a/drivers/staging/rtlwifi/phydm/phydm_features.h
> +++ b/drivers/staging/rtlwifi/phydm/phydm_features.h
> @@ -24,7 +24,7 @@
>
> */
>
> #ifndef __PHYDM_FEATURES_H__
> -#define __PHYDM_FEATURES
> +#define __PHYDM_FEATURES_H__
>
> /*phydm debyg report & tools*/
>
> diff --git a/drivers/staging/rtlwifi/phydm/phydm_kfree.h
> b/drivers/staging/rtlwifi/phydm/phydm_kfree.h
> index 1ee60059afc1..2c6b0a48e76e 100644
> --- a/drivers/staging/rtlwifi/phydm/phydm_kfree.h
> +++ b/drivers/staging/rtlwifi/phydm/phydm_kfree.h
> @@ -24,7 +24,7 @@
>
> */
>
> #ifndef __PHYDMKFREE_H__
> -#define __PHYDKFREE_H__
> +#define __PHYDKKFREE_H__
Close, but not quite :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: pi433: add descriptions for mutex locks
Hi Valentin, > But I have an idea to remove this kfifo_reset call in pi433_write > handling partial message writes: > > kfifo_reset(&device->tx_fifo); // TODO: maybe find a solution, not to > discard already stored, valid entries > > The writer could acquire the lock and than use kfifo_avail to check if > there is enough space to write the whole message. What do you think? Unfortunaly I can't find the time to have a closer look on the code this weekend - still busy with tax stuff :-( Idea sounds great. I'll try to look at the code and think about it during Easter hollidays. Cheers, Marcus -- Smarthome-Wolf UG (haftungsbeschränkt) Helene-Lange-Weg 23 80637 München Amtsgericht München, HRB 223529 Umastzsteuer-ID: DE304719911 Geschäftsführer: Marcus Wolf ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: bcm2835-audio: Release resources on module_exit()
On Sun, Mar 25, 2018 at 1:44 PM, Kirill Marinushkin wrote: > On 03/25/18 12:33, Andy Shevchenko wrote: >> On Fri, Mar 23, 2018 at 9:22 PM, Kirill Marinushkin >> wrote: >>> On 03/23/18 17:23, Andy Shevchenko wrote: After addressing above, FWIW, Reviewed-by: Andy Shevchenko >> Seems you missed my tag in new version. >> When someone gives you a tag and you are going to send a new version >> (w/o drastic changes), it's your responsibility to append it >> >> I will send a new mail with it, so, this time no need to resend. Just >> keep it for the future contributions. >> > > What is a "tag"? > I added in-reply-to this email, I added you as CC. > Isn't it enough to keep track of the versioning within a mailing list? > > Could you please clarify: what "tag" should I usually attach in addition? Section 13) in Submitting Patches [1] explains that. [1]: https://elixir.bootlin.com/linux/v4.16-rc6/source/Documentation/process/submitting-patches.rst -- With Best Regards, Andy Shevchenko ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: pi433: add descriptions for mutex locks
On Sun, Mar 25, 2018 at 03:00:09PM +0200, Marcus Wolf wrote: > Unfortunaly I can't find the time to have a closer look on the code this > weekend - still busy with tax stuff :-( > > Idea sounds great. I'll try to look at the code and think about it > during Easter hollidays. No problem, there is no hurry. But please do test the patch I sent yesterday: [PATCH] staging: pi433: cleanup tx_fifo locking As I don't have the hardware this is just compile tested now :) -- Valentin ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: pi433: add descriptions for mutex locks
Hi Valentin, I am not at home the next two weeks. So I can do a codereading on Easter, but testing will not take place earlier then mid/end of April :-( If you are interested, I can provide you an engineering sample of Pi433. Cheers, Marcus Am 25.03.2018 um 15:09 schrieb Valentin Vidic: > On Sun, Mar 25, 2018 at 03:00:09PM +0200, Marcus Wolf wrote: >> Unfortunaly I can't find the time to have a closer look on the code this >> weekend - still busy with tax stuff :-( >> >> Idea sounds great. I'll try to look at the code and think about it >> during Easter hollidays. > > No problem, there is no hurry. But please do test the patch I sent yesterday: > > [PATCH] staging: pi433: cleanup tx_fifo locking > > As I don't have the hardware this is just compile tested now :) > -- Smarthome-Wolf UG (haftungsbeschränkt) Helene-Lange-Weg 23 80637 München Amtsgericht München, HRB 223529 Umastzsteuer-ID: DE304719911 Geschäftsführer: Marcus Wolf ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: bcm2835-audio: Release resources on module_exit()
On 03/25/18 15:03, Andy Shevchenko wrote: > On Sun, Mar 25, 2018 at 1:44 PM, Kirill Marinushkin > wrote: >> On 03/25/18 12:33, Andy Shevchenko wrote: >>> On Fri, Mar 23, 2018 at 9:22 PM, Kirill Marinushkin >>> wrote: On 03/23/18 17:23, Andy Shevchenko wrote: > After addressing above, FWIW, > > Reviewed-by: Andy Shevchenko >>> Seems you missed my tag in new version. >>> When someone gives you a tag and you are going to send a new version >>> (w/o drastic changes), it's your responsibility to append it >>> >>> I will send a new mail with it, so, this time no need to resend. Just >>> keep it for the future contributions. >>> >> What is a "tag"? >> I added in-reply-to this email, I added you as CC. >> Isn't it enough to keep track of the versioning within a mailing list? >> >> Could you please clarify: what "tag" should I usually attach in addition? > Section 13) in Submitting Patches [1] explains that. > > [1]: > https://elixir.bootlin.com/linux/v4.16-rc6/source/Documentation/process/submitting-patches.rst > Thanks! ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: pi433: add descriptions for mutex locks
On Sun, Mar 25, 2018 at 03:12:52PM +0200, Marcus Wolf wrote: > I am not at home the next two weeks. So I can do a codereading on > Easter, but testing will not take place earlier then mid/end of April :-( > > If you are interested, I can provide you an engineering sample of Pi433. Sure, let me know which version of Rpi it supports and if I need two to test communication. -- Valentin ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
On Sat, 24 Mar 2018 15:45:21 -0700
John Syne wrote:
> > On Mar 24, 2018, at 8:02 AM, Jonathan Cameron wrote:
> >
> > On Mon, 19 Mar 2018 22:57:16 -0700
> > John Syne wrote:
> >
> >> Hi Jonathan,
> >>
> >> Thank you for all your hard work. Your feedback is really helpful. I’m
> >> surprised that no one from Analog Device has offered any suggestions.
> >>
> >
> > Sadly those active in the mainline linux kernel from ADI are focused in
> > other areas currently :(
> Good point. I will reach out to Analog Devices and get a name of someone
> who is knowledgeable in this area. Perhaps Lars-Peters has a suggestion.
Yes, Lars or Michael Hennerich (+CC) are my normal contact points.
I believe this is outside both of their areas, but they may be able to
put you in contact with the right person. If nothing else it would be
good to make someone in ADI aware that people care about linux support for
these parts!
> >> Anyway, please see my comments inline below
> >>
> >> Regards,
> >> John
> >>
> >>
> >>
> >>
> >>
> >>> On Mar 18, 2018, at 5:23 AM, Jonathan Cameron wrote:
> >>>
> >>> On Sat, 17 Mar 2018 23:11:45 -0700
> >>> John Syne wrote:
> >>>
> Hi Jonathan,
> >>> Hi John and All,
> >>>
> >>> I'd love to get some additional input on this from anyone interested.
> >>> There are a lot of weird and wonderful derived quantities in an energy
> >>> meter and it seems we need to make some fundamental changes to support
> >>> them - including potentially a userspace breaking change to the event
> >>> code description.
> >>>
>
> Here is the complete list of registers for the ADE7878 which I copied
> from the data sheet. I added a column “IIO Attribute” which I hope
> follows your IIO ABI. Please make any changes you feel are incorrect.
> BTW, there are several registers that cannot be generalized and are used
> purely for chip configuration. I think we should add a new naming
> convention, namely {register}_{}. Also, I see in the
> sys_bus_iio doc
No, registers can always be broken up in to real meaningful values if
they are exposed to userspace and userspace has a reason to tweak
them.
We never expose raw registers to userspace except through
debugfs and the clue is in the name - purely for debug.
> >
> >
> >
> >
> in_accel_x_peak_raw
>
> so shouldn’t the phase be represented as follows:
>
> in_current_A_scale
> >>> I'm still confused. What does A represent here? I assumed that was a
> >>> wild
> >>> card for the channel number before but clearly not.
> >>>
> >>> Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
> >>> I guess they sort of look like axis, and sort of like independent
> >>> channels.
> >>> So could be indexed or done via modifiers depending on how you look at
> >>> it.
> >> In metering terminology, the three phases are commonly referred to as RED,
> >> GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the
> >> ABCN nomenclature so anyone using this driver will probably have
> >> referenced the Analog Devices datasheet so this terminology should be
> >> familiar.
> > Which is more commonly used in general? We are designing what we hope
> > will be a general interface and last thing we want is to have everyone
> > not using ADI parts confused! Personally not assigning 'colours' makes
> > more sense to me.
> I did a little research and here is the naming used by vendor:
>
> ST Microelectronics: P1, P2, P3, N
> http://www.st.com/content/ccc/resource/technical/document/application_note/5e/f4/22/4d/90/96/4c/c4/CD00153264.pdf/files/CD00153264.pdf/jcr:content/translations/en.CD00153264.pdf
>
> Teridian: ABCN
> https://www.mouser.com/ds/2/256/71M6513-71M6513H-22603.pdf
>
> Maxim Integrated: ABCN
> https://datasheets.maximintegrated.com/en/ds/78M6631.pdf
>
> Microchip: ABCN
> http://ww1.microchip.com/downloads/en/DeviceDoc/51643b.pdf
>
> NXP: L1, L2, L3, N
> https://www.nxp.com/support/developer-resources/reference-designs/kinetis-km3x-256-mcu-three-phase-metering-reference-design:RD-KM3x-256-3PH-METERING
>
> Renesas: ABCN
> https://www.renesas.com/en-us/solutions/home/metering/power-meter-three.html
>
> So the most consistent would be ABCN
Cool. Makes sense to go with that. Thanks for checking this out.
>
>
> All vendor providing three phase energy metering ICs use the ABCN
> terminology.
> >
> > (btw please wrap any lines that don't need to be long to around 80
> > characters).
> >
> >>>
> >>> Hmm. With neutral in there as well I guess we need to make them
> >>> modifiers (but might change my mind later ;)
> >>>
> >>> Particularly as we are using the the modifier for RMS under the previous
> >>> plan... It appears we should treat that instead like we did for peak
> >>> and do it as an additional info mask element. The problem with doing that
> >>> on a continuous measurement is that we can't treat it as a ch
Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
On Sat, 24 Mar 2018 16:06:17 -0700 John Syne wrote: This thread is becoming unmanageable so I am cropping this down to just the questions that remain open. > >> Probably easier to copy and paste this table into a spreadsheet. Let me > >> know if there is anything I got wrong. Thank you again for all your help. > > Yeah, we need to shrink this if we do it again. > I’ll send an updated copy after this e-mail. Can you accept a spreadsheet > attachment or a CSV file? We need to keep the discussion visible on list so it needs to stay in plain text. Just need to drop any columns we aren't caring about to make it easier to read. > > ... > >> 0x439C CVAROS in_powerreactive0_phaseC_offset in > >> powerreactive 0 phaseC offset R/W 24 32 ZPSE S > >> 0x00Phase C total reactive power offset adjust (ADE7858, > >> ADE7868, and ADE7878). > >> 0x439D AFWGAIN in_power0_phaseA_fundamental_scale in > >> power 0 phaseA_fundamental scale R/W 24 32 ZPSE S > >> 0x00Phase A fundamental active power gain adjust. > >> Location reserved for ADE7854, ADE7858, and ADE7868. > > Hmm. Fundamental needs to be represented using a separate channel index > > and description of the frequency filters applied. That should map it > > a generic way. > How do I do this? Define additional channels with different index and for them use the the infomask elements IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY, IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY and provide suitable values from the read_raw callbacks for that channel. ... > >> 0x43C0 AIRMS in_current0_phaseA_rms in current 0 > >> phaseA_rms R 24 32 ZP S N/A4Phase A > >> current rms value. > > in_current0_phaseA_rms_raw as otherwise we don't know we need to apply > > in_current0_phaseA_rms_scale to it (or the shared value that maps to that). > > > Yeah, this is still confusion to me. This should read > in_current0_phaseA_rms_gain > as it directly affects the value in_current0_phaseA_rms_raw. We still have to > apply > a scale value to turn this cryptic number into something meaningful. So I'm a little lost. We have variable gain fine. Does it effect the necessary scale factor to go from raw to real value or not? 1) Yes it does - then roll it as appropriate into the _scale attribute. It should not be separated. This often requires some interesting maths but is a onetime thing as the value isn't changing dynamically. 2) No it doesn't - then it is calibgain as it represents a necessary parameter to change the incoming circuit to compensate for external effects. It is possible you have a mixture of the two and hence need both but that is normally only the case with devices where the calibgain is about fixing the factory calibration. ... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
On Sun, 25 Mar 2018 01:29:41 -0700 John Syne wrote: > Hi Jonathan, Hi John, Please wrap your normal emails (excepting tables) to 80 chars. > > I was speaking with Rodrigo and here is what I think must be done to move > ADE7878 out of staging > > Here are the steps as I see them: > > 1) Define the IIO Attributes so they are consistent with the IIO ABI. This >should be pretty simple given agreement on the naming convention. Please don't go just changing attribute names - the driver needs to use the standard approach of iio_chan_spec and create the vast majority of attributes automatically via that. There 'may' be a few corner cases wee don't want to make generic and they 'might' be exposed as attributes. I suspect this change is the 'big' job. The core support necessary for RMS and MAV (computedtype or whatever we call it) will need adding as well. That is a separate patch set with support for some examples in the dummy driver. > 2) Map the ADE7854 interrupt status to IIO events. This requires an >interrupt processing section. > 3) Add DeviceTree support. > 4) Create DeviceTree overlay for the ADE7854. More a case of bindings for now. If those are used via an overlay fine but given we don't have any boards with one one in mainline, this is an implementation detail for the user rather than part of moving this driver out of staging. > 5) Update ADE7854 probe to read in the DeviceTree register settings. > 6) Add support for power modes (PM1, PM2). This isn't necessary for a move out of staging (nice to have though). > 7) Not sure if we will support measurement streaming on the ADE7854. >The problem is ADE7854 is designed as an SPI master, which means >it controls the SPI clock, so the driver must support SPI slave >mode. However, the Linux Kernel does not currently support SPI >slave mode. We have three choices to make this work and they >are all a lot of work: 1) Add support for SPI Slave mode to the >kernel, 2) Use hardware to convert SPI signals to I2S signals >and with the use of a custom codec, use the ALSA framework to >stream the samples (this is an approach I used, but I don’t like >it), 3) Move the I2S driver out of the sound subsystem and use it >together with DMA to stream samples directly into the ADE7854 driver >(my preferred solutions). Perhaps Mark Brown has some ideas on how > to make this work. I'll be honest, this is an end of line part and frankly more than a little crazy. I would go with simply not supporting the measurement streaming at all for this part. If you really need it we can then move onto the how part, but from what you have said I'm guessing you don't care except in an abstract 'it would be nice' sort of a way? > > The ADE9000 will be much easier because it uses an SPI Slave interface. > > I hope I have captured everything, but let me know if I have missed anything. > That will do for now ;) I'm sure there will be details that need tidying up once we have the above done, but that's true for any new driver (and this will be nearly a new driver before things are done). Jonathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH][V3] staging: r8822be: fix typos in header guard macros
From: Colin Ian King The macros for __PHYDMKFREE_H__ and __PHYDM_FEATURES_H__ contain typos and don't match the #if guard check. Defined them correctly. Cleans up clang warnings: warning: '__PHYDMKFREE_H__' is used as a header guard here, followed by #define of a different macro [-Wheader-guard] warning: '__PHYDM_FEATURES_H__' is used as a header guard here, followed by #define of a different macro [-Wheader-guard] Signed-off-by: Colin Ian King --- drivers/staging/rtlwifi/phydm/phydm_features.h | 2 +- drivers/staging/rtlwifi/phydm/phydm_kfree.h| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/rtlwifi/phydm/phydm_features.h b/drivers/staging/rtlwifi/phydm/phydm_features.h index 37f6f0cd7235..a12361c6a1a0 100644 --- a/drivers/staging/rtlwifi/phydm/phydm_features.h +++ b/drivers/staging/rtlwifi/phydm/phydm_features.h @@ -24,7 +24,7 @@ */ #ifndef __PHYDM_FEATURES_H__ -#define __PHYDM_FEATURES +#define __PHYDM_FEATURES_H__ /*phydm debyg report & tools*/ diff --git a/drivers/staging/rtlwifi/phydm/phydm_kfree.h b/drivers/staging/rtlwifi/phydm/phydm_kfree.h index 1ee60059afc1..2c6b0a48e76e 100644 --- a/drivers/staging/rtlwifi/phydm/phydm_kfree.h +++ b/drivers/staging/rtlwifi/phydm/phydm_kfree.h @@ -24,7 +24,7 @@ */ #ifndef __PHYDMKFREE_H__ -#define __PHYDKFREE_H__ +#define __PHYDMKFREE_H__ #define KFREE_VERSION "1.0" -- 2.15.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: iio: tsl2x7x: use auto increment I2C protocol
On Sat, 24 Mar 2018 16:05:52 -0400
Brian Masney wrote:
> The hardware supports 16-bit ALS and proximity readings, however the
> datasheet recommends using the I2C auto increment protocol so that the
> correct high and low bytes are read even if the integration cycle ends
> between reading the lower and upper registers. More information about
> this protocol can be found at https://www.i2c-bus.org/auto-increment/.
>
> Signed-off-by: Brian Masney
This isn't what I'd normally expect to see when autoincrement is going on.
You normally have to keep the transfer from ending.
See below
However looking at the datasheet it seems to be doing exactly what
you have here so that wins the 'odd' award for the day.
Ah well. All I'm going to do is delete the reference to the standard
from this patch as it isn't complying with it...
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/staging/iio/light/tsl2x7x.c | 100
>
> 1 file changed, 67 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index 77a81d75af4f..8530bccdb317 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -80,6 +80,8 @@
> /* tsl2X7X cmd reg masks */
> #define TSL2X7X_CMD_REG 0x80
> #define TSL2X7X_CMD_SPL_FN 0x60
> +#define TSL2X7X_CMD_REPEAT_PROTO 0x00
> +#define TSL2X7X_CMD_AUTOINC_PROTO0x20
>
> #define TSL2X7X_CMD_PROX_INT_CLR 0X05
> #define TSL2X7X_CMD_ALS_INT_CLR 0x06
> @@ -320,6 +322,55 @@ static int tsl2x7x_write_control_reg(struct tsl2X7X_chip
> *chip, u8 data)
> return ret;
> }
>
> +static int tsl2x7x_read_autoinc_regs(struct tsl2X7X_chip *chip, int
> lower_reg,
> + int upper_reg)
> +{
> + u8 buf[2];
> + int ret;
> +
> + ret = i2c_smbus_write_byte(chip->client,
> +TSL2X7X_CMD_REG | TSL2X7X_CMD_AUTOINC_PROTO |
> +lower_reg);
> + if (ret < 0) {
> + dev_err(&chip->client->dev,
> + "%s: failed to enable auto increment protocol: %d\n",
> + __func__, ret);
> + return ret;
> + }
> +
> + ret = i2c_smbus_read_byte_data(chip->client,
> +TSL2X7X_CMD_REG | lower_reg);
> + if (ret < 0) {
> + dev_err(&chip->client->dev,
> + "%s: failed to read from register %x: %d\n", __func__,
> + lower_reg, ret);
> + return ret;
> + }
> + buf[0] = ret;
> +
The description you link to implies we can't hae a stop between these.
That would mean you need to do them in one go.
i2c_smbus_read_block_data would do that...
> + ret = i2c_smbus_read_byte_data(chip->client,
> +TSL2X7X_CMD_REG | upper_reg);
> + if (ret < 0) {
> + dev_err(&chip->client->dev,
> + "%s: failed to read from register %x: %d\n", __func__,
> + upper_reg, ret);
> + return ret;
> + }
> + buf[1] = ret;
> +
> + ret = i2c_smbus_write_byte(chip->client,
> +TSL2X7X_CMD_REG | TSL2X7X_CMD_REPEAT_PROTO |
> +lower_reg);
> + if (ret < 0) {
> + dev_err(&chip->client->dev,
> + "%s: failed to enable repeated byte protocol: %d\n",
> + __func__, ret);
> + return ret;
> + }
> +
> + return le16_to_cpup((const __le16 *)&buf[0]);
> +}
> +
> /**
> * tsl2x7x_get_lux() - Reads and calculates current lux value.
> * @indio_dev: pointer to IIO device
> @@ -340,9 +391,8 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
> struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> struct tsl2x7x_lux *p;
> u32 lux, ratio;
> - int i, ret;
> u64 lux64;
> - u8 buf[4];
> + int ret;
>
> mutex_lock(&chip->als_mutex);
>
> @@ -366,23 +416,17 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
> goto out_unlock;
> }
>
> - for (i = 0; i < 4; i++) {
> - int reg = TSL2X7X_CMD_REG | (TSL2X7X_ALS_CHAN0LO + i);
> -
> - ret = i2c_smbus_read_byte_data(chip->client, reg);
> - if (ret < 0) {
> - dev_err(&chip->client->dev,
> - "%s: failed to read from register %x: %d\n",
> - __func__, reg, ret);
> - goto out_unlock;
> - }
> -
> - buf[i] = ret;
> - }
> + ret = tsl2x7x_read_autoinc_regs(chip, TSL2X7X_ALS_CHAN0LO,
> + TSL2X7X_ALS_CHAN0HI);
> + if (ret < 0)
> + goto out_unlock;
> + chip->al
Re: [PATCH 2/4] staging: iio: tsl2x7x: move IIO_CHAN_INFO_CALIB{SCALE,BIAS} to IIO_LIGHT channel
On Sat, 24 Mar 2018 16:05:53 -0400
Brian Masney wrote:
> The IIO_CHAN_INFO_CALIBSCALE and IIO_CHAN_INFO_CALIBBIAS masks are
> currently associated with the IIO_INTENSITY channel but should be
> associated with the IIO_LIGHT channel since these values are used to
> calculate the lux. Directory listing of the sysfs attributes for a
> TSL2772 with this patch applied:
They may be used to calculate the lux, but as far as I can tell
they are gain controls on the underlying intensity channels.
>
> dev
> events
> in_illuminance0_calibbias
> in_illuminance0_calibrate
> in_illuminance0_calibscale
> in_illuminance0_calibscale_available
> in_illuminance0_input
> in_illuminance0_integration_time
> in_illuminance0_integration_time_available
I'd missed this before, but the integration time is also for
the two intensity readings. An argument could be made
for it also being a parameter of illuminance but
it definitely needs to be on for the intensity channels.
> in_illuminance0_lux_table
> in_illuminance0_target_input
> in_intensity0_raw
> in_intensity1_raw
> in_proximity0_calibrate
> in_proximity0_calibscale
> in_proximity0_calibscale_available
> in_proximity0_raw
> name
> of_node
> power
> subsystem
> uevent
So for this one I'm unconvinced.
Jonathan
>
> Signed-off-by: Brian Masney
> ---
> drivers/staging/iio/light/tsl2x7x.c | 24
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index 8530bccdb317..d5a237fb0a0b 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1491,14 +1491,14 @@ static const struct tsl2x7x_chip_info
> tsl2x7x_chip_info_tbl[] = {
> .indexed = 1,
> .channel = 0,
> .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> - BIT(IIO_CHAN_INFO_INT_TIME),
> + BIT(IIO_CHAN_INFO_INT_TIME) |
> + BIT(IIO_CHAN_INFO_CALIBSCALE) |
> + BIT(IIO_CHAN_INFO_CALIBBIAS),
> }, {
> .type = IIO_INTENSITY,
> .indexed = 1,
> .channel = 0,
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> - BIT(IIO_CHAN_INFO_CALIBSCALE) |
> - BIT(IIO_CHAN_INFO_CALIBBIAS),
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> .event_spec = tsl2x7x_events,
> .num_event_specs = ARRAY_SIZE(tsl2x7x_events),
> }, {
> @@ -1531,14 +1531,14 @@ static const struct tsl2x7x_chip_info
> tsl2x7x_chip_info_tbl[] = {
> .indexed = 1,
> .channel = 0,
> .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> - BIT(IIO_CHAN_INFO_INT_TIME),
> + BIT(IIO_CHAN_INFO_INT_TIME) |
> + BIT(IIO_CHAN_INFO_CALIBSCALE) |
> + BIT(IIO_CHAN_INFO_CALIBBIAS),
> }, {
> .type = IIO_INTENSITY,
> .indexed = 1,
> .channel = 0,
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> - BIT(IIO_CHAN_INFO_CALIBSCALE) |
> - BIT(IIO_CHAN_INFO_CALIBBIAS),
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> .event_spec = tsl2x7x_events,
> .num_event_specs = ARRAY_SIZE(tsl2x7x_events),
> }, {
> @@ -1580,14 +1580,14 @@ static const struct tsl2x7x_chip_info
> tsl2x7x_chip_info_tbl[] = {
> .indexed = 1,
> .channel = 0,
> .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> - BIT(IIO_CHAN_INFO_INT_TIME),
> + BIT(IIO_CHAN_INFO_INT_TIME) |
> + BIT(IIO_CHAN_INFO_CALIBSCALE) |
> + BIT(IIO_CHAN_INFO_CALIBBIAS),
> }, {
> .type = IIO_INTENSITY,
> .indexed = 1,
> .channel = 0,
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> - BIT(IIO_CHAN_INFO_CALIBSCALE) |
> - BIT(IIO_CHAN_INFO_CALIBBIAS),
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> .event_spec = tsl2x7x_events,
> .num_event_specs = ARRAY_SIZE(tsl2x7x_events),
> }, {
___
devel mailing list
de...@linuxdriverproject.org
Re: [PATCH 3/4] staging: iio: tsl2x7x: use either direction for IIO_EV_INFO_{ENABLE,PERIOD}
On Sat, 24 Mar 2018 16:05:54 -0400
Brian Masney wrote:
> The events IIO_EV_INFO_VALUE and IIO_EV_INFO_ENABLE currently have a
> falling and rising direction configured. There does not need to be a
> separate distinction so this patch changes these to use the
> either direction. Directory listing of event sysfs attributes for a
> TSL2772 with this patch applied:
>
> in_intensity0_thresh_either_en
> in_intensity0_thresh_either_period
> in_intensity0_thresh_falling_value
> in_intensity0_thresh_rising_value
> in_proximity0_thresh_either_en
> in_proximity0_thresh_either_period
> in_proximity0_thresh_falling_value
> in_proximity0_thresh_rising_value
>
> Signed-off-by: Brian Masney
This one is fine and reasonably clear of patch 2 so I can still apply it.
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/staging/iio/light/tsl2x7x.c | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index d5a237fb0a0b..940bea2378a9 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1469,17 +1469,16 @@ static const struct iio_event_spec tsl2x7x_events[] =
> {
> {
> .type = IIO_EV_TYPE_THRESH,
> .dir = IIO_EV_DIR_RISING,
> - .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> - BIT(IIO_EV_INFO_ENABLE),
> + .mask_separate = BIT(IIO_EV_INFO_VALUE),
> }, {
> .type = IIO_EV_TYPE_THRESH,
> .dir = IIO_EV_DIR_FALLING,
> - .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> - BIT(IIO_EV_INFO_ENABLE),
> + .mask_separate = BIT(IIO_EV_INFO_VALUE),
> }, {
> .type = IIO_EV_TYPE_THRESH,
> .dir = IIO_EV_DIR_EITHER,
> - .mask_separate = BIT(IIO_EV_INFO_PERIOD),
> + .mask_separate = BIT(IIO_EV_INFO_PERIOD) |
> + BIT(IIO_EV_INFO_ENABLE),
> },
> };
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] staging: iio: tsl2x7x: move out of staging
On Sat, 24 Mar 2018 16:05:55 -0400 Brian Masney wrote: > Move the tsl2x7x driver out of staging and into mainline. > > Signed-off-by: Brian Masney Various comments inline. I'm wondering if we should rename the driver before moving it. The wild cards bother me as we are very likely to see something not compatible in this namespace at some point.. There are parts already with an extra digit which some might think match... tsl25711 for example. Interestingly some of the links on the AMS website link back to the 2571 part and it turns out on the current data sheet that the distinction is around the supported voltages for i2c. Hmm. We should probably have these parts listed in the OF table as well as we will have people writing DT files based on the part number they fitted, not the generic part number they can figure out from the datasheet. What do you think? Bit of find and replace would deal with this. Just pick a representative part and name it all after that. Jonathan > --- > Note: I intentionally ran git format-patch with --no-renames since > Jonathan likes to see the whole files in the email body for staging > graduation patches. Thanks. > > The #include "tsl2x7x.h" was changed to #include > in tsl2x7x.c during the rename. > > drivers/iio/light/Kconfig |8 + > drivers/iio/light/Makefile|1 + > drivers/iio/light/tsl2x7x.c | 1784 > + > drivers/staging/iio/Kconfig |1 - > drivers/staging/iio/Makefile |1 - > drivers/staging/iio/light/Kconfig | 14 - > drivers/staging/iio/light/Makefile|5 - > drivers/staging/iio/light/tsl2x7x.c | 1784 > - > drivers/staging/iio/light/tsl2x7x.h | 103 -- > include/linux/platform_data/tsl2x7x.h | 103 ++ > 10 files changed, 1896 insertions(+), 1908 deletions(-) > create mode 100644 drivers/iio/light/tsl2x7x.c > delete mode 100644 drivers/staging/iio/light/Kconfig > delete mode 100644 drivers/staging/iio/light/Makefile > delete mode 100644 drivers/staging/iio/light/tsl2x7x.c > delete mode 100644 drivers/staging/iio/light/tsl2x7x.h > create mode 100644 include/linux/platform_data/tsl2x7x.h > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > index 074e50657366..8161e3e1ba1b 100644 > --- a/drivers/iio/light/Kconfig > +++ b/drivers/iio/light/Kconfig > @@ -409,6 +409,14 @@ config TSL2583 >Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices. >Access ALS data via iio, sysfs. > > +config TSL2x7x > + tristate "TAOS TSL/TMD2x71 and TSL/TMD2x72 Family of light and > proximity sensors" > + depends on I2C > + help > + Support for: tsl2571, tsl2671, tmd2671, tsl2771, tmd2771, tsl2572, > tsl2672, > + tmd2672, tsl2772, tmd2772 devices. > + Provides iio_events and direct access via sysfs. > + > config TSL4531 > tristate "TAOS TSL4531 ambient light sensors" > depends on I2C > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile > index f1777036d4f8..a20a3662950b 100644 > --- a/drivers/iio/light/Makefile > +++ b/drivers/iio/light/Makefile > @@ -40,6 +40,7 @@ obj-$(CONFIG_ST_UVIS25_SPI) += st_uvis25_spi.o > obj-$(CONFIG_TCS3414)+= tcs3414.o > obj-$(CONFIG_TCS3472)+= tcs3472.o > obj-$(CONFIG_TSL2583)+= tsl2583.o > +obj-$(CONFIG_TSL2x7x)+= tsl2x7x.o > obj-$(CONFIG_TSL4531)+= tsl4531.o > obj-$(CONFIG_US5182D)+= us5182d.o > obj-$(CONFIG_VCNL4000) += vcnl4000.o > diff --git a/drivers/iio/light/tsl2x7x.c b/drivers/iio/light/tsl2x7x.c > new file mode 100644 > index ..fffd23ae5c72 > --- /dev/null > +++ b/drivers/iio/light/tsl2x7x.c > @@ -0,0 +1,1784 @@ > +/* > + * Device driver for monitoring ambient light intensity in (lux) > + * and proximity detection (prox) within the TAOS TSL2X7X family of devices. > + * > + * Copyright (c) 2012, TAOS Corporation. > + * Copyright (c) 2017-2018 Brian Masney > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * 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. See the GNU General Public License for > + * more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* Cal defs*/ space before */ > +#define PROX_STAT_CAL0 > +#define PROX_STAT_SAMP 1 > +#define MAX_SAMPLES_CAL 200 > + > +/* TSL2X7X D
Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
> On Mar 25, 2018, at 9:29 AM, Jonathan Cameron wrote:
>
> On Sat, 24 Mar 2018 15:45:21 -0700
> John Syne wrote:
>
>>> On Mar 24, 2018, at 8:02 AM, Jonathan Cameron wrote:
>>>
>>> On Mon, 19 Mar 2018 22:57:16 -0700
>>> John Syne wrote:
>>>
Hi Jonathan,
Thank you for all your hard work. Your feedback is really helpful. I’m
surprised that no one from Analog Device has offered any suggestions.
>>>
>>> Sadly those active in the mainline linux kernel from ADI are focused in
>>> other areas currently :(
>> Good point. I will reach out to Analog Devices and get a name of someone
>> who is knowledgeable in this area. Perhaps Lars-Peters has a suggestion.
> Yes, Lars or Michael Hennerich (+CC) are my normal contact points.
> I believe this is outside both of their areas, but they may be able to
> put you in contact with the right person. If nothing else it would be
> good to make someone in ADI aware that people care about linux support for
> these parts!
I sent in a request to ADI for a contact person to help with the review.
>
Anyway, please see my comments inline below
Regards,
John
> On Mar 18, 2018, at 5:23 AM, Jonathan Cameron wrote:
>
> On Sat, 17 Mar 2018 23:11:45 -0700
> John Syne wrote:
>
>> Hi Jonathan,
> Hi John and All,
>
> I'd love to get some additional input on this from anyone interested.
> There are a lot of weird and wonderful derived quantities in an energy
> meter and it seems we need to make some fundamental changes to support
> them - including potentially a userspace breaking change to the event
> code description.
>
>>
>> Here is the complete list of registers for the ADE7878 which I copied
>> from the data sheet. I added a column “IIO Attribute” which I hope
>> follows your IIO ABI. Please make any changes you feel are incorrect.
>> BTW, there are several registers that cannot be generalized and are used
>> purely for chip configuration. I think we should add a new naming
>> convention, namely {register}_{}. Also, I see in the
>> sys_bus_iio doc
>
> No, registers can always be broken up in to real meaningful values if
> they are exposed to userspace and userspace has a reason to tweak
> them.
>
> We never expose raw registers to userspace except through
> debugfs and the clue is in the name - purely for debug.
OK
>
>
>>>
>>>
>>>
>>>
>> in_accel_x_peak_raw
>>
>> so shouldn’t the phase be represented as follows:
>>
>> in_current_A_scale
> I'm still confused. What does A represent here? I assumed that was a
> wild
> card for the channel number before but clearly not.
>
> Ah, you are labelling the 3 separate phases as A, B and C. Hmm.
> I guess they sort of look like axis, and sort of like independent
> channels.
> So could be indexed or done via modifiers depending on how you look at
> it.
In metering terminology, the three phases are commonly referred to as RED,
GREEN, BLUE or PhaseA, PhaseB, PhaseC and Neutral. Analog Devices uses the
ABCN nomenclature so anyone using this driver will probably have
referenced the Analog Devices datasheet so this terminology should be
familiar.
>>> Which is more commonly used in general? We are designing what we hope
>>> will be a general interface and last thing we want is to have everyone
>>> not using ADI parts confused! Personally not assigning 'colours' makes
>>> more sense to me.
>> I did a little research and here is the naming used by vendor:
>>
>> ST Microelectronics: P1, P2, P3, N
>> http://www.st.com/content/ccc/resource/technical/document/application_note/5e/f4/22/4d/90/96/4c/c4/CD00153264.pdf/files/CD00153264.pdf/jcr:content/translations/en.CD00153264.pdf
>>
>> Teridian: ABCN
>> https://www.mouser.com/ds/2/256/71M6513-71M6513H-22603.pdf
>>
>> Maxim Integrated: ABCN
>> https://datasheets.maximintegrated.com/en/ds/78M6631.pdf
>>
>> Microchip: ABCN
>> http://ww1.microchip.com/downloads/en/DeviceDoc/51643b.pdf
>>
>> NXP: L1, L2, L3, N
>> https://www.nxp.com/support/developer-resources/reference-designs/kinetis-km3x-256-mcu-three-phase-metering-reference-design:RD-KM3x-256-3PH-METERING
>>
>> Renesas: ABCN
>> https://www.renesas.com/en-us/solutions/home/metering/power-meter-three.html
>>
>> So the most consistent would be ABCN
> Cool. Makes sense to go with that. Thanks for checking this out.
>
>>
>>
>> All vendor providing three phase energy metering ICs use the ABCN
>> terminology.
>>>
>>> (btw please wrap any lines that don't need to be long to around 80
>>> characters).
>>>
>
> Hmm. With neutral in there as well I guess we need to make them
> modifiers (but might change my mind later ;)
>
> Particularly as we are using the the modifier for RMS under the previous
> plan... It appears we should
Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
> On Mar 25, 2018, at 9:44 AM, Jonathan Cameron wrote: > > On Sat, 24 Mar 2018 16:06:17 -0700 > John Syne wrote: > > This thread is becoming unmanageable so I am cropping this down to just > the questions that remain open. > Probably easier to copy and paste this table into a spreadsheet. Let me know if there is anything I got wrong. Thank you again for all your help. >>> Yeah, we need to shrink this if we do it again. >> I’ll send an updated copy after this e-mail. Can you accept a spreadsheet >> attachment or a CSV file? > We need to keep the discussion visible on list so it needs to stay in > plain text. Just need to drop any columns we aren't caring about to make > it easier to read. Done >>> > ... 0x439C CVAROS in_powerreactive0_phaseC_offset in powerreactive 0 phaseC offset R/W 24 32 ZPSE S 0x00Phase C total reactive power offset adjust (ADE7858, ADE7868, and ADE7878). 0x439D AFWGAIN in_power0_phaseA_fundamental_scale in power 0 phaseA_fundamental scale R/W 24 32 ZPSE S 0x00Phase A fundamental active power gain adjust. Location reserved for ADE7854, ADE7858, and ADE7868. >>> Hmm. Fundamental needs to be represented using a separate channel index >>> and description of the frequency filters applied. That should map it >>> a generic way. >> How do I do this? > Define additional channels with different index and for them > use the the infomask elements > IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY, > IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY > and provide suitable values from the read_raw callbacks for that > channel. > > ... 0x43C0 AIRMS in_current0_phaseA_rms in current 0 phaseA_rms R 24 32 ZP S N/A4Phase A current rms value. >>> in_current0_phaseA_rms_raw as otherwise we don't know we need to apply >>> in_current0_phaseA_rms_scale to it (or the shared value that maps to that). >>> >> Yeah, this is still confusion to me. This should read >> in_current0_phaseA_rms_gain >> as it directly affects the value in_current0_phaseA_rms_raw. We still have >> to apply >> a scale value to turn this cryptic number into something meaningful. > So I'm a little lost. We have variable gain fine. > Does it effect the necessary scale factor to go from raw to real value or not? Yes > 1) Yes it does - then roll it as appropriate into the _scale attribute. > It should not be separated. This often requires some interesting maths > but is a onetime thing as the value isn't changing dynamically. > > 2) No it doesn't - then it is calibgain as it represents a necessary > parameter to change the incoming circuit to compensate for external effects. The hardwaregain and calibgain are setup only once and do not vary. The ADE7854 can accept various input ranges such as 0-250mV, 0-5V, etc The calibgain is used to compensate for the inaccuracies of the sensors and the termination components. > > It is possible you have a mixture of the two and hence need both but that > is normally only the case with devices where the calibgain is about fixing > the factory calibration. > > ... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
> > On Mar 25, 2018, at 9:44 AM, Jonathan Cameron wrote: > > On Sat, 24 Mar 2018 16:06:17 -0700 > John Syne wrote: > > This thread is becoming unmanageable so I am cropping this down to just > the questions that remain open. > Probably easier to copy and paste this table into a spreadsheet. Let me know if there is anything I got wrong. Thank you again for all your help. >>> Yeah, we need to shrink this if we do it again. >> I’ll send an updated copy after this e-mail. Can you accept a spreadsheet >> attachment or a CSV file? > We need to keep the discussion visible on list so it needs to stay in > plain text. Just need to drop any columns we aren't caring about to make > it easier to read. Done >>> > ... 0x439C CVAROS in_powerreactive0_phaseC_offset in powerreactive 0 phaseC offset R/W 24 32 ZPSE S 0x00Phase C total reactive power offset adjust (ADE7858, ADE7868, and ADE7878). 0x439D AFWGAIN in_power0_phaseA_fundamental_scale in power 0 phaseA_fundamental scale R/W 24 32 ZPSE S 0x00Phase A fundamental active power gain adjust. Location reserved for ADE7854, ADE7858, and ADE7868. >>> Hmm. Fundamental needs to be represented using a separate channel index >>> and description of the frequency filters applied. That should map it >>> a generic way. >> How do I do this? > Define additional channels with different index and for them > use the the infomask elements > IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY, > IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY > and provide suitable values from the read_raw callbacks for that > channel. > > ... 0x43C0 AIRMS in_current0_phaseA_rms in current 0 phaseA_rms R 24 32 ZP S N/A4Phase A current rms value. >>> in_current0_phaseA_rms_raw as otherwise we don't know we need to apply >>> in_current0_phaseA_rms_scale to it (or the shared value that maps to that). >>> >> Yeah, this is still confusion to me. This should read >> in_current0_phaseA_rms_gain >> as it directly affects the value in_current0_phaseA_rms_raw. We still have >> to apply >> a scale value to turn this cryptic number into something meaningful. > So I'm a little lost. We have variable gain fine. > Does it effect the necessary scale factor to go from raw to real value or not? Yes > 1) Yes it does - then roll it as appropriate into the _scale attribute. > It should not be separated. This often requires some interesting maths > but is a onetime thing as the value isn't changing dynamically. > > 2) No it doesn't - then it is calibgain as it represents a necessary > parameter to change the incoming circuit to compensate for external effects. The hardwaregain and calibgain are setup only once and do not vary. The ADE7854 can accept various input ranges such as 0-250mV, 0-5V, etc The calibgain is used to compensate for the inaccuracies of the sensors and the termination components. > > It is possible you have a mixture of the two and hence need both but that > is normally only the case with devices where the calibgain is about fixing > the factory calibration. > > ... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: meter ABI: (was Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR)
> On Mar 25, 2018, at 9:54 AM, Jonathan Cameron wrote: > > On Sun, 25 Mar 2018 01:29:41 -0700 > John Syne wrote: > >> Hi Jonathan, > Hi John, > > Please wrap your normal emails (excepting tables) to 80 chars. Yeah, I’m trying to do that, but I use a Mac and Apple Mail doesn’t have a feature to wrap at 80 automatically so I have to do this manually. > >> >> I was speaking with Rodrigo and here is what I think must be done to move >> ADE7878 out of staging >> >> Here are the steps as I see them: >> >> 1) Define the IIO Attributes so they are consistent with the IIO ABI. This >> should be pretty simple given agreement on the naming convention. > Please don't go just changing attribute names - the driver needs to > use the standard approach of iio_chan_spec and create the vast > majority of attributes automatically via that. There 'may' be > a few corner cases wee don't want to make generic and they 'might' > be exposed as attributes. > > I suspect this change is the 'big' job. The core support necessary > for RMS and MAV (computedtype or whatever we call it) will need adding > as well. That is a separate patch set with support for some examples > in the dummy driver. Great that you mentioned this, because I though we only needed to modify the IIO_ATTR naming. I’ll take a look at iio_chan_spec and the examples in the dummy driver. > >> 2) Map the ADE7854 interrupt status to IIO events. This requires an >> interrupt processing section. >> 3) Add DeviceTree support. >> 4) Create DeviceTree overlay for the ADE7854. > More a case of bindings for now. If those are used via an overlay > fine but given we don't have any boards with one one in mainline, > this is an implementation detail for the user rather than part > of moving this driver out of staging. I was thinking more along the lines of documentation to show a developer what was needed to get the driver working. > >> 5) Update ADE7854 probe to read in the DeviceTree register settings. >> 6) Add support for power modes (PM1, PM2). > This isn't necessary for a move out of staging (nice to have though). > >> 7) Not sure if we will support measurement streaming on the ADE7854. >> The problem is ADE7854 is designed as an SPI master, which means >> it controls the SPI clock, so the driver must support SPI slave >> mode. However, the Linux Kernel does not currently support SPI >> slave mode. We have three choices to make this work and they >> are all a lot of work: 1) Add support for SPI Slave mode to the >> kernel, 2) Use hardware to convert SPI signals to I2S signals >> and with the use of a custom codec, use the ALSA framework to >> stream the samples (this is an approach I used, but I don’t like >> it), 3) Move the I2S driver out of the sound subsystem and use it >> together with DMA to stream samples directly into the ADE7854 driver >> (my preferred solutions). Perhaps Mark Brown has some ideas on how >>to make this work. > I'll be honest, this is an end of line part and frankly more than > a little crazy. I would go with simply not supporting the measurement > streaming at all for this part. If you really need it we can then > move onto the how part, but from what you have said I'm guessing you > don't care except in an abstract 'it would be nice' sort of a way? Yeah, I’m moving to the ADE9000 so I’ll drop this. > >> >> The ADE9000 will be much easier because it uses an SPI Slave interface. >> >> I hope I have captured everything, but let me know if I have missed anything. >> > > That will do for now ;) I'm sure there will be details that need > tidying up once we have the above done, but that's true for any new > driver (and this will be nearly a new driver before things are done). Thank you again for all the detailed feedback. > > Jonathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net-next,0/2] hv_netvsc: Fix/improve RX path error handling
From: Haiyang Zhang Date: Thu, 22 Mar 2018 12:01:12 -0700 > Fix the status code returned to the host. Also add range > check for rx packet offset and length. Series applied, thank you. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
