Re: [PATCH v2 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag

[Guenter Roeck]

On Tue, Feb 09, 2021 at 07:08:17PM +0800, Yicong Yang wrote:
> From: Junhao He 
> 
> We use ccflags-$(CONFIG_HWMON_DEBUG_CHIP) for the debug
> message in drivers/hwmon, but the DEBUG flag will not pass to
> the subdirectory.
> 
> Considering CONFIG_HWMON_DEBUG_CHIP intends to have DEBUG
> recursively in driver/hwmon. It will be clearer
> to use subdir-ccflags-* instead of ccflags-* to inherit
> the debug settings from Kconfig when traversing subdirectories,
> and it will avoid omittance of DEBUG define when debug messages
> added in the subdirectories.
> 

The above paragraph doesn't add clarity and may as well be dropped.
On the other side, the commit message still doesn't mention that
pr_debug depends on DEBUG, which I am sure many people don't know
or remember. This is the prime reason why this patch is acceptable,
so it most definitely needs to be mentioned here.

Guenter

> Suggested-by: Bjorn Helgaas 
> Signed-off-by: Junhao He 
> Signed-off-by: Yicong Yang 
> ---
>  drivers/hwmon/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 09a86c5..1c0c089 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -201,5 +201,5 @@ obj-$(CONFIG_SENSORS_XGENE)   += xgene-hwmon.o
>  obj-$(CONFIG_SENSORS_OCC)+= occ/
>  obj-$(CONFIG_PMBUS)  += pmbus/
>  
> -ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
> +subdir-ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
>  
> -- 
> 2.8.1
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag

[Guenter Roeck]

On 2/5/21 12:08 PM, Bjorn Helgaas wrote:
> On Fri, Feb 05, 2021 at 10:28:32AM -0800, Guenter Roeck wrote:
>> On Fri, Feb 05, 2021 at 05:44:13PM +0800, Yicong Yang wrote:
>>> From: Junhao He 
>>>
>>> Use subdir-ccflags-* instead of ccflags-* to inherit the debug
>>> settings from Kconfig when traversing subdirectories.
>>>
>>> Suggested-by: Bjorn Helgaas 
>>> Signed-off-by: Junhao He 
>>> Signed-off-by: Yicong Yang 
>>
>> What problem does this fix ? Maybe I am missing it, but I don't see
>> DEBUG being used in a subdirectory of drivers/hwmon.
> 
> It's my fault for raising this question [1].  Yicong fixed a real
> problem in drivers/pci, where we are currently using
> 
>   ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
> 
> so CONFIG_PCI_DEBUG=y turns on debug in drivers/pci, but not in the
> subdirectories.  That's surprising to users.
> 
> So my question was whether we should default to using subdir-ccflags
> for -DDEBUG in general, and only use ccflags when we have
> subdirectories that have their own debug options, e.g.,
> 
>   drivers/i2c/Makefile:ccflags-$(CONFIG_I2C_DEBUG_CORE) := -DDEBUG
>   drivers/i2c/algos/Makefile:ccflags-$(CONFIG_I2C_DEBUG_ALGO) := -DDEBUG
>   drivers/i2c/busses/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
>   drivers/i2c/muxes/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> 
> I mentioned drivers/hwmon along with a few others that have
> subdirectories, do not have per-subdirectory debug options, and use
> ccflags.  I didn't try to determine whether those subdirectories
> currently use -DDEBUG.
> 
> In the case of drivers/hwmon, several drivers do use pr_debug(),
> and CONFIG_HWMON_DEBUG_CHIP=y turns those on.  But if somebody
> were to add pr_debug() to drivers/hwmon/occ/common.c, for example,
> CONFIG_HWMON_DEBUG_CHIP=y would *not* turn it on.  That sounds
> surprising to me, but if that's what you intend, that's totally fine.
> 

That does make sense, but that explanation is missing from the
description.

Guenter

> [1] https://lore.kernel.org/r/20210204161048.GA68790@bjorn-Precision-5520
> 
>>> ---
>>>  drivers/hwmon/Makefile | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>>> index 09a86c5..1c0c089 100644
>>> --- a/drivers/hwmon/Makefile
>>> +++ b/drivers/hwmon/Makefile
>>> @@ -201,5 +201,5 @@ obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o
>>>  obj-$(CONFIG_SENSORS_OCC)  += occ/
>>>  obj-$(CONFIG_PMBUS)+= pmbus/
>>>  
>>> -ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
>>> +subdir-ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
>>>  
>>> -- 
>>> 2.8.1
>>>

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag

[Guenter Roeck]

On Fri, Feb 05, 2021 at 05:44:13PM +0800, Yicong Yang wrote:
> From: Junhao He 
> 
> Use subdir-ccflags-* instead of ccflags-* to inherit the debug
> settings from Kconfig when traversing subdirectories.
> 
> Suggested-by: Bjorn Helgaas 
> Signed-off-by: Junhao He 
> Signed-off-by: Yicong Yang 

What problem does this fix ? Maybe I am missing it, but I don't see
DEBUG being used in a subdirectory of drivers/hwmon.

Guenter

> ---
>  drivers/hwmon/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 09a86c5..1c0c089 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -201,5 +201,5 @@ obj-$(CONFIG_SENSORS_XGENE)   += xgene-hwmon.o
>  obj-$(CONFIG_SENSORS_OCC)+= occ/
>  obj-$(CONFIG_PMBUS)  += pmbus/
>  
> -ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
> +subdir-ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
>  
> -- 
> 2.8.1
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: octeon: delete driver

[Guenter Roeck]

On 2/5/20 1:03 AM, Geert Uytterhoeven wrote:

On Wed, Feb 5, 2020 at 4:57 AM Guenter Roeck  wrote:

On 2/4/20 7:34 PM, Dan Carpenter wrote:

On Tue, Feb 04, 2020 at 12:31:16PM -0800, Matthew Wilcox wrote:

On Tue, Feb 04, 2020 at 08:06:14PM +, Chris Packham wrote:

On Tue, 2020-02-04 at 07:09 +, gre...@linuxfoundation.org wrote:

On Tue, Feb 04, 2020 at 04:02:15AM +, Chris Packham wrote:

On Tue, 2020-02-04 at 10:21 +0300, Dan Carpenter wrote:

My advice is to delete all the COMPILE_TEST code.  That stuff was a
constant source of confusion and headaches.


I was also going to suggest this. Since the COMPILE_TEST has been a
source of trouble I was going to propose dropping the || COMPILE_TEST
from the Kconfig for the octeon drivers.


Not having it also causes problems.  I didn't originally add it for
shits and giggles.


I wonder if the kbuild bot does enough cross compile build testing these
days to detect compile problems.  It might have improved to the point
where COMPILE_TEST isn't required.


It depends...


Not really. Looking at the build failures in the mainline kernel right now:

Failed builds:
 alpha:allmodconfig
 arm:allmodconfig
 i386:allyesconfig
 i386:allmodconfig
 m68k:allmodconfig
 microblaze:mmu_defconfig
 mips:allmodconfig
 parisc:allmodconfig
 powerpc:allmodconfig
 s390:allmodconfig
 sparc64:allmodconfig


I did receive a report from nore...@ellerman.id.au for the m68k build
failure. But that was sent to me only, not to the offender, and I do my
own builds anyway.

More interesting, that report happened after the offending commit landed
upstream, while it had been in next for 4 weeks.



m68k in -next builds fine for me, and did for a while. I have not seen a build
failure there. There must be a context commit causing this failure, or what
is (or was) in -next differs from what is in mainline.


Many of those don't even _have_ specific configurations causing the build 
failures.


Exactly. These are the "easy" ones, as the all*config builds enable as
much infrastructure as possible.  It's much harder if some common
dependency is not fulfilled in some specific config.



Yes, that is correct. But that doesn't mean that it would be a good idea
to retire COMPILE_TEST.

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: octeon: delete driver

[Guenter Roeck]

On 2/4/20 7:34 PM, Dan Carpenter wrote:

On Tue, Feb 04, 2020 at 12:31:16PM -0800, Matthew Wilcox wrote:

On Tue, Feb 04, 2020 at 08:06:14PM +, Chris Packham wrote:

On Tue, 2020-02-04 at 07:09 +, gre...@linuxfoundation.org wrote:

On Tue, Feb 04, 2020 at 04:02:15AM +, Chris Packham wrote:

On Tue, 2020-02-04 at 10:21 +0300, Dan Carpenter wrote:

My advice is to delete all the COMPILE_TEST code.  That stuff was a
constant source of confusion and headaches.


I was also going to suggest this. Since the COMPILE_TEST has been a
source of trouble I was going to propose dropping the || COMPILE_TEST
from the Kconfig for the octeon drivers.


Not having it also causes problems.  I didn't originally add it for
shits and giggles.


I wonder if the kbuild bot does enough cross compile build testing these
days to detect compile problems.  It might have improved to the point
where COMPILE_TEST isn't required.



Not really. Looking at the build failures in the mainline kernel right now:

Failed builds:
alpha:allmodconfig
arm:allmodconfig
i386:allyesconfig
i386:allmodconfig
m68k:allmodconfig
microblaze:mmu_defconfig
mips:allmodconfig
parisc:allmodconfig
powerpc:allmodconfig
s390:allmodconfig
sparc64:allmodconfig

Many of those don't even _have_ specific configurations causing the build 
failures.

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [SPAM] Re: [PATCH 1/2] staging: octeon: delete driver

[Guenter Roeck]

On Tue, Feb 04, 2020 at 08:06:14PM +, Chris Packham wrote:
> On Tue, 2020-02-04 at 07:09 +, gre...@linuxfoundation.org wrote:
> > On Tue, Feb 04, 2020 at 04:02:15AM +, Chris Packham wrote:
> > > I'll pipe up on this thread too
> > > 
> > > On Tue, 2019-12-10 at 02:42 -0800, Guenter Roeck wrote:
> > > > On 12/10/19 1:15 AM, Greg Kroah-Hartman wrote:
> > > > > This driver has been in the tree since 2009 with no real movement to 
> > > > > get
> > > > > it out.  Now it is starting to cause build issues and other problems 
> > > > > for
> > > > > people who want to fix coding style problems, but can not actually 
> > > > > build
> > > > > it.
> > > > > 
> > > > > As nothing is happening here, just delete the module entirely.
> > > > > 
> > > > > Reported-by: Guenter Roeck 
> > > > > Cc: David Daney 
> > > > > Cc: "David S. Miller" 
> > > > > Cc: "Matthew Wilcox (Oracle)" 
> > > > > Cc: Guenter Roeck 
> > > > > Cc: YueHaibing 
> > > > > Cc: Aaro Koskinen 
> > > > > Cc: Wambui Karuga 
> > > > > Cc: Julia Lawall 
> > > > > Cc: Florian Westphal 
> > > > > Cc: Geert Uytterhoeven 
> > > > > Cc: Branden Bonaby 
> > > > > Cc: "Petr Štetiar" 
> > > > > Cc: Sandro Volery 
> > > > > Cc: Paul Burton 
> > > > > Cc: Dan Carpenter 
> > > > > Cc: Giovanni Gherdovich 
> > > > > Cc: Valery Ivanov 
> > > > > Signed-off-by: Greg Kroah-Hartman 
> > > > 
> > > > Acked-by: Guenter Roeck 
> > > 
> > > Please can we keep this driver. We do have platforms using it and we
> > > would like it to stay around.
> > > 
> > > Clearly we'll need to sort things out to a point where they build
> > > successfully. We've been hoping to see this move out of staging ever
> > > since we selected Cavium as a vendor.
> > 
> > Great, can you send me a patchset that reverts this and fixes the build
> > issues and accept maintainership of the code?
> > 
> 
> Yep will do.
> 
> On Tue, 2020-02-04 at 10:21 +0300, Dan Carpenter wrote:
> > My advice is to delete all the COMPILE_TEST code.  That stuff was a
> > constant source of confusion and headaches.
> 
> I was also going to suggest this. Since the COMPILE_TEST has been a
> source of trouble I was going to propose dropping the || COMPILE_TEST
> from the Kconfig for the octeon drivers.

It isn't just the Kconfig file, there is also a lot of actual _code_
that depends on COMPILE_TEST.

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: octeon: delete driver

[Guenter Roeck]

On Tue, Dec 10, 2019 at 11:48:49PM +0200, Aaro Koskinen wrote:
> On Tue, Dec 10, 2019 at 12:15:15PM -0800, Guenter Roeck wrote:
> > On Tue, Dec 10, 2019 at 09:46:59PM +0200, Aaro Koskinen wrote:
> > > On Tue, Dec 10, 2019 at 01:01:20PM +0100, Greg Kroah-Hartman wrote:
> > > > I have no idea :(
> > > 
> > > It's stated in the TODO file you are deleting (visible in your
> > > patch): "This driver is functional and supports Ethernet on
> > > OCTEON+/OCTEON2/OCTEON3 chips at least up to CN7030."
> > > 
> > > This includes e.g. some D-Link routers and Uniquiti EdgeRouters. You
> > > can check from /proc/cpuinfo if you are running on this MIPS SoC.
> > 
> > It also results in "mips:allmodconfig" build failures in mainline
> > and is for that reason being marked as BROKEN. Unfortunately,
> > misguided attempts to clean it up had the opposite effect.
> 
> This was because of stubs hack added by someone - people who do not run
> or care about the hardware can now break it for others with their
> silly x86 "compile test"s.
> 

Thast was the first breakage. The second was to replace typedefs with
structures without considering that those typedefs are still used
throughout the Cavium code, creating conflicts between "mystruct_t" and
"struct mystruct" in various API calls. It may well be that this
"improvement" was tested with x86_64:allmodconfig - if it was tested
in the first place. It was most definitely not tested with
cavium_octeon_defconfig, much less with real hardware.

Pretty much none of the changes made to the driver in the recent
past have improved it. On the contrary, it is getting worse. With no
one committed to get the driver out of staging, I don't think there
is a reasonable alternative to removing it. For my part I am for sure
not looking forward having to deal with it breaking over and over
again and having to spend time tracking down the breakage.

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: octeon: delete driver

[Guenter Roeck]

On Tue, Dec 10, 2019 at 09:46:59PM +0200, Aaro Koskinen wrote:
> On Tue, Dec 10, 2019 at 01:01:20PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Dec 10, 2019 at 12:40:54PM +0100, Sandro Volery wrote:
> > > Doesn't octeon have drivers out of staging already?
> > > What is this module for?
> > 
> > I have no idea :(
> 
> It's stated in the TODO file you are deleting (visible in your
> patch): "This driver is functional and supports Ethernet on
> OCTEON+/OCTEON2/OCTEON3 chips at least up to CN7030."
> 
> This includes e.g. some D-Link routers and Uniquiti EdgeRouters. You
> can check from /proc/cpuinfo if you are running on this MIPS SoC.
> 

It also results in "mips:allmodconfig" build failures in mainline
and is for that reason being marked as BROKEN. Unfortunately,
misguided attempts to clean it up had the opposite effect.

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: octeon: delete driver

[Guenter Roeck]

On 12/10/19 1:15 AM, Greg Kroah-Hartman wrote:

This driver has been in the tree since 2009 with no real movement to get
it out.  Now it is starting to cause build issues and other problems for
people who want to fix coding style problems, but can not actually build
it.

As nothing is happening here, just delete the module entirely.

Reported-by: Guenter Roeck 
Cc: David Daney 
Cc: "David S. Miller" 
Cc: "Matthew Wilcox (Oracle)" 
Cc: Guenter Roeck 
Cc: YueHaibing 
Cc: Aaro Koskinen 
Cc: Wambui Karuga 
Cc: Julia Lawall 
Cc: Florian Westphal 
Cc: Geert Uytterhoeven 
Cc: Branden Bonaby 
Cc: "Petr Štetiar" 
Cc: Sandro Volery 
Cc: Paul Burton 
Cc: Dan Carpenter 
Cc: Giovanni Gherdovich 
Cc: Valery Ivanov 
Signed-off-by: Greg Kroah-Hartman 


Acked-by: Guenter Roeck 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging/octeon: Mark Ethernet driver as BROKEN

[Guenter Roeck]

On Mon, Dec 02, 2019 at 05:52:31PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Dec 02, 2019 at 06:18:36AM -0800, Guenter Roeck wrote:
> > The code doesn't compile due to incompatible pointer errors such as
> > 
> > drivers/staging/octeon/ethernet-tx.c:649:50: error:
> > passing argument 1 of 'cvmx_wqe_get_grp' from incompatible pointer type
> > 
> > This is due to mixing, for example, cvmx_wqe_t with 'struct cvmx_wqe'.
> > 
> > Unfortunately, one can not just revert the primary offending commit, as 
> > doing so
> > results in secondary errors. This is made worse by the fact that the 
> > "removed"
> > typedefs still exist and are used widely outside the staging directory,
> > making the entire set of "remove typedef" changes pointless and wrong.
> 
> Ugh, sorry about that.
> 
> > Reflect reality and mark the driver as BROKEN.
> 
> Should I just delete this thing?  No one seems to be using it and there
> is no move to get it out of staging at all.
> 
> Will anyone actually miss it?  It can always come back of someone
> does...
> 

All it does is causing trouble and misguided attempts to clean it up.
If anything, the whole thing goes into the wrong direction (declare a
complete set of dummy functions just to be able to build the driver
with COMPILE_TEST ? Seriously ?).

I second the motion to drop it. This has been in staging for 10 years.
Don't we have some kind of time limit for code in staging ? If not,
should we ? If anyone really needs it, that person or group should
really invest the time to get it out of staging for good.

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging/octeon: Mark Ethernet driver as BROKEN

[Guenter Roeck]

The code doesn't compile due to incompatible pointer errors such as

drivers/staging/octeon/ethernet-tx.c:649:50: error:
passing argument 1 of 'cvmx_wqe_get_grp' from incompatible pointer type

This is due to mixing, for example, cvmx_wqe_t with 'struct cvmx_wqe'.

Unfortunately, one can not just revert the primary offending commit, as doing so
results in secondary errors. This is made worse by the fact that the "removed"
typedefs still exist and are used widely outside the staging directory,
making the entire set of "remove typedef" changes pointless and wrong.

Reflect reality and mark the driver as BROKEN.

Fixes: ef1fe6b7369a ("staging: octeon: remove typedef declaration for cvmx_wqe")
Fixes: 73aef0c9d2c6 ("staging: octeon: remove typedef declaration for 
cvmx_helper_link_info")
Cc: Wambui Karuga 
Cc: Julia Lawall 
Signed-off-by: Guenter Roeck 
---
 drivers/staging/octeon/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/octeon/Kconfig b/drivers/staging/octeon/Kconfig
index 5319909eb2f6..e7f4ddcc1361 100644
--- a/drivers/staging/octeon/Kconfig
+++ b/drivers/staging/octeon/Kconfig
@@ -3,6 +3,7 @@ config OCTEON_ETHERNET
tristate "Cavium Networks Octeon Ethernet support"
depends on CAVIUM_OCTEON_SOC || COMPILE_TEST
depends on NETDEVICES
+   depends on BROKEN
select PHYLIB
select MDIO_OCTEON
help
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] iio: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver

[Guenter Roeck]

On Wed, Nov 13, 2019 at 08:10:50AM +, Hennerich, Michael wrote:
> 
> 
> > -Original Message-
> > From: Guenter Roeck  On Behalf Of Guenter Roeck
> > Sent: Dienstag, 12. November 2019 20:18
> > To: Jonathan Cameron 
> > Cc: Bia, Beniamin ; ji...@kernel.org;
> > l...@metafoo.de; Hennerich, Michael ;
> > pme...@pmeerw.net; gre...@linuxfoundation.org; linux-
> > i...@vger.kernel.org; de...@driverdev.osuosl.org; linux-
> > ker...@vger.kernel.org; mark.rutl...@arm.com; robh...@kernel.org;
> > devicet...@vger.kernel.org; paul...@linux.ibm.com;
> > mchehab+sams...@kernel.org; linus.wall...@linaro.org;
> > nicolas.fe...@microchip.com; biabenia...@outlook.com; Jean Delvare
> > 
> > Subject: Re: [PATCH 1/3] iio: Add ADM1177 Hot Swap Controller and Digital
> > Power Monitor driver
> > 
> > On Tue, Nov 12, 2019 at 05:37:57PM +, Jonathan Cameron wrote:
> > > On Tue, 12 Nov 2019 17:35:50 +0200
> > > Beniamin Bia  wrote:
> > >
> > > > From: Michael Hennerich 
> > > >
> > > > ADM1177 is a Hot Swap Controller and Digital Power Monitor with Soft
> > > > Start Pin.
> > > >
> > > > Datasheet:
> > > > Link:
> > > > https://www.analog.com/media/en/technical-documentation/data-
> > sheets/
> > > > ADM1177.pdf
> > > >
> > > > Signed-off-by: Michael Hennerich 
> > > > Co-developed-by: Beniamin Bia 
> > > > Signed-off-by: Beniamin Bia 
> > >
> > > Hi Beniamin,
> > >
> > > A couple immediate thoughts.
> > >
> > > 1. That cc list has some rather non obvious people on it.  Unless 
> > > something
> > >fairly surprising is going on, probably better to cut it back a bit.
> > >
> > > 2. Why IIO?  Not entirely obvious to me.  From first glance this is 
> > > definitely
> > >doing hardware monitoring.  If there is a reason there should be a 
> > > clear
> > >statement here on why.
> > >
> > 
> > I don't see why this is implemented as iio driver. I think it should be
> > implemented as hardware monitoring driver.
> 
> Totally agree that this driver could have been implemented as HWMON driver.
> Well we use this device as USB supply monitor on our embedded portably kits, 
> to detect low VBUS or excess current draw. 
> 
> ADALM-PLUTO and ADALM2000:
> https://www.analog.com/en/design-center/evaluation-hardware-and-software/evaluation-boards-kits/adalm-pluto.html
> 
> https://www.analog.com/en/design-center/evaluation-hardware-and-software/evaluation-boards-kits/ADALM2000.html
> 
> The only connectivity to the host PC is via IIO/libiio USB Gadget FS and 
> Ethernet backends.
> 
> We recommend people to read the IIO attributes whenever they report an issue.
> Unless libiio supports directly HWMON or HWMON adds an IIO bridge we would 
> prefer this driver being exposed as IIO device, since HWMON users still can 
> use the IIO/HWMON bridge.
> 

I do not think this is a valid argument.

- This is a hardware monitoring chip.
- Implementing kernel support as IIO driver, keeping it out of tree for years,
  establishing an iio based use case, and then pressuring kernel maintainers
  to accept an iio driver seems inappropriate.
- The argument of "we need libiio support for this chip" could effectively
  be used to re-implement pretty much all hwmon drivers as iio drivers.
- The iio->hwmon bridge would add complexity for the majority of potential
  users of this chip. Focus should be on the majority, not on one use case.
- Userspace may as well use libsensors and/or sensors to do the necessary
  access - or implement it if neded. Or add a libsensors based backend to
  libiio (or to iiod).
- Last but not least, it would be more appropriate to implement a generic
  hwmon->iio bridge for iio use cases for chips supported by hwmon drivers,
  similar to the hwmon->thermal bridge.

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] iio: Add ADM1177 Hot Swap Controller and Digital Power Monitor driver

[Guenter Roeck]

On Tue, Nov 12, 2019 at 05:37:57PM +, Jonathan Cameron wrote:
> On Tue, 12 Nov 2019 17:35:50 +0200
> Beniamin Bia  wrote:
> 
> > From: Michael Hennerich 
> > 
> > ADM1177 is a Hot Swap Controller and Digital Power Monitor with
> > Soft Start Pin.
> > 
> > Datasheet:
> > Link: 
> > https://www.analog.com/media/en/technical-documentation/data-sheets/ADM1177.pdf
> > 
> > Signed-off-by: Michael Hennerich 
> > Co-developed-by: Beniamin Bia 
> > Signed-off-by: Beniamin Bia 
> 
> Hi Beniamin,
> 
> A couple immediate thoughts.
> 
> 1. That cc list has some rather non obvious people on it.  Unless something
>fairly surprising is going on, probably better to cut it back a bit.
> 
> 2. Why IIO?  Not entirely obvious to me.  From first glance this is definitely
>doing hardware monitoring.  If there is a reason there should be a clear
>statement here on why.
> 

I don't see why this is implemented as iio driver. I think it should be 
implemented
as hardware monitoring driver.

Some more comments below.

Guenter

> +CC Guenter and Jean as hwmon maintainers.
> 
> Whilst I'm here, a very quick review below.
> 
> > ---
> >  drivers/iio/adc/Kconfig   |  12 ++
> >  drivers/iio/adc/Makefile  |   1 +
> >  drivers/iio/adc/adm1177.c | 257 ++
> >  3 files changed, 270 insertions(+)
> >  create mode 100644 drivers/iio/adc/adm1177.c
> > 
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 9554890a3200..4db8c6dcb595 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -228,6 +228,18 @@ config ASPEED_ADC
> >   To compile this driver as a module, choose M here: the module will be
> >   called aspeed_adc.
> >  
> > +config ADM1177
> > +   tristate "Analog Devices ADM1177 Digital Power Monitor driver"
> > +   depends on I2C
> > +   help
> > + Say yes here to build support for Analog Devices:
> > + ADM1177 Hot Swap Controller and Digital Power Monitor
> > + with Soft Start Pin. Provides direct access
> > + via sysfs.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called adm1177.
> > +
> >  config AT91_ADC
> > tristate "Atmel AT91 ADC"
> > depends on ARCH_AT91
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index 5ecc481c2967..7899d6a158f3 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -24,6 +24,7 @@ obj-$(CONFIG_AD7887) += ad7887.o
> >  obj-$(CONFIG_AD7949) += ad7949.o
> >  obj-$(CONFIG_AD799X) += ad799x.o
> >  obj-$(CONFIG_ASPEED_ADC) += aspeed_adc.o
> > +obj-$(CONFIG_ADM1177) += adm1177.o
> >  obj-$(CONFIG_AT91_ADC) += at91_adc.o
> >  obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
> >  obj-$(CONFIG_AXP20X_ADC) += axp20x_adc.o
> > diff --git a/drivers/iio/adc/adm1177.c b/drivers/iio/adc/adm1177.c
> > new file mode 100644
> > index ..daec34566a65
> > --- /dev/null
> > +++ b/drivers/iio/adc/adm1177.c
> > @@ -0,0 +1,257 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * ADM1177 Hot Swap Controller and Digital Power Monitor with Soft Start 
> > Pin
> > + *
> > + * Copyright 2015-2019 Analog Devices Inc.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/*  Command Byte Operations */
> > +#define ADM1177_CMD_V_CONT BIT(0)
> > +#define ADM1177_CMD_V_ONCE BIT(1)
> > +#define ADM1177_CMD_I_CONT BIT(2)
> > +#define ADM1177_CMD_I_ONCE BIT(3)
> > +#define ADM1177_CMD_VRANGE BIT(4)
> > +#define ADM1177_CMD_STATUS_RD  BIT(6)
> > +
> > +/* Extended Register */
> > +#define ADM1177_REG_ALERT_EN   1
> > +#define ADM1177_REG_ALERT_TH   2
> > +#define ADM1177_REG_CONTROL3
> > +
> > +/* ADM1177_REG_ALERT_EN */
> > +#define ADM1177_EN_ADC_OC1 BIT(0)
> > +#define ADM1177_EN_ADC_OC4 BIT(1)
> > +#define ADM1177_EN_HS_ALERTBIT(2)
> > +#define ADM1177_EN_OFF_ALERT   BIT(3)
> > +#define ADM1177_CLEAR  BIT(4)
> > +
> > +/* ADM1177_REG_CONTROL */
> > +#define ADM1177_SWOFF  BIT(0)
> > +
> > +#define ADM1177_BITS   12
> > +
> > +/**
> > + * struct adm1177_state - driver instance specific data
> > + * @client pointer to i2c client
> > + * @regregulator info for the the power supply of the 
> > device
> > + * @commandinternal control register
> > + * @r_sense_uohm   current sense resistor value
> > + * @alert_threshold_ua current limit for shutdown
> > + * @vrange_highinternal voltage divider
> > + */
> > +struct adm1177_state {
> > +   struct i2c_client   *client;
> > +   struct regulator*reg;
> > +   u8  command;
> > +   u32 r_sense_uohm;
> > +   u32 alert_threshold_ua;
> > +   boolvrange_high;
> > +};
> > +
> > 

[PATCH] staging/octeon: Fix test build on MIPS

[Guenter Roeck]

mips:allmodconfig fails to build.

drivers/staging/octeon/ethernet-rx.c: In function 'cvm_oct_poll':
drivers/staging/octeon/ethernet-defines.h:30:38: error:
'CONFIG_CAVIUM_OCTEON_CVMSEG_SIZE' undeclared

Octeon defines are only available if CONFIG_CPU_CAVIUM_OCTEON
is enabled. Since the driver uses those defines, we have to use
the dummy defines if this flag is not enabled.

Cc: Matthew Wilcox (Oracle) 
Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS")
Signed-off-by: Guenter Roeck 
---
 drivers/staging/octeon/octeon-ethernet.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/octeon/octeon-ethernet.h 
b/drivers/staging/octeon/octeon-ethernet.h
index a8a864b40913..70848c6c86ec 100644
--- a/drivers/staging/octeon/octeon-ethernet.h
+++ b/drivers/staging/octeon/octeon-ethernet.h
@@ -14,7 +14,7 @@
 #include 
 #include 
 
-#ifdef CONFIG_MIPS
+#ifdef CONFIG_CPU_CAVIUM_OCTEON
 
 #include 
 
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: kpc2000: Fix build failure caused by wrong include file

[Guenter Roeck]

xtensa:allmodconfig fails to build.

arch/xtensa/include/asm/uaccess.h: In function 'clear_user':
arch/xtensa/include/asm/uaccess.h:40:22: error:
implicit declaration of function 'uaccess_kernel'

uaccess_kernel() is declared in linux/uaccess.h, not asm/uaccess.h.

Fixes: 7df95299b94a ("staging: kpc2000: Add DMA driver")
Cc: Matt Sickler 
Signed-off-by: Guenter Roeck 
---
 drivers/staging/kpc2000/kpc_dma/fileops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 5741d2b49a7d..e741fa753ca1 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -8,7 +8,7 @@
 #include /* error codes */
 #include /* size_t */
 #include 
-#include /* copy_*_user */
+#include /* copy_*_user */
 #include   /* aio stuff */
 #include 
 #include 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next] staging: octeon: fix build failure with XFRM enabled

[Guenter Roeck]

On Fri, Dec 21, 2018 at 09:57:26PM +0100, Florian Westphal wrote:
> skb->sp doesn't exist anymore in the next-next tree, so mips defconfig
> no longer builds.  Use helper instead to reset the secpath.
> 
> Not even compile tested.
> 
It does fix the build error.

Tested-by: Guenter Roeck 

> Cc: Greg Kroah-Hartman 
> Reported-by: Guenter Roeck 
> Fixes: 4165079ba328d ("net: switch secpath to use skb extension 
> infrastructure")
> Signed-off-by: Florian Westphal 
> ---
>  Greg, David:
> 
>  The patch will not break build for a tree that lacks the 'Fixes'
>  commit, so this can also go in via staging tree.
>  OTOH, net-next build is broken for mips/octeon, so I think in
>  this case net-next might make more sense?
> 
> diff --git a/drivers/staging/octeon/ethernet-tx.c 
> b/drivers/staging/octeon/ethernet-tx.c
> index df3441b815bb..317c9720467c 100644
> --- a/drivers/staging/octeon/ethernet-tx.c
> +++ b/drivers/staging/octeon/ethernet-tx.c
> @@ -359,8 +359,7 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device 
> *dev)
>   dst_release(skb_dst(skb));
>   skb_dst_set(skb, NULL);
>  #ifdef CONFIG_XFRM
> - secpath_put(skb->sp);
> - skb->sp = NULL;
> + secpath_reset(skb);
>  #endif
>   nf_reset(skb);
>  
> -- 
> 2.19.2
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] android: binder_alloc: Include asm/cacheflush.h after linux/ include files

[Guenter Roeck]

If asm/cacheflush.h is included first, the following build warnings are
seen with sparc32 builds.

In file included from ./arch/sparc/include/asm/cacheflush.h:11:0,
from drivers/android/binder_alloc.c:20:
./arch/sparc/include/asm/cacheflush_32.h:40:37: warning:
'struct page' declared inside parameter list

Moving the asm/ include after linux/ includes fixes the problem.

Suggested-by: Linus Torvalds 
Signed-off-by: Guenter Roeck 
---
 drivers/android/binder_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 2628806c64a2..2c258dcf9d72 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -17,7 +17,6 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include 
 #include 
 #include 
 #include 
@@ -28,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "binder_alloc.h"
 #include "binder_trace.h"
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] android: binder: Include asm/cacheflush.h after linux/ include files

[Guenter Roeck]

If asm/cacheflush.h is included first, the following build warnings are
seen with sparc32 builds.

In file included from arch/sparc/include/asm/cacheflush.h:11:0,
from drivers/android/binder.c:54:
arch/sparc/include/asm/cacheflush_32.h:40:37: warning:
'struct page' declared inside parameter list will not be visible
outside of this definition or declaration

Moving the asm/ include after linux/ includes solves the problem.

Suggested-by: Linus Torvalds 
Signed-off-by: Guenter Roeck 
---
 drivers/android/binder.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 95283f3bb51c..1cc2fa16af8b 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -51,7 +51,6 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include 
 #include 
 #include 
 #include 
@@ -73,6 +72,9 @@
 #include 
 
 #include 
+
+#include 
+
 #include "binder_alloc.h"
 #include "binder_trace.h"
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] media: staging: omap4iss: Include asm/cacheflush.h after generic includes

[Guenter Roeck]

Including asm/cacheflush.h first results in the following build error when
trying to build sparc32:allmodconfig.

In file included from arch/sparc/include/asm/page.h:10:0,
 from arch/sparc/include/asm/string_32.h:13,
 from arch/sparc/include/asm/string.h:7,
 from include/linux/string.h:20,
 from include/linux/bitmap.h:9,
 from include/linux/cpumask.h:12,
 from arch/sparc/include/asm/smp_32.h:15,
 from arch/sparc/include/asm/smp.h:7,
 from arch/sparc/include/asm/switch_to_32.h:5,
 from arch/sparc/include/asm/switch_to.h:7,
 from arch/sparc/include/asm/ptrace.h:120,
 from arch/sparc/include/asm/thread_info_32.h:19,
 from arch/sparc/include/asm/thread_info.h:7,
 from include/linux/thread_info.h:38,
 from arch/sparc/include/asm/current.h:15,
 from include/linux/mutex.h:14,
 from include/linux/notifier.h:14,
 from include/linux/clk.h:17,
 from drivers/staging/media/omap4iss/iss_video.c:15:
include/linux/highmem.h: In function 'clear_user_highpage':
include/linux/highmem.h:137:31: error:
passing argument 1 of 'sparc_flush_page_to_ram' from incompatible
pointer type

Include generic includes files first to fix the problem.

Fixes: fc96d58c10162 ("[media] v4l: omap4iss: Add support for OMAP4 camera 
interface - Video devices")
Suggested-by: Linus Torvalds 
Cc: David Miller 
Cc: Randy Dunlap 
Signed-off-by: Guenter Roeck 
---
 drivers/staging/media/omap4iss/iss_video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c 
b/drivers/staging/media/omap4iss/iss_video.c
index a3a83424a926..16478fe9e3f8 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -11,7 +11,6 @@
  * (at your option) any later version.
  */
 
-#include 
 #include 
 #include 
 #include 
@@ -24,6 +23,8 @@
 #include 
 #include 
 
+#include 
+
 #include "iss_video.h"
 #include "iss.h"
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: speakup: Replace strncpy with memcpy

[Guenter Roeck]

gcc 8.1.0 generates the following warnings.

drivers/staging/speakup/kobjects.c: In function 'punc_store':
drivers/staging/speakup/kobjects.c:522:2: warning:
'strncpy' output truncated before terminating nul
copying as many bytes from a string as its length
drivers/staging/speakup/kobjects.c:504:6: note: length computed here

drivers/staging/speakup/kobjects.c: In function 'synth_store':
drivers/staging/speakup/kobjects.c:391:2: warning:
'strncpy' output truncated before terminating nul
copying as many bytes from a string as its length
drivers/staging/speakup/kobjects.c:388:8: note: length computed here

Using strncpy() is indeed less than perfect since the length of data to
be copied has already been determined with strlen(). Replace strncpy()
with memcpy() to address the warning and optimize the code a little.

Signed-off-by: Guenter Roeck 
---
 drivers/staging/speakup/kobjects.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/speakup/kobjects.c 
b/drivers/staging/speakup/kobjects.c
index f1f90222186b..08f11cc17371 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -388,7 +388,7 @@ static ssize_t synth_store(struct kobject *kobj, struct 
kobj_attribute *attr,
len = strlen(buf);
if (len < 2 || len > 9)
return -EINVAL;
-   strncpy(new_synth_name, buf, len);
+   memcpy(new_synth_name, buf, len);
if (new_synth_name[len - 1] == '\n')
len--;
new_synth_name[len] = '\0';
@@ -519,7 +519,7 @@ static ssize_t punc_store(struct kobject *kobj, struct 
kobj_attribute *attr,
return -EINVAL;
}
 
-   strncpy(punc_buf, buf, x);
+   memcpy(punc_buf, buf, x);
 
while (x && punc_buf[x - 1] == '\n')
x--;
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 0/3] hwmon: (nct7904) Fix style issues

[Guenter Roeck]

On Wed, Jun 13, 2018 at 05:13:22PM +0200, Jakob Albert wrote:
> This set of patches fixes style errors reported by checkpatch.pl. This
> adapts the code to the coding style.
> 
> Changes since v1:
> * Changed patch subjects and descriptions
> 
> 
> 
> Jakob Albert (3):
>   hwmon: (nct7904) Fix SPACING errors
>   hwmon: (nct7904) Fix CODE_INDENT error
>   hwmon: (nct7904) Fix UNSPECIFIED_INT warning
> 

Series applied to hwmon-next.

Thanks,
Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/3] staging: hwmon: Fix SPACING errors

[Guenter Roeck]

On Tue, Jun 12, 2018 at 08:56:58PM +0200, Jakob Albert wrote:
> Fix SPACING errors in drivers/hwmon/nct7904.c reported by checkpatch.pl
> 

Subject for all patches in this series should be

hwmon: (nct7904) 

There is no need to reference the file name in the patch description.
It is obvious from both Subject and the patch itself.

linux-hw...@vger.kernel.org needs to be in Cc:.

As Greg already mentioned, this series has nothing to do with staging.

Thanks,
Guenter

> Signed-off-by: Lorenz Kaestle 
> Signed-off-by: Jakob Albert 
> ---
>  drivers/hwmon/nct7904.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/nct7904.c b/drivers/hwmon/nct7904.c
> index 95a68ab..7de2421 100644
> --- a/drivers/hwmon/nct7904.c
> +++ b/drivers/hwmon/nct7904.c
> @@ -159,7 +159,7 @@ static int nct7904_read_fan(struct device *dev, u32 attr, 
> int channel,
>   unsigned int cnt, rpm;
>   int ret;
>  
> - switch(attr) {
> + switch (attr) {
>   case hwmon_fan_input:
>   ret = nct7904_read_reg16(data, BANK_0,
>FANIN1_HV_REG + channel * 2);
> @@ -200,7 +200,7 @@ static int nct7904_read_in(struct device *dev, u32 attr, 
> int channel,
>  
>   index = nct7904_chan_to_index[channel];
>  
> - switch(attr) {
> + switch (attr) {
>   case hwmon_in_input:
>   ret = nct7904_read_reg16(data, BANK_0,
>VSEN1_HV_REG + index * 2);
> @@ -236,7 +236,7 @@ static int nct7904_read_temp(struct device *dev, u32 
> attr, int channel,
>   struct nct7904_data *data = dev_get_drvdata(dev);
>   int ret, temp;
>  
> - switch(attr) {
> + switch (attr) {
>   case hwmon_temp_input:
>   if (channel == 0)
>   ret = nct7904_read_reg16(data, BANK_0, LTD_HV_REG);
> @@ -276,7 +276,7 @@ static int nct7904_read_pwm(struct device *dev, u32 attr, 
> int channel,
>   struct nct7904_data *data = dev_get_drvdata(dev);
>   int ret;
>  
> - switch(attr) {
> + switch (attr) {
>   case hwmon_pwm_input:
>   ret = nct7904_read_reg(data, BANK_3, FANCTL1_OUT_REG + channel);
>   if (ret < 0)
> @@ -301,7 +301,7 @@ static int nct7904_write_pwm(struct device *dev, u32 
> attr, int channel,
>   struct nct7904_data *data = dev_get_drvdata(dev);
>   int ret;
>  
> - switch(attr) {
> + switch (attr) {
>   case hwmon_pwm_input:
>   if (val < 0 || val > 255)
>   return -EINVAL;
> @@ -322,7 +322,7 @@ static int nct7904_write_pwm(struct device *dev, u32 
> attr, int channel,
>  
>  static umode_t nct7904_pwm_is_visible(const void *_data, u32 attr, int 
> channel)
>  {
> - switch(attr) {
> + switch (attr) {
>   case hwmon_pwm_input:
>   case hwmon_pwm_enable:
>   return S_IRUGO | S_IWUSR;
> -- 
> 2.7.4
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 11/13] staging: typec: tcpci: keep the not connecting cc line open

[Guenter Roeck]

On 03/28/2018 09:06 AM, Li Jun wrote:

While set polarity, we should keep the not connecting cc line to be
open.



The more I look at this code, the more I am confused by it.

The original code doesn't touch the CC lines. This function only sets the 
polarity.
Is it really appropriate to touch the CC lines in the same function ?

Guenter


Signed-off-by: Li Jun 
---
  drivers/staging/typec/tcpci.c | 18 ++
  1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index d5b4e4e..b58bd59 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -185,15 +185,25 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
  enum typec_cc_polarity polarity)
  {
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+   unsigned int reg;
int ret;
  
-	ret = regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,

-  (polarity == TYPEC_POLARITY_CC2) ?
-  TCPC_TCPC_CTRL_ORIENTATION : 0);
+   /* Keep the disconnect cc line open */
+   ret = regmap_read(tcpci->regmap, TCPC_ROLE_CTRL, );
if (ret < 0)
return ret;
  
-	return 0;

+   if (polarity == TYPEC_POLARITY_CC2)
+   reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT;
+   else
+   reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT;
+   ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+   if (ret < 0)
+   return ret;
+
+   return regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
+  (polarity == TYPEC_POLARITY_CC2) ?
+  TCPC_TCPC_CTRL_ORIENTATION : 0);
  }
  
  static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)




___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 10/13] usb: typec: tcpm: set cc for drp toggling attach

[Guenter Roeck]

On Thu, Mar 29, 2018 at 12:06:15AM +0800, Li Jun wrote:
> In case of drp toggling, we may need set correct cc value for role control
> after attach as it may never been set.
> 

Isn't CC set by the lower level driver in this case ? In other words, is it ever
necessary to call back into the low level driver to set CC again ? Doing that in
attached state seems a bit late.

It may make more sense to update port->cc_req when the state machine leaves
DRP_TOGGLING state, ie in _tcpm_cc_change(), and to do it without callback
into the low level driver (it should not be necessary).

Guenter

> Signed-off-by: Li Jun 
> ---
>  drivers/usb/typec/tcpm.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 218c230..72d4232 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -2126,6 +2126,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
>   tcpm_set_attached_state(port, false);
>   port->try_src_count = 0;
>   port->try_snk_count = 0;
> + port->cc_req = 0;
>  }
>  
>  static void tcpm_detach(struct tcpm_port *port)
> @@ -2361,6 +2362,8 @@ static void run_state_machine(struct tcpm_port *port)
>   break;
>  
>   case SRC_ATTACHED:
> + if (!port->cc_req)
> + tcpm_set_cc(port, tcpm_rp_cc(port));
>   ret = tcpm_src_attach(port);
>   tcpm_set_state(port, SRC_UNATTACHED,
>  ret < 0 ? 0 : PD_T_PS_SOURCE_ON);
> @@ -2531,6 +2534,8 @@ static void run_state_machine(struct tcpm_port *port)
>   tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
>   break;
>   case SNK_ATTACHED:
> + if (!port->cc_req)
> + tcpm_set_cc(port, TYPEC_CC_RD);
>   ret = tcpm_snk_attach(port);
>   if (ret < 0)
>   tcpm_set_state(port, SNK_UNATTACHED, 0);
> -- 
> 2.7.4
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] typec: tcpm: fusb302: Do not log an error on -EPROBE_DEFER

[Guenter Roeck]

On 02/25/2018 07:20 AM, Hans de Goede wrote:

Do not log an error if tcpm_register_port() fails with -EPROBE_DEFER.

Fixes: commit cf140a356971 ("typec: fusb302: Use dev_err during probe")
Signed-off-by: Hans de Goede <hdego...@redhat.com>


Reviewed-by: Guenter Roeck <li...@roeck-us.net>


---
  drivers/usb/typec/fusb302/fusb302.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
index b267b907bf24..a7b06053a538 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1856,7 +1856,8 @@ static int fusb302_probe(struct i2c_client *client,
chip->tcpm_port = tcpm_register_port(>dev, >tcpc_dev);
if (IS_ERR(chip->tcpm_port)) {
ret = PTR_ERR(chip->tcpm_port);
-   dev_err(dev, "cannot register tcpm port, ret=%d", ret);
+   if (ret != -EPROBE_DEFER)
+   dev_err(dev, "cannot register tcpm port, ret=%d", ret);
goto destroy_workqueue;
}
  



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[Guenter Roeck]

On 12/23/2017 05:48 AM, Greg KH wrote:

On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:

Hi all,

When I tried to use devm_ioremap function and review related code, I found
devm_ioremap and devm_ioremap_nocache is almost the same with each other,
except one use ioremap while the other use ioremap_nocache.


For all arches?  Really?  Look at MIPS, and x86, they have different
functions.



Both mips and x86 end up mapping the same function, but other arches don't.
mn10300 is one where ioremap and ioremap_nocache are definitely different.

Guenter


While ioremap's
default function is ioremap_nocache, so devm_ioremap_nocache also have the
same function with devm_ioremap, which can just be killed to reduce the size
of devres.o(from 20304 bytes to 18992 bytes in my compile environment).

I have posted two versions, which use macro instead of function for
devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
devm_ioremap_nocache for no need to keep a macro around for the duplicate
thing. So here comes v3 and please help to review.


I don't think this can be done, what am I missing?  These functions are
not identical, sorry for missing that before.

thanks,

greg k-h



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support using tcpc_gen_mux support

[Guenter Roeck]

On Wed, Sep 13, 2017 at 05:48:25PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 13-09-17 17:07, Rob Herring wrote:
> >On Wed, Sep 13, 2017 at 9:06 AM, Hans de Goede  wrote:
> >>Hi,
> >>
> >>
> >>On 13-09-17 15:38, Rob Herring wrote:
> >>>
> >>>On Wed, Sep 13, 2017 at 3:56 AM, Hans de Goede 
> >>>wrote:
> 
> Hi,
> 
> 
> On 13-09-17 00:20, Rob Herring wrote:
> >
> >
> >On Tue, Sep 05, 2017 at 06:42:20PM +0200, Hans de Goede wrote:
> >>
> >>
> >>Add mux support to the fusb302 driver, call devm_tcpc_gen_mux_create()
> >>to let the generic tcpc_mux_dev code create a tcpc_mux_dev for us.
> >>
> >>Also document the mux-names used by the generic tcpc_mux_dev code in
> >>our devicetree bindings.
> >>
> >>Cc: Rob Herring 
> >>Cc: Mark Rutland 
> >>Cc: devicet...@vger.kernel.org
> >>Signed-off-by: Hans de Goede 
> >>---
> >>Documentation/devicetree/bindings/usb/fcs,fusb302.txt |  3 +++
> >>drivers/staging/typec/fusb302/fusb302.c   | 11
> >>++-
> >>2 files changed, 13 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
> >>b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
> >>index 472facfa5a71..63d639eadacd 100644
> >>--- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
> >>+++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
> >>@@ -6,6 +6,9 @@ Required properties :
> >>- interrupts : Interrupt specifier
> >>  Optional properties :
> >>+- mux-controls   : List of mux-ctrl-specifiers containing 1 or
> >>2
> >>muxes
> >>+- mux-names  : "type-c-mode-mux" when using 1 mux, or
> >>+   "type-c-mode-mux", "usb-role-mux" when
> >>using
> >>2 muxes
> >
> >
> >
> >I'm not sure this is the right place for this. The mux is outside the
> >FUSB302. In a type-C connector node or USB phy node would make more
> >sense to me.
> 
> 
> 
> The mux certainly does not belong in the USB phy node, it sits between
> the
> USB PHY
> and the Type-C connector and can for example also mux the Type-C pins to
> a
> Display
> Port PHY.
> >>>
> >>>
> >>>Thinking about this some more, the mux(es) should be its own node(s).
> >>>Then the question is where to put the nodes.
> >>
> >>
> >>Right, the mux will be its own node per
> >>Documentation/devicetree/bindings/mux/mux-controller.txt
> >>the bindings bit this patch is adding is only adding phandles pointing
> >>to that mux-node as the fusb302 "consumes" the mux functionality.
> >>
> >>So as such (the fusb302 is the component which should control the mux)
> >>I do believe that the bindings this patch adds are correct.
> >
> >Humm, that's not how the mux binding works. The mux controller is what
> >drives the mux select lines and is the provider. The consumer is the
> >mux device itself. What decides the mux state is determined by what
> >you are muxing, not which node has mux-controls property.
> >
> >By putting mux-controls in fusb302 node, you are saying fusb302 is a
> >mux (or contains a mux).
> >
> >
> As for putting it in a type-C connector node, currently we do not have
> such
> a node,
> >>>
> >>>
> >>>Well, you should. Type-C connectors are certainly complicated enough
> >>>that we'll need one. Plus we already require connector nodes for
> >>>display outputs, so what do we do once you add display muxing?
> >>
> >>
> >>An interesting question, I'm working on this for x86 + ACPI boards actually,
> >>not a board using DT I've been adding DT bindings docs for device-properties
> >>I use because that seems like the right thing to do where the binding is
> >>obvious
> >>(which I believe it is in this case as the fusb302 is the mux consumer) and
> >>because the device-property code should work the same on x86 + ACPI
> >>(where some platform-specific drivers attach the device properties) and
> >>on e.g. ARM + DT.
> >>
> >>The rest should probably be left to be figured out when an actual DT
> >>using device using the fusb302 or another Type-C controller shows up.
> >
> >Well this is a new one (maybe, I suppose others have sneaked by). If
> >ACPI folks want to use DT bindings, then what do I care. But I have no
> >interest in reviewing ACPI properties. The whole notion of sharing
> >bindings between DT and ACPI beyond anything trivial is flawed IMO.
> >The ptifalls have been discussed multiple times before, so I'm not
> >going to repeat them here.
> 
> Ok, so shall I just drop the 
> Documentation/devicetree/bindings/usb/fcs,fusb302.txt
> part of this patch then ?
> 
FWIW, Android (where the fusb302 driver is coming from) does use dt.
On the other side I assume they won't jump on the new mux handling

Re: [PATCH v3 1/5] staging: typec: tcpm: Drop commented out code

[Guenter Roeck]

On Tue, Sep 12, 2017 at 10:38:39AM +0300, Heikki Krogerus wrote:
> On Mon, Sep 11, 2017 at 08:32:04PM -0700, Guenter Roeck wrote:
> > Commented out code can be added as needed. Drop it.
> > Also drop TODO and an obsolete XXX comment.
> > 
> > Signed-off-by: Guenter Roeck <li...@roeck-us.net>
> > ---
> > v2, v3: No change
> > 
> >  drivers/staging/typec/tcpm.c | 37 +
> >  1 file changed, 1 insertion(+), 36 deletions(-)
> 
> Nice! Just to be sure: The idea is to leave tcpci.c in staging, right?
> 
Yes, until we get test coverage or Greg gets tired of keeping it there
(in that case we would probably have to drop it).

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 3/5] staging: typec: pd: Document struct pd_message

[Guenter Roeck]

struct pd_message is the format of a PD message as seen on the wire.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
v2: No change
v3: Fix document tag

 drivers/staging/typec/pd.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/staging/typec/pd.h b/drivers/staging/typec/pd.h
index 30b32ad72acd..42a10883a2cb 100644
--- a/drivers/staging/typec/pd.h
+++ b/drivers/staging/typec/pd.h
@@ -104,6 +104,11 @@ static inline unsigned int pd_header_msgid_le(__le16 
header)
 
 #define PD_MAX_PAYLOAD 7
 
+/**
+ * struct pd_message - PD message as seen on wire
+ * @header:PD message header
+ * @payload:   PD message payload
+ */
 struct pd_message {
__le16 header;
__le32 payload[PD_MAX_PAYLOAD];
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 1/5] staging: typec: tcpm: Drop commented out code

[Guenter Roeck]

Commented out code can be added as needed. Drop it.
Also drop TODO and an obsolete XXX comment.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
v2, v3: No change

 drivers/staging/typec/tcpm.c | 37 +
 1 file changed, 1 insertion(+), 36 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 8af62e74d54c..cb25ec8334b0 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -908,27 +908,6 @@ static void svdm_consume_identity(struct tcpm_port *port, 
const __le32 *payload,
 
memset(>mode_data, 0, sizeof(port->mode_data));
 
-#if 0 /* Not really a match */
-   switch (PD_IDH_PTYPE(vdo)) {
-   case IDH_PTYPE_UNDEF:
-   port->partner.type = TYPEC_PARTNER_NONE; /* no longer exists */
-   break;
-   case IDH_PTYPE_HUB:
-   break;
-   case IDH_PTYPE_PERIPH:
-   break;
-   case IDH_PTYPE_PCABLE:
-   break;
-   case IDH_PTYPE_ACABLE:
-   break;
-   case IDH_PTYPE_AMA:
-   port->partner.type = TYPEC_PARTNER_ALTMODE;
-   break;
-   default:
-   break;
-   }
-#endif
-
port->partner_ident.id_header = vdo;
port->partner_ident.cert_stat = le32_to_cpu(payload[VDO_INDEX_CSTAT]);
port->partner_ident.product = product;
@@ -1103,11 +1082,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const 
__le32 *payload, int cnt,
response[0] = VDO(svid, 1, CMD_DISCOVER_MODES);
rlen = 1;
} else {
-#if 0
-   response[0] = pd_dfp_enter_mode(port, 0, 0);
-   if (response[0])
-   rlen = 1;
-#endif
+   /* enter alternate mode if/when implemented */
}
break;
case CMD_ENTER_MODE:
@@ -1145,10 +1120,6 @@ static void tcpm_handle_vdm_request(struct tcpm_port 
*port,
 
if (PD_VDO_SVDM(p0))
rlen = tcpm_pd_svdm(port, payload, cnt, response);
-#if 0
-   else
-   rlen = tcpm_pd_custom_vdm(port, cnt, payload, response);
-#endif
 
if (rlen > 0) {
tcpm_queue_vdm(port, response[0], [1], rlen - 1);
@@ -2442,7 +2413,6 @@ static void run_state_machine(struct tcpm_port *port)
tcpm_set_state(port, SNK_STARTUP, 0);
break;
case SNK_STARTUP:
-   /* XXX: callback into infrastructure */
opmode =  tcpm_get_pwr_opmode(port->polarity ?
  port->cc2 : port->cc1);
typec_set_pwr_opmode(port->typec_port, opmode);
@@ -3589,11 +3559,6 @@ struct tcpm_port *tcpm_register_port(struct device *dev, 
struct tcpc_dev *tcpc)
 
port->partner_desc.identity = >partner_ident;
port->port_type = tcpc->config->type;
-   /*
-* TODO:
-*  - alt_modes, set_alt_mode
-*  - {debug,audio}_accessory
-*/
 
port->typec_port = typec_register_port(port->dev, >typec_caps);
if (!port->typec_port) {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 5/5] usb: typec: fusb302: Move out of staging

[Guenter Roeck]

The driver is in good enough shape to be moved out of staging.
Do it.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
v2: Use format-patch -M
v3: No change

 drivers/staging/typec/Kconfig|  2 --
 drivers/staging/typec/Makefile   |  1 -
 drivers/staging/typec/fusb302/TODO   | 10 --
 drivers/usb/typec/Kconfig|  6 ++
 drivers/usb/typec/Makefile   |  1 +
 drivers/{staging => usb}/typec/fusb302/Kconfig   |  0
 drivers/{staging => usb}/typec/fusb302/Makefile  |  0
 drivers/{staging => usb}/typec/fusb302/fusb302.c |  0
 drivers/{staging => usb}/typec/fusb302/fusb302_reg.h |  0
 9 files changed, 7 insertions(+), 13 deletions(-)
 delete mode 100644 drivers/staging/typec/fusb302/TODO
 rename drivers/{staging => usb}/typec/fusb302/Kconfig (100%)
 rename drivers/{staging => usb}/typec/fusb302/Makefile (100%)
 rename drivers/{staging => usb}/typec/fusb302/fusb302.c (100%)
 rename drivers/{staging => usb}/typec/fusb302/fusb302_reg.h (100%)

diff --git a/drivers/staging/typec/Kconfig b/drivers/staging/typec/Kconfig
index 31fad23c2553..5359f556d203 100644
--- a/drivers/staging/typec/Kconfig
+++ b/drivers/staging/typec/Kconfig
@@ -9,8 +9,6 @@ config TYPEC_TCPCI
help
  Type-C Port Controller driver for TCPCI-compliant controller.
 
-source "drivers/staging/typec/fusb302/Kconfig"
-
 endif
 
 endmenu
diff --git a/drivers/staging/typec/Makefile b/drivers/staging/typec/Makefile
index e1df3f0fde10..53d649abcb53 100644
--- a/drivers/staging/typec/Makefile
+++ b/drivers/staging/typec/Makefile
@@ -1,2 +1 @@
 obj-$(CONFIG_TYPEC_TCPCI)  += tcpci.o
-obj-y  += fusb302/
diff --git a/drivers/staging/typec/fusb302/TODO 
b/drivers/staging/typec/fusb302/TODO
deleted file mode 100644
index 19b466eb585d..
--- a/drivers/staging/typec/fusb302/TODO
+++ /dev/null
@@ -1,10 +0,0 @@
-fusb302:
-- Find a better logging scheme, at least not having the same debugging/logging
-  code replicated here and in tcpm
-- Find a non-hacky way to coordinate between PM and I2C access
-- Documentation? The FUSB302 datasheet provides information on the chip to help
-  understand the code. But it may still be helpful to have a documentation.
-- We may want to replace the  "fcs,max-snk-microvolt", "fcs,max-snk-microamp",
-  "fcs,max-snk-microwatt" and "fcs,operating-snk-microwatt" device(tree)
-  properties with properties which are part of a generic type-c controller
-  devicetree binding.
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 888605860091..819c0ed2b200 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -12,6 +12,12 @@ config TYPEC_TCPM
  The Type-C Port Controller Manager provides a USB PD and USB Type-C
  state machine for use with Type-C Port Controllers.
 
+if TYPEC_TCPM
+
+source "drivers/usb/typec/fusb302/Kconfig"
+
+endif
+
 config TYPEC_WCOVE
tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
depends on ACPI
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index eb883984724b..b77688ce1f16 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_TYPEC)+= typec.o
 obj-$(CONFIG_TYPEC_TCPM)   += tcpm.o
+obj-y  += fusb302/
 obj-$(CONFIG_TYPEC_WCOVE)  += typec_wcove.o
 obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
diff --git a/drivers/staging/typec/fusb302/Kconfig 
b/drivers/usb/typec/fusb302/Kconfig
similarity index 100%
rename from drivers/staging/typec/fusb302/Kconfig
rename to drivers/usb/typec/fusb302/Kconfig
diff --git a/drivers/staging/typec/fusb302/Makefile 
b/drivers/usb/typec/fusb302/Makefile
similarity index 100%
rename from drivers/staging/typec/fusb302/Makefile
rename to drivers/usb/typec/fusb302/Makefile
diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
similarity index 100%
rename from drivers/staging/typec/fusb302/fusb302.c
rename to drivers/usb/typec/fusb302/fusb302.c
diff --git a/drivers/staging/typec/fusb302/fusb302_reg.h 
b/drivers/usb/typec/fusb302/fusb302_reg.h
similarity index 100%
rename from drivers/staging/typec/fusb302/fusb302_reg.h
rename to drivers/usb/typec/fusb302/fusb302_reg.h
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 4/5] typec: tcpm: Move out of staging

[Guenter Roeck]

Move tcpm (USB Type-C Port Manager) out of staging.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
v2: Use format-patch -M
v3: No change

 drivers/staging/typec/Kconfig |  8 
 drivers/staging/typec/Makefile|  1 -
 drivers/staging/typec/TODO| 10 --
 drivers/staging/typec/fusb302/fusb302.c   |  4 ++--
 drivers/staging/typec/tcpci.c |  4 ++--
 drivers/usb/typec/Kconfig |  8 
 drivers/usb/typec/Makefile|  1 +
 drivers/{staging => usb}/typec/tcpm.c |  9 -
 {drivers/staging/typec => include/linux/usb}/pd.h |  0
 {drivers/staging/typec => include/linux/usb}/pd_bdo.h |  0
 {drivers/staging/typec => include/linux/usb}/pd_vdo.h |  0
 {drivers/staging/typec => include/linux/usb}/tcpm.h   |  0
 12 files changed, 17 insertions(+), 28 deletions(-)
 rename drivers/{staging => usb}/typec/tcpm.c (99%)
 rename {drivers/staging/typec => include/linux/usb}/pd.h (100%)
 rename {drivers/staging/typec => include/linux/usb}/pd_bdo.h (100%)
 rename {drivers/staging/typec => include/linux/usb}/pd_vdo.h (100%)
 rename {drivers/staging/typec => include/linux/usb}/tcpm.h (100%)

diff --git a/drivers/staging/typec/Kconfig b/drivers/staging/typec/Kconfig
index 37a0781b0d0c..31fad23c2553 100644
--- a/drivers/staging/typec/Kconfig
+++ b/drivers/staging/typec/Kconfig
@@ -1,13 +1,5 @@
 menu "USB Power Delivery and Type-C drivers"
 
-config TYPEC_TCPM
-   tristate "USB Type-C Port Controller Manager"
-   depends on USB
-   select TYPEC
-   help
- The Type-C Port Controller Manager provides a USB PD and USB Type-C
- state machine for use with Type-C Port Controllers.
-
 if TYPEC_TCPM
 
 config TYPEC_TCPCI
diff --git a/drivers/staging/typec/Makefile b/drivers/staging/typec/Makefile
index 30a7e29cbc9e..e1df3f0fde10 100644
--- a/drivers/staging/typec/Makefile
+++ b/drivers/staging/typec/Makefile
@@ -1,3 +1,2 @@
-obj-$(CONFIG_TYPEC_TCPM)   += tcpm.o
 obj-$(CONFIG_TYPEC_TCPCI)  += tcpci.o
 obj-y  += fusb302/
diff --git a/drivers/staging/typec/TODO b/drivers/staging/typec/TODO
index bc1f97a2d1bf..53fe2f726c88 100644
--- a/drivers/staging/typec/TODO
+++ b/drivers/staging/typec/TODO
@@ -1,13 +1,3 @@
-tcpm:
-- Add documentation (at the very least for the API to low level drivers)
-- Split PD code into separate file
-- Check if it makes sense to use tracepoints instead of debugfs for debug logs
-- Implement Alternate Mode handling
-- Address "#if 0" code if not addressed with the above
-- Validate all comments marked with "XXX"; either address or remove comments
-- Add support for USB PD 3.0. While not mandatory, at least fast role swap
-  as well as authentication support would be very desirable.
-
 tcpci:
 - Test with real hardware
 
diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index fc6a3cf74eb3..e790b67d4953 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -37,11 +37,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #include "fusb302_reg.h"
-#include "../tcpm.h"
-#include "../pd.h"
 
 /*
  * When the device is SNK, BC_LVL interrupt is used to monitor cc pins
diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index df72d8b01e73..4636804ea1a4 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -20,11 +20,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
-#include "pd.h"
 #include "tcpci.h"
-#include "tcpm.h"
 
 #define PD_RETRY_COUNT 3
 
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index bc1b7745f1d4..888605860091 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -4,6 +4,14 @@ menu "USB Power Delivery and Type-C drivers"
 config TYPEC
tristate
 
+config TYPEC_TCPM
+   tristate "USB Type-C Port Controller Manager"
+   depends on USB
+   select TYPEC
+   help
+ The Type-C Port Controller Manager provides a USB PD and USB Type-C
+ state machine for use with Type-C Port Controllers.
+
 config TYPEC_WCOVE
tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
depends on ACPI
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index bc214f15f1b5..eb883984724b 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_TYPEC)+= typec.o
+obj-$(CONFIG_TYPEC_TCPM)   += tcpm.o
 obj-$(CONFIG_TYPEC_WCOVE)  += typec_wcove.o
 obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
diff --git a/drivers/staging/typec/tcpm.c b/drivers/usb/typec/tcpm.

[PATCH v3 2/5] staging: typec: tcpm: Document data structures

[Guenter Roeck]

Document struct tcpc_config and struct tcpc_dev.
Drop unused TCPC_USB_SWITCH_RESTORE.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
v2, v3: No change

 drivers/staging/typec/tcpm.h | 57 ++--
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 7e9a6b7b5cd6..073197f0d2bb 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -54,6 +54,27 @@ enum tcpm_transmit_type {
TCPC_TX_BIST_MODE_2 = 7
 };
 
+/**
+ * struct tcpc_config - Port configuration
+ * @src_pdo:   PDO parameters sent to port partner as response to
+ * PD_CTRL_GET_SOURCE_CAP message
+ * @nr_src_pdo:Number of entries in @src_pdo
+ * @snk_pdo:   PDO parameters sent to partner as response to
+ * PD_CTRL_GET_SINK_CAP message
+ * @nr_snk_pdo:Number of entries in @snk_pdo
+ * @max_snk_mv:Maximum acceptable sink voltage in mV
+ * @max_snk_ma:Maximum sink current in mA
+ * @max_snk_mw:Maximum required sink power in mW
+ * @operating_snk_mw:
+ * Required operating sink power in mW
+ * @type:  Port type (TYPEC_PORT_DFP, TYPEC_PORT_UFP, or
+ * TYPEC_PORT_DRP)
+ * @default_role:
+ * Default port role (TYPEC_SINK or TYPEC_SOURCE).
+ * Set to TYPEC_NO_PREFERRED_ROLE if no default role.
+ * @try_role_hw:True if try.{Src,Snk} is implemented in hardware
+ * @alt_modes: List of supported alternate modes
+ */
 struct tcpc_config {
const u32 *src_pdo;
unsigned int nr_src_pdo;
@@ -79,7 +100,6 @@ struct tcpc_config {
 enum tcpc_usb_switch {
TCPC_USB_SWITCH_CONNECT,
TCPC_USB_SWITCH_DISCONNECT,
-   TCPC_USB_SWITCH_RESTORE,/* TODO FIXME */
 };
 
 /* Mux state attributes */
@@ -104,17 +124,40 @@ struct tcpc_mux_dev {
void *priv_data;
 };
 
+/**
+ * struct tcpc_dev - Port configuration and callback functions
+ * @config:Pointer to port configuration
+ * @get_vbus:  Called to read current VBUS state
+ * @get_current_limit:
+ * Optional; called by the tcpm core when configured as a snk
+ * and cc=Rp-def. This allows the tcpm to provide a fallback
+ * current-limit detection method for the cc=Rp-def case.
+ * For example, some tcpcs may include BC1.2 charger detection
+ * and use that in this case.
+ * @set_cc:Called to set value of CC pins
+ * @get_cc:Called to read current CC pin values
+ * @set_polarity:
+ * Called to set polarity
+ * @set_vconn: Called to enable or disable VCONN
+ * @set_vbus:  Called to enable or disable VBUS
+ * @set_current_limit:
+ * Optional; called to set current limit as negotiated
+ * with partner.
+ * @set_pd_rx: Called to enable or disable reception of PD messages
+ * @set_roles: Called to set power and data roles
+ * @start_drp_toggling:
+ * Optional; if supported by hardware, called to start DRP
+ * toggling. DRP toggling is stopped automatically if
+ * a connection is established.
+ * @try_role:  Optional; called to set a preferred role
+ * @pd_transmit:Called to transmit PD message
+ * @mux:   Pointer to multiplexer data
+ */
 struct tcpc_dev {
const struct tcpc_config *config;
 
int (*init)(struct tcpc_dev *dev);
int (*get_vbus)(struct tcpc_dev *dev);
-   /*
-* This optional callback gets called by the tcpm core when configured
-* as a snk and cc=Rp-def. This allows the tcpm to provide a fallback
-* current-limit detection method for the cc=Rp-def case. E.g. some
-* tcpcs may include BC1.2 charger detection and use that in this case.
-*/
int (*get_current_limit)(struct tcpc_dev *dev);
int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc);
int (*get_cc)(struct tcpc_dev *dev, enum typec_cc_status *cc1,
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/5] staging: typec: tcpm: Drop commented out code

[Guenter Roeck]

On Mon, Sep 11, 2017 at 11:26:01AM -0700, Greg Kroah-Hartman wrote:
> On Mon, Sep 11, 2017 at 10:37:11AM -0700, Guenter Roeck wrote:
> > On Mon, Sep 11, 2017 at 09:23:34AM -0700, Greg Kroah-Hartman wrote:
> > > On Sun, Sep 10, 2017 at 01:37:01PM -0700, Guenter Roeck wrote:
> > > > Commented out code can be added as needed. Drop it.
> > > > Also drop TODO and an obsolete XXX comment.
> > > > 
> > > > Signed-off-by: Guenter Roeck <li...@roeck-us.net>
> > > > ---
> > > >  drivers/staging/typec/tcpm.c | 37 +
> > > >  1 file changed, 1 insertion(+), 36 deletions(-)
> > > 
> > > Thans for the series, I'll queue it up once 4.14-rc1 is out and make a
> > > branch that can get pulled into both the staging and usb trees, so that
> > > future work on this can all happen through the usb tree.
> > > 
> > Should I resend to add the '/**' in patch 3, or can you take care of it ?
> 
> A resend, with -M format, would always be appreciated :)
> 
I did that, as v2, but it does not include the comment change in patch 3.

I'll send v3 tonight.

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/5] staging: typec: tcpm: Drop commented out code

[Guenter Roeck]

On Mon, Sep 11, 2017 at 09:23:34AM -0700, Greg Kroah-Hartman wrote:
> On Sun, Sep 10, 2017 at 01:37:01PM -0700, Guenter Roeck wrote:
> > Commented out code can be added as needed. Drop it.
> > Also drop TODO and an obsolete XXX comment.
> > 
> > Signed-off-by: Guenter Roeck <li...@roeck-us.net>
> > ---
> >  drivers/staging/typec/tcpm.c | 37 +
> >  1 file changed, 1 insertion(+), 36 deletions(-)
> 
> Thans for the series, I'll queue it up once 4.14-rc1 is out and make a
> branch that can get pulled into both the staging and usb trees, so that
> future work on this can all happen through the usb tree.
> 
Should I resend to add the '/**' in patch 3, or can you take care of it ?

Thanks,
Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 5/5] usb: typec: fusb302: Move out of staging

[Guenter Roeck]

The driver is in good enough shape to be moved out of staging.
Do it.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
v2: Use format-patch -M

 drivers/staging/typec/Kconfig|  2 --
 drivers/staging/typec/Makefile   |  1 -
 drivers/staging/typec/fusb302/TODO   | 10 --
 drivers/usb/typec/Kconfig|  6 ++
 drivers/usb/typec/Makefile   |  1 +
 drivers/{staging => usb}/typec/fusb302/Kconfig   |  0
 drivers/{staging => usb}/typec/fusb302/Makefile  |  0
 drivers/{staging => usb}/typec/fusb302/fusb302.c |  0
 drivers/{staging => usb}/typec/fusb302/fusb302_reg.h |  0
 9 files changed, 7 insertions(+), 13 deletions(-)
 delete mode 100644 drivers/staging/typec/fusb302/TODO
 rename drivers/{staging => usb}/typec/fusb302/Kconfig (100%)
 rename drivers/{staging => usb}/typec/fusb302/Makefile (100%)
 rename drivers/{staging => usb}/typec/fusb302/fusb302.c (100%)
 rename drivers/{staging => usb}/typec/fusb302/fusb302_reg.h (100%)

diff --git a/drivers/staging/typec/Kconfig b/drivers/staging/typec/Kconfig
index 31fad23c2553..5359f556d203 100644
--- a/drivers/staging/typec/Kconfig
+++ b/drivers/staging/typec/Kconfig
@@ -9,8 +9,6 @@ config TYPEC_TCPCI
help
  Type-C Port Controller driver for TCPCI-compliant controller.
 
-source "drivers/staging/typec/fusb302/Kconfig"
-
 endif
 
 endmenu
diff --git a/drivers/staging/typec/Makefile b/drivers/staging/typec/Makefile
index e1df3f0fde10..53d649abcb53 100644
--- a/drivers/staging/typec/Makefile
+++ b/drivers/staging/typec/Makefile
@@ -1,2 +1 @@
 obj-$(CONFIG_TYPEC_TCPCI)  += tcpci.o
-obj-y  += fusb302/
diff --git a/drivers/staging/typec/fusb302/TODO 
b/drivers/staging/typec/fusb302/TODO
deleted file mode 100644
index 19b466eb585d..
--- a/drivers/staging/typec/fusb302/TODO
+++ /dev/null
@@ -1,10 +0,0 @@
-fusb302:
-- Find a better logging scheme, at least not having the same debugging/logging
-  code replicated here and in tcpm
-- Find a non-hacky way to coordinate between PM and I2C access
-- Documentation? The FUSB302 datasheet provides information on the chip to help
-  understand the code. But it may still be helpful to have a documentation.
-- We may want to replace the  "fcs,max-snk-microvolt", "fcs,max-snk-microamp",
-  "fcs,max-snk-microwatt" and "fcs,operating-snk-microwatt" device(tree)
-  properties with properties which are part of a generic type-c controller
-  devicetree binding.
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 888605860091..819c0ed2b200 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -12,6 +12,12 @@ config TYPEC_TCPM
  The Type-C Port Controller Manager provides a USB PD and USB Type-C
  state machine for use with Type-C Port Controllers.
 
+if TYPEC_TCPM
+
+source "drivers/usb/typec/fusb302/Kconfig"
+
+endif
+
 config TYPEC_WCOVE
tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
depends on ACPI
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index eb883984724b..b77688ce1f16 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_TYPEC)+= typec.o
 obj-$(CONFIG_TYPEC_TCPM)   += tcpm.o
+obj-y  += fusb302/
 obj-$(CONFIG_TYPEC_WCOVE)  += typec_wcove.o
 obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
diff --git a/drivers/staging/typec/fusb302/Kconfig 
b/drivers/usb/typec/fusb302/Kconfig
similarity index 100%
rename from drivers/staging/typec/fusb302/Kconfig
rename to drivers/usb/typec/fusb302/Kconfig
diff --git a/drivers/staging/typec/fusb302/Makefile 
b/drivers/usb/typec/fusb302/Makefile
similarity index 100%
rename from drivers/staging/typec/fusb302/Makefile
rename to drivers/usb/typec/fusb302/Makefile
diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
similarity index 100%
rename from drivers/staging/typec/fusb302/fusb302.c
rename to drivers/usb/typec/fusb302/fusb302.c
diff --git a/drivers/staging/typec/fusb302/fusb302_reg.h 
b/drivers/usb/typec/fusb302/fusb302_reg.h
similarity index 100%
rename from drivers/staging/typec/fusb302/fusb302_reg.h
rename to drivers/usb/typec/fusb302/fusb302_reg.h
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 4/5] typec: tcpm: Move out of staging

[Guenter Roeck]

Move tcpm (USB Type-C Port Manager) out of staging.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
v2: Use format-patch -M

 drivers/staging/typec/Kconfig |  8 
 drivers/staging/typec/Makefile|  1 -
 drivers/staging/typec/TODO| 10 --
 drivers/staging/typec/fusb302/fusb302.c   |  4 ++--
 drivers/staging/typec/tcpci.c |  4 ++--
 drivers/usb/typec/Kconfig |  8 
 drivers/usb/typec/Makefile|  1 +
 drivers/{staging => usb}/typec/tcpm.c |  9 -
 {drivers/staging/typec => include/linux/usb}/pd.h |  0
 {drivers/staging/typec => include/linux/usb}/pd_bdo.h |  0
 {drivers/staging/typec => include/linux/usb}/pd_vdo.h |  0
 {drivers/staging/typec => include/linux/usb}/tcpm.h   |  0
 12 files changed, 17 insertions(+), 28 deletions(-)
 rename drivers/{staging => usb}/typec/tcpm.c (99%)
 rename {drivers/staging/typec => include/linux/usb}/pd.h (100%)
 rename {drivers/staging/typec => include/linux/usb}/pd_bdo.h (100%)
 rename {drivers/staging/typec => include/linux/usb}/pd_vdo.h (100%)
 rename {drivers/staging/typec => include/linux/usb}/tcpm.h (100%)

diff --git a/drivers/staging/typec/Kconfig b/drivers/staging/typec/Kconfig
index 37a0781b0d0c..31fad23c2553 100644
--- a/drivers/staging/typec/Kconfig
+++ b/drivers/staging/typec/Kconfig
@@ -1,13 +1,5 @@
 menu "USB Power Delivery and Type-C drivers"
 
-config TYPEC_TCPM
-   tristate "USB Type-C Port Controller Manager"
-   depends on USB
-   select TYPEC
-   help
- The Type-C Port Controller Manager provides a USB PD and USB Type-C
- state machine for use with Type-C Port Controllers.
-
 if TYPEC_TCPM
 
 config TYPEC_TCPCI
diff --git a/drivers/staging/typec/Makefile b/drivers/staging/typec/Makefile
index 30a7e29cbc9e..e1df3f0fde10 100644
--- a/drivers/staging/typec/Makefile
+++ b/drivers/staging/typec/Makefile
@@ -1,3 +1,2 @@
-obj-$(CONFIG_TYPEC_TCPM)   += tcpm.o
 obj-$(CONFIG_TYPEC_TCPCI)  += tcpci.o
 obj-y  += fusb302/
diff --git a/drivers/staging/typec/TODO b/drivers/staging/typec/TODO
index bc1f97a2d1bf..53fe2f726c88 100644
--- a/drivers/staging/typec/TODO
+++ b/drivers/staging/typec/TODO
@@ -1,13 +1,3 @@
-tcpm:
-- Add documentation (at the very least for the API to low level drivers)
-- Split PD code into separate file
-- Check if it makes sense to use tracepoints instead of debugfs for debug logs
-- Implement Alternate Mode handling
-- Address "#if 0" code if not addressed with the above
-- Validate all comments marked with "XXX"; either address or remove comments
-- Add support for USB PD 3.0. While not mandatory, at least fast role swap
-  as well as authentication support would be very desirable.
-
 tcpci:
 - Test with real hardware
 
diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index fc6a3cf74eb3..e790b67d4953 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -37,11 +37,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #include "fusb302_reg.h"
-#include "../tcpm.h"
-#include "../pd.h"
 
 /*
  * When the device is SNK, BC_LVL interrupt is used to monitor cc pins
diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index df72d8b01e73..4636804ea1a4 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -20,11 +20,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
-#include "pd.h"
 #include "tcpci.h"
-#include "tcpm.h"
 
 #define PD_RETRY_COUNT 3
 
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index bc1b7745f1d4..888605860091 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -4,6 +4,14 @@ menu "USB Power Delivery and Type-C drivers"
 config TYPEC
tristate
 
+config TYPEC_TCPM
+   tristate "USB Type-C Port Controller Manager"
+   depends on USB
+   select TYPEC
+   help
+ The Type-C Port Controller Manager provides a USB PD and USB Type-C
+ state machine for use with Type-C Port Controllers.
+
 config TYPEC_WCOVE
tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
depends on ACPI
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index bc214f15f1b5..eb883984724b 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_TYPEC)+= typec.o
+obj-$(CONFIG_TYPEC_TCPM)   += tcpm.o
 obj-$(CONFIG_TYPEC_WCOVE)  += typec_wcove.o
 obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
diff --git a/drivers/staging/typec/tcpm.c b/drivers/usb/typec/tcpm.c
similarity 

[PATCH v2 2/5] staging: typec: tcpm: Document data structures

[Guenter Roeck]

Document struct tcpc_config and struct tcpc_dev.
Drop unused TCPC_USB_SWITCH_RESTORE.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
v2: No change

 drivers/staging/typec/tcpm.h | 57 ++--
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 7e9a6b7b5cd6..073197f0d2bb 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -54,6 +54,27 @@ enum tcpm_transmit_type {
TCPC_TX_BIST_MODE_2 = 7
 };
 
+/**
+ * struct tcpc_config - Port configuration
+ * @src_pdo:   PDO parameters sent to port partner as response to
+ * PD_CTRL_GET_SOURCE_CAP message
+ * @nr_src_pdo:Number of entries in @src_pdo
+ * @snk_pdo:   PDO parameters sent to partner as response to
+ * PD_CTRL_GET_SINK_CAP message
+ * @nr_snk_pdo:Number of entries in @snk_pdo
+ * @max_snk_mv:Maximum acceptable sink voltage in mV
+ * @max_snk_ma:Maximum sink current in mA
+ * @max_snk_mw:Maximum required sink power in mW
+ * @operating_snk_mw:
+ * Required operating sink power in mW
+ * @type:  Port type (TYPEC_PORT_DFP, TYPEC_PORT_UFP, or
+ * TYPEC_PORT_DRP)
+ * @default_role:
+ * Default port role (TYPEC_SINK or TYPEC_SOURCE).
+ * Set to TYPEC_NO_PREFERRED_ROLE if no default role.
+ * @try_role_hw:True if try.{Src,Snk} is implemented in hardware
+ * @alt_modes: List of supported alternate modes
+ */
 struct tcpc_config {
const u32 *src_pdo;
unsigned int nr_src_pdo;
@@ -79,7 +100,6 @@ struct tcpc_config {
 enum tcpc_usb_switch {
TCPC_USB_SWITCH_CONNECT,
TCPC_USB_SWITCH_DISCONNECT,
-   TCPC_USB_SWITCH_RESTORE,/* TODO FIXME */
 };
 
 /* Mux state attributes */
@@ -104,17 +124,40 @@ struct tcpc_mux_dev {
void *priv_data;
 };
 
+/**
+ * struct tcpc_dev - Port configuration and callback functions
+ * @config:Pointer to port configuration
+ * @get_vbus:  Called to read current VBUS state
+ * @get_current_limit:
+ * Optional; called by the tcpm core when configured as a snk
+ * and cc=Rp-def. This allows the tcpm to provide a fallback
+ * current-limit detection method for the cc=Rp-def case.
+ * For example, some tcpcs may include BC1.2 charger detection
+ * and use that in this case.
+ * @set_cc:Called to set value of CC pins
+ * @get_cc:Called to read current CC pin values
+ * @set_polarity:
+ * Called to set polarity
+ * @set_vconn: Called to enable or disable VCONN
+ * @set_vbus:  Called to enable or disable VBUS
+ * @set_current_limit:
+ * Optional; called to set current limit as negotiated
+ * with partner.
+ * @set_pd_rx: Called to enable or disable reception of PD messages
+ * @set_roles: Called to set power and data roles
+ * @start_drp_toggling:
+ * Optional; if supported by hardware, called to start DRP
+ * toggling. DRP toggling is stopped automatically if
+ * a connection is established.
+ * @try_role:  Optional; called to set a preferred role
+ * @pd_transmit:Called to transmit PD message
+ * @mux:   Pointer to multiplexer data
+ */
 struct tcpc_dev {
const struct tcpc_config *config;
 
int (*init)(struct tcpc_dev *dev);
int (*get_vbus)(struct tcpc_dev *dev);
-   /*
-* This optional callback gets called by the tcpm core when configured
-* as a snk and cc=Rp-def. This allows the tcpm to provide a fallback
-* current-limit detection method for the cc=Rp-def case. E.g. some
-* tcpcs may include BC1.2 charger detection and use that in this case.
-*/
int (*get_current_limit)(struct tcpc_dev *dev);
int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc);
int (*get_cc)(struct tcpc_dev *dev, enum typec_cc_status *cc1,
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 3/5] staging: typec: pd: Document struct pd_message

[Guenter Roeck]

struct pd_message is the format of a PD message as seen on the wire.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
v2: No change

 drivers/staging/typec/pd.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/staging/typec/pd.h b/drivers/staging/typec/pd.h
index 30b32ad72acd..42a10883a2cb 100644
--- a/drivers/staging/typec/pd.h
+++ b/drivers/staging/typec/pd.h
@@ -104,6 +104,11 @@ static inline unsigned int pd_header_msgid_le(__le16 
header)
 
 #define PD_MAX_PAYLOAD 7
 
+/*
+ * struct pd_message - PD message as seen on wire
+ * @header:PD message header
+ * @payload:   PD message payload
+ */
 struct pd_message {
__le16 header;
__le32 payload[PD_MAX_PAYLOAD];
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/5] staging: typec: tcpm: Drop commented out code

[Guenter Roeck]

Commented out code can be added as needed. Drop it.
Also drop TODO and an obsolete XXX comment.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
v2: No change

 drivers/staging/typec/tcpm.c | 37 +
 1 file changed, 1 insertion(+), 36 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 8af62e74d54c..cb25ec8334b0 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -908,27 +908,6 @@ static void svdm_consume_identity(struct tcpm_port *port, 
const __le32 *payload,
 
memset(>mode_data, 0, sizeof(port->mode_data));
 
-#if 0 /* Not really a match */
-   switch (PD_IDH_PTYPE(vdo)) {
-   case IDH_PTYPE_UNDEF:
-   port->partner.type = TYPEC_PARTNER_NONE; /* no longer exists */
-   break;
-   case IDH_PTYPE_HUB:
-   break;
-   case IDH_PTYPE_PERIPH:
-   break;
-   case IDH_PTYPE_PCABLE:
-   break;
-   case IDH_PTYPE_ACABLE:
-   break;
-   case IDH_PTYPE_AMA:
-   port->partner.type = TYPEC_PARTNER_ALTMODE;
-   break;
-   default:
-   break;
-   }
-#endif
-
port->partner_ident.id_header = vdo;
port->partner_ident.cert_stat = le32_to_cpu(payload[VDO_INDEX_CSTAT]);
port->partner_ident.product = product;
@@ -1103,11 +1082,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const 
__le32 *payload, int cnt,
response[0] = VDO(svid, 1, CMD_DISCOVER_MODES);
rlen = 1;
} else {
-#if 0
-   response[0] = pd_dfp_enter_mode(port, 0, 0);
-   if (response[0])
-   rlen = 1;
-#endif
+   /* enter alternate mode if/when implemented */
}
break;
case CMD_ENTER_MODE:
@@ -1145,10 +1120,6 @@ static void tcpm_handle_vdm_request(struct tcpm_port 
*port,
 
if (PD_VDO_SVDM(p0))
rlen = tcpm_pd_svdm(port, payload, cnt, response);
-#if 0
-   else
-   rlen = tcpm_pd_custom_vdm(port, cnt, payload, response);
-#endif
 
if (rlen > 0) {
tcpm_queue_vdm(port, response[0], [1], rlen - 1);
@@ -2442,7 +2413,6 @@ static void run_state_machine(struct tcpm_port *port)
tcpm_set_state(port, SNK_STARTUP, 0);
break;
case SNK_STARTUP:
-   /* XXX: callback into infrastructure */
opmode =  tcpm_get_pwr_opmode(port->polarity ?
  port->cc2 : port->cc1);
typec_set_pwr_opmode(port->typec_port, opmode);
@@ -3589,11 +3559,6 @@ struct tcpm_port *tcpm_register_port(struct device *dev, 
struct tcpc_dev *tcpc)
 
port->partner_desc.identity = >partner_ident;
port->port_type = tcpc->config->type;
-   /*
-* TODO:
-*  - alt_modes, set_alt_mode
-*  - {debug,audio}_accessory
-*/
 
port->typec_port = typec_register_port(port->dev, >typec_caps);
if (!port->typec_port) {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 08/11] staging: typec: tcpm: Set mux to device mode when configured as such

[Guenter Roeck]

On 09/05/2017 09:42 AM, Hans de Goede wrote:

Setting the mux to TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT when the
data-role is device is not correct. Plenty of devices support operating
as USB device through a (separate) USB device controller.

So this commit instead splits out TYPEC_MUX_USB into TYPEC_MUX_USB_HOST
and TYPEC_MUX_USB_DEVICE and makes tcpm_set_roles() set the mux
accordingly.

Likewise TCPC_MUX_DP gets renamed to TCPC_MUX_DP_SRC to make clear that
this is for configuring the Type-C port as a Display Port source, not a
sink.

Last this commit makes tcpm_reset_port() to set the mux to
TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT so that it does not and up
staying in host (and with this commit also device) mode after a detach.


This sentence is hard to understand.


Signed-off-by: Hans de Goede <hdego...@redhat.com>


Otherwise

Reviewed-by: Guenter Roeck <li...@roeck-us.net>


---
  drivers/staging/typec/tcpm.c |  7 ---
  drivers/staging/typec/tcpm.h | 22 ++
  2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 8af62e74d54c..ffe7e26d4ed3 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -752,11 +752,11 @@ static int tcpm_set_roles(struct tcpm_port *port, bool 
attached,
int ret;
  
  	if (data == TYPEC_HOST)

-   ret = tcpm_mux_set(port, TYPEC_MUX_USB,
+   ret = tcpm_mux_set(port, TYPEC_MUX_USB_HOST,
   TCPC_USB_SWITCH_CONNECT);
else
-   ret = tcpm_mux_set(port, TYPEC_MUX_NONE,
-  TCPC_USB_SWITCH_DISCONNECT);
+   ret = tcpm_mux_set(port, TYPEC_MUX_USB_DEVICE,
+  TCPC_USB_SWITCH_CONNECT);
if (ret < 0)
return ret;
  
@@ -2025,6 +2025,7 @@ static void tcpm_reset_port(struct tcpm_port *port)

tcpm_init_vconn(port);
tcpm_set_current_limit(port, 0, 0);
tcpm_set_polarity(port, TYPEC_POLARITY_CC1);
+   tcpm_mux_set(port, TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT);
tcpm_set_attached_state(port, false);
port->try_src_count = 0;
port->try_snk_count = 0;
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 7e9a6b7b5cd6..f662eed48c86 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -83,17 +83,23 @@ enum tcpc_usb_switch {
  };
  
  /* Mux state attributes */

-#define TCPC_MUX_USB_ENABLED   BIT(0)  /* USB enabled */
-#define TCPC_MUX_DP_ENABLEDBIT(1)  /* DP enabled */
-#define TCPC_MUX_POLARITY_INVERTED BIT(2)  /* Polarity inverted */
+#define TCPC_MUX_USB_DEVICE_ENABLEDBIT(0)  /* USB device enabled */
+#define TCPC_MUX_USB_HOST_ENABLED  BIT(1)  /* USB host enabled */
+#define TCPC_MUX_DP_SRC_ENABLEDBIT(2)  /* DP enabled */
+#define TCPC_MUX_POLARITY_INVERTED BIT(3)  /* Polarity inverted */
  
  /* Mux modes, decoded to attributes */

  enum tcpc_mux_mode {
-   TYPEC_MUX_NONE  = 0,/* Open switch */
-   TYPEC_MUX_USB   = TCPC_MUX_USB_ENABLED, /* USB only */
-   TYPEC_MUX_DP= TCPC_MUX_DP_ENABLED,  /* DP only */
-   TYPEC_MUX_DOCK  = TCPC_MUX_USB_ENABLED |/* Both USB and DP */
- TCPC_MUX_DP_ENABLED,
+   /* Open switch */
+   TYPEC_MUX_NONE = 0,
+   /* USB device only */
+   TYPEC_MUX_USB_DEVICE = TCPC_MUX_USB_DEVICE_ENABLED,
+   /* USB host only */
+   TYPEC_MUX_USB_HOST = TCPC_MUX_USB_HOST_ENABLED,
+   /* DP source only */
+   TYPEC_MUX_DP = TCPC_MUX_DP_SRC_ENABLED,
+   /* Both USB host and DP source */
+   TYPEC_MUX_DOCK = TCPC_MUX_USB_HOST_ENABLED | TCPC_MUX_DP_SRC_ENABLED,
  };
  
  struct tcpc_mux_dev {




___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/5] typec: tcpm: Move out of staging

[Guenter Roeck]

On 09/10/2017 01:46 PM, Joe Perches wrote:

On Sun, 2017-09-10 at 13:37 -0700, Guenter Roeck wrote:

Move tcpm (USB Type-C Port Manager) out of staging.


git format-patch -M



Thanks - I'll resend but wait a bit for additional comments before I do.

Guenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/5] staging: typec: pd: Document struct pd_message

[Guenter Roeck]

struct pd_message is the format of a PD message as seen on the wire.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
 drivers/staging/typec/pd.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/staging/typec/pd.h b/drivers/staging/typec/pd.h
index 30b32ad72acd..42a10883a2cb 100644
--- a/drivers/staging/typec/pd.h
+++ b/drivers/staging/typec/pd.h
@@ -104,6 +104,11 @@ static inline unsigned int pd_header_msgid_le(__le16 
header)
 
 #define PD_MAX_PAYLOAD 7
 
+/*
+ * struct pd_message - PD message as seen on wire
+ * @header:PD message header
+ * @payload:   PD message payload
+ */
 struct pd_message {
__le16 header;
__le32 payload[PD_MAX_PAYLOAD];
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/5] staging: typec: tcpm: Document data structures

[Guenter Roeck]

Document struct tcpc_config and struct tcpc_dev.
Drop unused TCPC_USB_SWITCH_RESTORE.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
 drivers/staging/typec/tcpm.h | 57 ++--
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 7e9a6b7b5cd6..073197f0d2bb 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -54,6 +54,27 @@ enum tcpm_transmit_type {
TCPC_TX_BIST_MODE_2 = 7
 };
 
+/**
+ * struct tcpc_config - Port configuration
+ * @src_pdo:   PDO parameters sent to port partner as response to
+ * PD_CTRL_GET_SOURCE_CAP message
+ * @nr_src_pdo:Number of entries in @src_pdo
+ * @snk_pdo:   PDO parameters sent to partner as response to
+ * PD_CTRL_GET_SINK_CAP message
+ * @nr_snk_pdo:Number of entries in @snk_pdo
+ * @max_snk_mv:Maximum acceptable sink voltage in mV
+ * @max_snk_ma:Maximum sink current in mA
+ * @max_snk_mw:Maximum required sink power in mW
+ * @operating_snk_mw:
+ * Required operating sink power in mW
+ * @type:  Port type (TYPEC_PORT_DFP, TYPEC_PORT_UFP, or
+ * TYPEC_PORT_DRP)
+ * @default_role:
+ * Default port role (TYPEC_SINK or TYPEC_SOURCE).
+ * Set to TYPEC_NO_PREFERRED_ROLE if no default role.
+ * @try_role_hw:True if try.{Src,Snk} is implemented in hardware
+ * @alt_modes: List of supported alternate modes
+ */
 struct tcpc_config {
const u32 *src_pdo;
unsigned int nr_src_pdo;
@@ -79,7 +100,6 @@ struct tcpc_config {
 enum tcpc_usb_switch {
TCPC_USB_SWITCH_CONNECT,
TCPC_USB_SWITCH_DISCONNECT,
-   TCPC_USB_SWITCH_RESTORE,/* TODO FIXME */
 };
 
 /* Mux state attributes */
@@ -104,17 +124,40 @@ struct tcpc_mux_dev {
void *priv_data;
 };
 
+/**
+ * struct tcpc_dev - Port configuration and callback functions
+ * @config:Pointer to port configuration
+ * @get_vbus:  Called to read current VBUS state
+ * @get_current_limit:
+ * Optional; called by the tcpm core when configured as a snk
+ * and cc=Rp-def. This allows the tcpm to provide a fallback
+ * current-limit detection method for the cc=Rp-def case.
+ * For example, some tcpcs may include BC1.2 charger detection
+ * and use that in this case.
+ * @set_cc:Called to set value of CC pins
+ * @get_cc:Called to read current CC pin values
+ * @set_polarity:
+ * Called to set polarity
+ * @set_vconn: Called to enable or disable VCONN
+ * @set_vbus:  Called to enable or disable VBUS
+ * @set_current_limit:
+ * Optional; called to set current limit as negotiated
+ * with partner.
+ * @set_pd_rx: Called to enable or disable reception of PD messages
+ * @set_roles: Called to set power and data roles
+ * @start_drp_toggling:
+ * Optional; if supported by hardware, called to start DRP
+ * toggling. DRP toggling is stopped automatically if
+ * a connection is established.
+ * @try_role:  Optional; called to set a preferred role
+ * @pd_transmit:Called to transmit PD message
+ * @mux:   Pointer to multiplexer data
+ */
 struct tcpc_dev {
const struct tcpc_config *config;
 
int (*init)(struct tcpc_dev *dev);
int (*get_vbus)(struct tcpc_dev *dev);
-   /*
-* This optional callback gets called by the tcpm core when configured
-* as a snk and cc=Rp-def. This allows the tcpm to provide a fallback
-* current-limit detection method for the cc=Rp-def case. E.g. some
-* tcpcs may include BC1.2 charger detection and use that in this case.
-*/
int (*get_current_limit)(struct tcpc_dev *dev);
int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc);
int (*get_cc)(struct tcpc_dev *dev, enum typec_cc_status *cc1,
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/5] staging: typec: tcpm: Drop commented out code

[Guenter Roeck]

Commented out code can be added as needed. Drop it.
Also drop TODO and an obsolete XXX comment.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
 drivers/staging/typec/tcpm.c | 37 +
 1 file changed, 1 insertion(+), 36 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 8af62e74d54c..cb25ec8334b0 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -908,27 +908,6 @@ static void svdm_consume_identity(struct tcpm_port *port, 
const __le32 *payload,
 
memset(>mode_data, 0, sizeof(port->mode_data));
 
-#if 0 /* Not really a match */
-   switch (PD_IDH_PTYPE(vdo)) {
-   case IDH_PTYPE_UNDEF:
-   port->partner.type = TYPEC_PARTNER_NONE; /* no longer exists */
-   break;
-   case IDH_PTYPE_HUB:
-   break;
-   case IDH_PTYPE_PERIPH:
-   break;
-   case IDH_PTYPE_PCABLE:
-   break;
-   case IDH_PTYPE_ACABLE:
-   break;
-   case IDH_PTYPE_AMA:
-   port->partner.type = TYPEC_PARTNER_ALTMODE;
-   break;
-   default:
-   break;
-   }
-#endif
-
port->partner_ident.id_header = vdo;
port->partner_ident.cert_stat = le32_to_cpu(payload[VDO_INDEX_CSTAT]);
port->partner_ident.product = product;
@@ -1103,11 +1082,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const 
__le32 *payload, int cnt,
response[0] = VDO(svid, 1, CMD_DISCOVER_MODES);
rlen = 1;
} else {
-#if 0
-   response[0] = pd_dfp_enter_mode(port, 0, 0);
-   if (response[0])
-   rlen = 1;
-#endif
+   /* enter alternate mode if/when implemented */
}
break;
case CMD_ENTER_MODE:
@@ -1145,10 +1120,6 @@ static void tcpm_handle_vdm_request(struct tcpm_port 
*port,
 
if (PD_VDO_SVDM(p0))
rlen = tcpm_pd_svdm(port, payload, cnt, response);
-#if 0
-   else
-   rlen = tcpm_pd_custom_vdm(port, cnt, payload, response);
-#endif
 
if (rlen > 0) {
tcpm_queue_vdm(port, response[0], [1], rlen - 1);
@@ -2442,7 +2413,6 @@ static void run_state_machine(struct tcpm_port *port)
tcpm_set_state(port, SNK_STARTUP, 0);
break;
case SNK_STARTUP:
-   /* XXX: callback into infrastructure */
opmode =  tcpm_get_pwr_opmode(port->polarity ?
  port->cc2 : port->cc1);
typec_set_pwr_opmode(port->typec_port, opmode);
@@ -3589,11 +3559,6 @@ struct tcpm_port *tcpm_register_port(struct device *dev, 
struct tcpc_dev *tcpc)
 
port->partner_desc.identity = >partner_ident;
port->port_type = tcpc->config->type;
-   /*
-* TODO:
-*  - alt_modes, set_alt_mode
-*  - {debug,audio}_accessory
-*/
 
port->typec_port = typec_register_port(port->dev, >typec_caps);
if (!port->typec_port) {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: typec: tcpm: Validate source and sink caps

[Guenter Roeck]

On 09/09/2017 09:54 AM, Guenter Roeck wrote:

On 09/08/2017 12:13 PM, Greg Kroah-Hartman wrote:

On Fri, Sep 08, 2017 at 10:29:52AM -0700, Badhri Jagan Sridharan wrote:

On Fri, Sep 8, 2017 at 2:45 AM, Greg Kroah-Hartman
<gre...@linuxfoundation.org> wrote:

On Thu, Sep 07, 2017 at 06:22:13PM -0700, Badhri Jagan Sridharan wrote:

The source and sink caps should follow the following rules.
This patch validates whether the src_caps/snk_caps adheres
to it.

6.4.1 Capabilities Message
A Capabilities message (Source Capabilities message or Sink
Capabilities message) shall have at least one Power
Data Object for vSafe5V. The Capabilities message shall also
contain the sending Port’s information followed by up to
6 additional Power Data Objects. Power Data Objects in a
Capabilities message shall be sent in the following order:

1. The vSafe5V Fixed Supply Object shall always be the first object.
2. The remaining Fixed Supply Objects, if present, shall be sent
in voltage order; lowest to highest.
3. The Battery Supply Objects, if present shall be sent in Minimum
Voltage order; lowest to highest.
4. The Variable Supply (non-battery) Objects, if present, shall be
sent in Minimum Voltage order; lowest to highest.

Errors in source/sink_caps of the local port will prevent
the port registration. Whereas, errors in source caps of partner
device would only log them.

Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>
---
  drivers/staging/typec/pd.h   |   2 +
  drivers/staging/typec/tcpm.c | 107 +++
  drivers/staging/typec/tcpm.h |  16 +++
  3 files changed, 108 insertions(+), 17 deletions(-)


Before you add more stuff to this driver, what is needed to get it out
of staging?  That would be more useful to do now, right?


Actually not adding more features here. There was a bug where
the phone wasnt charging at the highest possible power output,
came up with the these patches while debugging that issue.


Ok, but again, when is this going to get out of staging?


I have a set of patches ready to do just that. It only moves tcpm - tcpci
is still untested as far as I know, and fusb302 has unapproved devicetree
properties.



Actually, the dt properties are Acked by Rob, so we can move that driver as well
(TODO is a bit misleading there). I'll add that to the patch series.

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: typec: tcpm: Validate source and sink caps

[Guenter Roeck]

On 09/08/2017 12:13 PM, Greg Kroah-Hartman wrote:

On Fri, Sep 08, 2017 at 10:29:52AM -0700, Badhri Jagan Sridharan wrote:

On Fri, Sep 8, 2017 at 2:45 AM, Greg Kroah-Hartman
 wrote:

On Thu, Sep 07, 2017 at 06:22:13PM -0700, Badhri Jagan Sridharan wrote:

The source and sink caps should follow the following rules.
This patch validates whether the src_caps/snk_caps adheres
to it.

6.4.1 Capabilities Message
A Capabilities message (Source Capabilities message or Sink
Capabilities message) shall have at least one Power
Data Object for vSafe5V. The Capabilities message shall also
contain the sending Port’s information followed by up to
6 additional Power Data Objects. Power Data Objects in a
Capabilities message shall be sent in the following order:

1. The vSafe5V Fixed Supply Object shall always be the first object.
2. The remaining Fixed Supply Objects, if present, shall be sent
in voltage order; lowest to highest.
3. The Battery Supply Objects, if present shall be sent in Minimum
Voltage order; lowest to highest.
4. The Variable Supply (non-battery) Objects, if present, shall be
sent in Minimum Voltage order; lowest to highest.

Errors in source/sink_caps of the local port will prevent
the port registration. Whereas, errors in source caps of partner
device would only log them.

Signed-off-by: Badhri Jagan Sridharan 
---
  drivers/staging/typec/pd.h   |   2 +
  drivers/staging/typec/tcpm.c | 107 +++
  drivers/staging/typec/tcpm.h |  16 +++
  3 files changed, 108 insertions(+), 17 deletions(-)


Before you add more stuff to this driver, what is needed to get it out
of staging?  That would be more useful to do now, right?


Actually not adding more features here. There was a bug where
the phone wasnt charging at the highest possible power output,
came up with the these patches while debugging that issue.


Ok, but again, when is this going to get out of staging?


I have a set of patches ready to do just that. It only moves tcpm - tcpci
is still untested as far as I know, and fusb302 has unapproved devicetree
properties.

Want me to send it out now or after -rc1 ?

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: typec: tcpm: Validate source and sink caps

[Guenter Roeck]

On Fri, Sep 08, 2017 at 09:13:25PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Sep 08, 2017 at 10:29:52AM -0700, Badhri Jagan Sridharan wrote:
> > On Fri, Sep 8, 2017 at 2:45 AM, Greg Kroah-Hartman
> >  wrote:
> > > On Thu, Sep 07, 2017 at 06:22:13PM -0700, Badhri Jagan Sridharan wrote:
> > >> The source and sink caps should follow the following rules.
> > >> This patch validates whether the src_caps/snk_caps adheres
> > >> to it.
> > >>
> > >> 6.4.1 Capabilities Message
> > >> A Capabilities message (Source Capabilities message or Sink
> > >> Capabilities message) shall have at least one Power
> > >> Data Object for vSafe5V. The Capabilities message shall also
> > >> contain the sending Port’s information followed by up to
> > >> 6 additional Power Data Objects. Power Data Objects in a
> > >> Capabilities message shall be sent in the following order:
> > >>
> > >> 1. The vSafe5V Fixed Supply Object shall always be the first object.
> > >> 2. The remaining Fixed Supply Objects, if present, shall be sent
> > >>in voltage order; lowest to highest.
> > >> 3. The Battery Supply Objects, if present shall be sent in Minimum
> > >>Voltage order; lowest to highest.
> > >> 4. The Variable Supply (non-battery) Objects, if present, shall be
> > >>sent in Minimum Voltage order; lowest to highest.
> > >>
> > >> Errors in source/sink_caps of the local port will prevent
> > >> the port registration. Whereas, errors in source caps of partner
> > >> device would only log them.
> > >>
> > >> Signed-off-by: Badhri Jagan Sridharan 
> > >> ---
> > >>  drivers/staging/typec/pd.h   |   2 +
> > >>  drivers/staging/typec/tcpm.c | 107 
> > >> +++
> > >>  drivers/staging/typec/tcpm.h |  16 +++
> > >>  3 files changed, 108 insertions(+), 17 deletions(-)
> > >
> > > Before you add more stuff to this driver, what is needed to get it out
> > > of staging?  That would be more useful to do now, right?
> > 
> > Actually not adding more features here. There was a bug where
> > the phone wasnt charging at the highest possible power output,
> > came up with the these patches while debugging that issue.
> 
> Ok, but again, when is this going to get out of staging?

Code wise nothing. Add documentation to data structures, eliminate dead code,
drop and left over FIXME / TODO (those are not substantial enough to retain).

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] staging: typec: tcpm: Only request matching pdos

[Guenter Roeck]

On Sat, Sep 09, 2017 at 11:48:48AM +0300, Dan Carpenter wrote:
> On Fri, Sep 08, 2017 at 10:43:40AM -0700, Badhri Jagan Sridharan wrote:
> > >>   /*
> > >> -  * Select the source PDO providing the most power while staying 
> > >> within
> > >> -  * the board's voltage limits. Prefer PDO providing exp
> > >> +  * Select the source PDO providing the most power which has a
> > >> +  * matchig sink cap. Prefer PDO providing exp
> > >^^^  ^
> > > "matching".  What does "providing exp" mean?
> > 
> > Actually that was moved forward from the existing code.
> > So not sure what that means ? I can remove it, if needed.
> > 
> 
> If I can't understand it, it probably means that I don't have any
> background knowledge, but if you can't understand it then probably no
> one can.  :P  Feel free to delete.
> 
Agreed. I don't recall what it means either ;-). Most likely something
got lost.

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/11] mux: consumer.h: Add MUX_USB_* state constant defines

[Guenter Roeck]

On Sat, Sep 02, 2017 at 05:59:14PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 02-09-17 16:59, Guenter Roeck wrote:
> >On 09/01/2017 02:48 PM, Hans de Goede wrote:
> >>Add MUX_USB_* state constant defines, which can be used by USB
> >>device/host and Type-C polarity/role/altmode mux drivers and consumers
> >>to ensure that they agree on the meaning of the mux_control_select()
> >>state argument.
> >>
> >>Signed-off-by: Hans de Goede <hdego...@redhat.com>
> >>---
> >>  include/linux/mux/consumer.h | 16 
> >>  1 file changed, 16 insertions(+)
> >>
> >>diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
> >>index 912dd48a3a5d..e3ec9b4db962 100644
> >>--- a/include/linux/mux/consumer.h
> >>+++ b/include/linux/mux/consumer.h
> >>@@ -15,6 +15,22 @@
> >>  #include 
> >>+/*
> >>+ * Mux state values for USB muxes, used for both USB device/host role muxes
> >>+ * as well as for Type-C polarity/role/altmode muxes.
> >>+ *
> >>+ * MUX_USB_POLARITY_INV may be or-ed together with any other mux-state as
> >>+ * inverted-polarity (Type-C plugged in upside down) can happen with any
> >>+ * other mux-state.
> >>+ */
> >>+#define MUX_USB_POLARITY_INVBIT(0)   /* Polarity inverted bit */
> >>+#define MUX_USB_NONE(1 << 1) /* Mux open / not connected */
> >
> >
> >Why BIT(0) but (1 << 1) and so on ?
> 
> Because the polarity can be or-ed together with any of the other options.
> Each option can be selected normal and inverted polarity (connector
> inserted upside down).
> 
Ah yes, it is (2 << 1), not (1 << 2). But then why don't the options start
with (0 << 1) ?

I'll have to look into the series more closely; so far the polarity was
a separate parameter to tcpm_mux_set() and the low level API.

Guenter

> Regards,
> 
> Hans
> 
> >>+#define MUX_USB_DEVICE(2 << 1) /* USB device mode */
> >>+#define MUX_USB_HOST(3 << 1) /* USB host mode */
> >>+#define MUX_USB_HOST_AND_DP_SRC(4 << 1) /* USB host + 2 lanes Display 
> >>Port */
> >>+#define MUX_USB_DP_SRC(5 << 1) /* 4 lanes Display Port source */
> >>+#define MUX_USB_STATES(6 << 1)
> >>+
> >>  struct device;
> >>  struct mux_control;
> >>
> >
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/11] mux: consumer.h: Add MUX_USB_* state constant defines

[Guenter Roeck]

On 09/01/2017 02:48 PM, Hans de Goede wrote:

Add MUX_USB_* state constant defines, which can be used by USB
device/host and Type-C polarity/role/altmode mux drivers and consumers
to ensure that they agree on the meaning of the mux_control_select()
state argument.

Signed-off-by: Hans de Goede 
---
  include/linux/mux/consumer.h | 16 
  1 file changed, 16 insertions(+)

diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index 912dd48a3a5d..e3ec9b4db962 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -15,6 +15,22 @@
  
  #include 
  
+/*

+ * Mux state values for USB muxes, used for both USB device/host role muxes
+ * as well as for Type-C polarity/role/altmode muxes.
+ *
+ * MUX_USB_POLARITY_INV may be or-ed together with any other mux-state as
+ * inverted-polarity (Type-C plugged in upside down) can happen with any
+ * other mux-state.
+ */
+#define MUX_USB_POLARITY_INV   BIT(0)   /* Polarity inverted bit */
+#define MUX_USB_NONE   (1 << 1) /* Mux open / not connected */



Why BIT(0) but (1 << 1) and so on ?

Guenter



+#define MUX_USB_DEVICE (2 << 1) /* USB device mode */
+#define MUX_USB_HOST   (3 << 1) /* USB host mode */
+#define MUX_USB_HOST_AND_DP_SRC(4 << 1) /* USB host + 2 lanes Display 
Port */
+#define MUX_USB_DP_SRC (5 << 1) /* 4 lanes Display Port source */
+#define MUX_USB_STATES (6 << 1)
+
  struct device;
  struct mux_control;
  



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH][staging-next] staging: typec: fusb302: make structure fusb302_psy_desc static

[Guenter Roeck]

On Fri, Sep 01, 2017 at 11:20:57AM +0100, Colin King wrote:
> From: Colin Ian King <colin.k...@canonical.com>
> 
> The const structure fusb302_psy_desc is local to the source and
> does not need to be in global scope, so make it static.
> 
> Cleans up sparse warnings
> symbol 'fusb302_psy_desc' was not declared. Should it be static?
> 
> Signed-off-by: Colin Ian King <colin.k...@canonical.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/fusb302/fusb302.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/typec/fusb302/fusb302.c 
> b/drivers/staging/typec/fusb302/fusb302.c
> index cf6355f59cd9..fc6a3cf74eb3 100644
> --- a/drivers/staging/typec/fusb302/fusb302.c
> +++ b/drivers/staging/typec/fusb302/fusb302.c
> @@ -1723,7 +1723,7 @@ static enum power_supply_property 
> fusb302_psy_properties[] = {
>   POWER_SUPPLY_PROP_CURRENT_MAX,
>  };
>  
> -const struct power_supply_desc fusb302_psy_desc = {
> +static const struct power_supply_desc fusb302_psy_desc = {
>   .name   = "fusb302-typec-source",
>   .type   = POWER_SUPPLY_TYPE_USB_TYPE_C,
>   .properties = fusb302_psy_properties,
> -- 
> 2.14.1
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 07/11] staging: typec: fusb302: Export current-limit through a power_supply class dev

[Guenter Roeck]

On 08/30/2017 02:48 AM, Hans de Goede wrote:

The fusb302 Type-C port-controller cannot control the current-limit
directly, so we need to exported the limit so that another driver
(e.g. the charger driver) can pick the limit up and configure the
system accordingly.

The power-supply subsys already provides infrastructure for this,
power-supply devices have the notion of being supplied by another
power-supply and have properties through which we can export the
current-limit.

Register a power_supply and export the current-limit through the
power_supply's current-max property.

Cc: "Yueyao (Nathan) Zhu" <yue...@google.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>


Reviewed-by: Guenter Roeck <li...@roeck-us.net>


---
Changes in v2:
-Put the psy class device code directly in fusb302.c rather then introducing
  helpers which are only used by fusb302.c
-Add an online property to the psy so that upower does not mistake it for a
  second battery in the system
---
  drivers/staging/typec/fusb302/Kconfig   |  2 +-
  drivers/staging/typec/fusb302/fusb302.c | 63 +++--
  2 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/typec/fusb302/Kconfig 
b/drivers/staging/typec/fusb302/Kconfig
index fce099ff39fe..48a4f2fcee03 100644
--- a/drivers/staging/typec/fusb302/Kconfig
+++ b/drivers/staging/typec/fusb302/Kconfig
@@ -1,6 +1,6 @@
  config TYPEC_FUSB302
tristate "Fairchild FUSB302 Type-C chip driver"
-   depends on I2C
+   depends on I2C && POWER_SUPPLY
help
  The Fairchild FUSB302 Type-C chip driver that works with
  Type-C Port Controller Manager to provide USB PD and USB
diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index 6f007f66d597..cf6355f59cd9 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -28,6 +28,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -108,6 +109,11 @@ struct fusb302_chip {
/* lock for sharing chip states */
struct mutex lock;
  
+	/* psy + psy status */

+   struct power_supply *psy;
+   u32 current_limit;
+   u32 supply_voltage;
+
/* chip status */
enum toggling_mode toggling_mode;
enum src_current_status src_current_status;
@@ -876,11 +882,13 @@ static int tcpm_set_vbus(struct tcpc_dev *dev, bool on, 
bool charge)
chip->vbus_on = on;
fusb302_log(chip, "vbus := %s", on ? "On" : "Off");
}
-   if (chip->charge_on == charge)
+   if (chip->charge_on == charge) {
fusb302_log(chip, "charge is already %s",
charge ? "On" : "Off");
-   else
+   } else {
chip->charge_on = charge;
+   power_supply_changed(chip->psy);
+   }
  
  done:

mutex_unlock(>lock);
@@ -896,6 +904,11 @@ static int tcpm_set_current_limit(struct tcpc_dev *dev, 
u32 max_ma, u32 mv)
fusb302_log(chip, "current limit: %d ma, %d mv (not implemented)",
max_ma, mv);
  
+	chip->supply_voltage = mv;

+   chip->current_limit = max_ma;
+
+   power_supply_changed(chip->psy);
+
return 0;
  }
  
@@ -1681,6 +1694,43 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id)

return IRQ_HANDLED;
  }
  
+static int fusb302_psy_get_property(struct power_supply *psy,

+   enum power_supply_property psp,
+   union power_supply_propval *val)
+{
+   struct fusb302_chip *chip = power_supply_get_drvdata(psy);
+
+   switch (psp) {
+   case POWER_SUPPLY_PROP_ONLINE:
+   val->intval = chip->charge_on;
+   break;
+   case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+   val->intval = chip->supply_voltage * 1000; /* mV -> µV */
+   break;
+   case POWER_SUPPLY_PROP_CURRENT_MAX:
+   val->intval = chip->current_limit * 1000; /* mA -> µA */
+   break;
+   default:
+   return -ENODATA;
+   }
+
+   return 0;
+}
+
+static enum power_supply_property fusb302_psy_properties[] = {
+   POWER_SUPPLY_PROP_ONLINE,
+   POWER_SUPPLY_PROP_VOLTAGE_NOW,
+   POWER_SUPPLY_PROP_CURRENT_MAX,
+};
+
+const struct power_supply_desc fusb302_psy_desc = {
+   .name   = "fusb302-typec-source",
+   .type   = POWER_SUPPLY_TYPE_USB_TYPE_C,
+   .properties = fusb302_psy_properties,
+   .num_properties = ARRAY_SIZE(fusb302_psy_properties),
+   .get_property   = fusb302_psy_get_property,
+};
+
  static int init_gpio(struct fusb302_chip *chip)
  {
struct device_node *node;
@@ -1720,6 +1770,7 @@ static int fusb302_p

Re: [PATCH v3 06/11] staging: typec: fusb302: Add support for USB2 charger detection through extcon

[Guenter Roeck]

On 08/30/2017 02:48 AM, Hans de Goede wrote:

The fusb302 port-controller relies on an external device doing USB2
charger-type detection.

The Intel Whiskey Cove PMIC with which the fusb302 is combined on some
X86/ACPI platforms already has a charger-type detection driver which
uses extcon to communicate the detected charger-type.

Rather then inventing a new API for USB2 charger-type detection
specifically for use with the tcpm code, this commit simply re-uses the
existing extcon API and uses that do USB2 charger detection.

Note that the "fcs,extcon-name" property name is only for kernel internal
use by X86/ACPI platform code and as such is NOT documented in
the fusb302 devicetree bindings.

Cc: "Yueyao (Nathan) Zhu" <yue...@google.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>


Reviewed-by: Guenter Roeck <li...@roeck-us.net>


---
Changes in v2:
-Put extcon code directly in fusb302.c rather then introducing helpers
  which are only used by fusb302.c

Changes in v3:
-Improve commit message
---
  drivers/staging/typec/fusb302/fusb302.c | 49 +
  1 file changed, 49 insertions(+)

diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index 675161cf4f3a..6f007f66d597 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -17,6 +17,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -96,6 +97,7 @@ struct fusb302_chip {
  
  	int gpio_int_n;

int gpio_int_n_irq;
+   struct extcon_dev *extcon;
  
  	struct workqueue_struct *wq;

struct delayed_work bc_lvl_handler;
@@ -516,6 +518,38 @@ static int tcpm_get_vbus(struct tcpc_dev *dev)
return ret;
  }
  
+static int tcpm_get_current_limit(struct tcpc_dev *dev)

+{
+   struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
+tcpc_dev);
+   int current_limit = 0;
+   unsigned long timeout;
+
+   if (!chip->extcon)
+   return 0;
+
+   /*
+* USB2 Charger detection may still be in progress when we get here,
+* this can take upto 600ms, wait 800ms max.
+*/
+   timeout = jiffies + msecs_to_jiffies(800);
+   do {
+   if (extcon_get_state(chip->extcon, EXTCON_CHG_USB_SDP) == 1)
+   current_limit = 500;
+
+   if (extcon_get_state(chip->extcon, EXTCON_CHG_USB_CDP) == 1 ||
+   extcon_get_state(chip->extcon, EXTCON_CHG_USB_ACA) == 1)
+   current_limit = 1500;
+
+   if (extcon_get_state(chip->extcon, EXTCON_CHG_USB_DCP) == 1)
+   current_limit = 2000;
+
+   msleep(50);
+   } while (current_limit == 0 && time_before(jiffies, timeout));
+
+   return current_limit;
+}
+
  static int fusb302_set_cc_pull(struct fusb302_chip *chip,
   bool pull_up, bool pull_down)
  {
@@ -1201,6 +1235,7 @@ static void init_tcpc_dev(struct tcpc_dev 
*fusb302_tcpc_dev)
  {
fusb302_tcpc_dev->init = tcpm_init;
fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
+   fusb302_tcpc_dev->get_current_limit = tcpm_get_current_limit;
fusb302_tcpc_dev->set_cc = tcpm_set_cc;
fusb302_tcpc_dev->get_cc = tcpm_get_cc;
fusb302_tcpc_dev->set_polarity = tcpm_set_polarity;
@@ -1685,6 +1720,7 @@ static int fusb302_probe(struct i2c_client *client,
struct fusb302_chip *chip;
struct i2c_adapter *adapter;
struct device *dev = >dev;
+   const char *name;
int ret = 0;
u32 v;
  
@@ -1717,6 +1753,19 @@ static int fusb302_probe(struct i2c_client *client,

if (!device_property_read_u32(dev, "fcs,operating-sink-microwatt", ))
chip->tcpc_config.operating_snk_mw = v / 1000;
  
+	/*

+* Devicetree platforms should get extcon via phandle (not yet
+* supported). On ACPI platforms, we get the name from a device prop.
+* This device prop is for kernel internal use only and is expected
+* to be set by the platform code which also registers the i2c client
+* for the fusb302.
+*/
+   if (device_property_read_string(dev, "fcs,extcon-name", ) == 0) {
+   chip->extcon = extcon_get_extcon_dev(name);
+   if (!chip->extcon)
+   return -EPROBE_DEFER;
+   }
+
ret = fusb302_debugfs_init(chip);
if (ret < 0)
return ret;



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 05/11] staging: typec: fusb302: Use client->irq as irq if set

[Guenter Roeck]

On 08/30/2017 02:48 AM, Hans de Goede wrote:

The fusb302 is also used on x86 systems where the platform code sets
the irq in client->irq and there is no gpio named fcs,int_n.

Cc: "Yueyao (Nathan) Zhu" <yue...@google.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>


Reviewed-by: Guenter Roeck <li...@roeck-us.net>


---
  drivers/staging/typec/fusb302/fusb302.c | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index 1c1751c994db..675161cf4f3a 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1735,9 +1735,13 @@ static int fusb302_probe(struct i2c_client *client,
goto destroy_workqueue;
}
  
-	ret = init_gpio(chip);

-   if (ret < 0)
-   goto destroy_workqueue;
+   if (client->irq) {
+   chip->gpio_int_n_irq = client->irq;
+   } else {
+   ret = init_gpio(chip);
+   if (ret < 0)
+   goto destroy_workqueue;
+   }
  
  	chip->tcpm_port = tcpm_register_port(>dev, >tcpc_dev);

if (IS_ERR(chip->tcpm_port)) {



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 04/11] staging: typec: fusb302: Get max snk mv/ma/mw from device-properties

[Guenter Roeck]

On 08/30/2017 02:48 AM, Hans de Goede wrote:

This is board specific info so it should come from board config, such
as devicetree.

I've chosen to prefix these with "fcs," treating them as fusb302 driver
specific for now. We may want to revisit this and replace these with
properties which are part of a (to be written) generic type-c controller
devicetree binding.

Since this commit adds new dt-properties it also adds devicetree-bindings
documentation (which so far was absent for the fusb302 driver).

Cc: Rob Herring <robh...@kernel.org>
Cc: Frank Rowand <frowand.l...@gmail.com>
Cc: devicet...@vger.kernel.org
Cc: "Yueyao (Nathan) Zhu" <yue...@google.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
Acked-by: Rob Herring <r...@kernel.org>


Reviewed-by: Guenter Roeck <li...@roeck-us.net>


---
Changes in v2:
-Use micro... instead of mili...
-Add devicetree bindings documentation

Changes in v3:
-Use sink rather then snk in property names
-Add Rob's Acked-by
---
  .../devicetree/bindings/usb/fcs,fusb302.txt| 29 ++
  drivers/staging/typec/fusb302/TODO |  4 +++
  drivers/staging/typec/fusb302/fusb302.c| 18 +-
  3 files changed, 50 insertions(+), 1 deletion(-)
  create mode 100644 Documentation/devicetree/bindings/usb/fcs,fusb302.txt

diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt 
b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
new file mode 100644
index ..472facfa5a71
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
@@ -0,0 +1,29 @@
+Fairchild FUSB302 Type-C Port controllers
+
+Required properties :
+- compatible : "fcs,fusb302"
+- reg: I2C slave address
+- interrupts : Interrupt specifier
+
+Optional properties :
+- fcs,max-sink-microvolt : Maximum voltage to negotiate when configured as sink
+- fcs,max-sink-microamp  : Maximum current to negotiate when configured as sink
+- fcs,max-sink-microwatt : Maximum power to negotiate when configured as sink
+  If this is less then max-sink-microvolt *
+  max-sink-microamp then the configured current will
+  be clamped.
+- fcs,operating-sink-microwatt :
+  Minimum amount of power accepted from a sink
+  when negotiating
+
+Example:
+
+fusb302: typec-portc@54 {
+   compatible = "fcs,fusb302";
+   reg = <0x54>;
+   interrupt-parent = <_intc>;
+   interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+   fcs,max-sink-microvolt = <1200>;
+   fcs,max-sink-microamp = <300>;
+   fcs,max-sink-microwatt = <3600>;
+};
diff --git a/drivers/staging/typec/fusb302/TODO 
b/drivers/staging/typec/fusb302/TODO
index 4933a1d92c32..19b466eb585d 100644
--- a/drivers/staging/typec/fusb302/TODO
+++ b/drivers/staging/typec/fusb302/TODO
@@ -4,3 +4,7 @@ fusb302:
  - Find a non-hacky way to coordinate between PM and I2C access
  - Documentation? The FUSB302 datasheet provides information on the chip to 
help
understand the code. But it may still be helpful to have a documentation.
+- We may want to replace the  "fcs,max-snk-microvolt", "fcs,max-snk-microamp",
+  "fcs,max-snk-microwatt" and "fcs,operating-snk-microwatt" device(tree)
+  properties with properties which are part of a generic type-c controller
+  devicetree binding.
diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index 6baed06a3c0d..1c1751c994db 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -90,6 +90,7 @@ struct fusb302_chip {
struct i2c_client *i2c_client;
struct tcpm_port *tcpm_port;
struct tcpc_dev tcpc_dev;
+   struct tcpc_config tcpc_config;
  
  	struct regulator *vbus;
  
@@ -1198,7 +1199,6 @@ static const struct tcpc_config fusb302_tcpc_config = {
  
  static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)

  {
-   fusb302_tcpc_dev->config = _tcpc_config;
fusb302_tcpc_dev->init = tcpm_init;
fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
fusb302_tcpc_dev->set_cc = tcpm_set_cc;
@@ -1684,7 +1684,9 @@ static int fusb302_probe(struct i2c_client *client,
  {
struct fusb302_chip *chip;
struct i2c_adapter *adapter;
+   struct device *dev = >dev;
int ret = 0;
+   u32 v;
  
  	adapter = to_i2c_adapter(client->dev.parent);

if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
@@ -1699,8 +1701,22 @@ static int fusb302_probe(struct i2c_client *client,
chip->i2c_client = client;
i2c_set_clientdata(client, chip);
chip->dev = >dev;
+   chip->tcpc_config = fusb302_tcpc_config;
+   chip->

Re: [PATCH v3 03/11] staging: typec: fusb302: Set max supply voltage to 5V

[Guenter Roeck]

On 08/30/2017 02:48 AM, Hans de Goede wrote:

Anything higher then 5V may damage hardware not capable of it, so
the only sane default here is 5V. If a board is able to handle a
higher voltage that should come from board specific data such as
device-tree and not be hard coded into the fusb302 code.

Cc: "Yueyao (Nathan) Zhu" <yue...@google.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>


Reviewed-by: Guenter Roeck <li...@roeck-us.net>


---
  drivers/staging/typec/fusb302/fusb302.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index 03a3809d18f0..6baed06a3c0d 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1187,9 +1187,9 @@ static const struct tcpc_config fusb302_tcpc_config = {
.nr_src_pdo = ARRAY_SIZE(src_pdo),
.snk_pdo = snk_pdo,
.nr_snk_pdo = ARRAY_SIZE(snk_pdo),
-   .max_snk_mv = 9000,
+   .max_snk_mv = 5000,
.max_snk_ma = 3000,
-   .max_snk_mw = 27000,
+   .max_snk_mw = 15000,
.operating_snk_mw = 2500,
.type = TYPEC_PORT_DRP,
.default_role = TYPEC_SINK,



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/6] staging: usb: typec: tcpm set port type callback

[Guenter Roeck]

On 08/27/2017 11:01 AM, Greg Kroah-Hartman wrote:

On Sat, Aug 26, 2017 at 10:23:24PM -0700, Badhri Jagan Sridharan wrote:

The port type callback call enquires the tcpc_dev if
the requested port type is supported. If supported, then
performs a tcpm reset if required after setting the tcpm
internal port_type variable.

Check against the tcpm port_type instead of checking
against caps.type as port_type reflects the current
configuration.

Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>
Reviewed-by: Guenter Roeck <li...@roeck-us.net>
---
  drivers/staging/typec/tcpm.c | 52 ++--
  1 file changed, 41 insertions(+), 11 deletions(-)


This series is really messed up.  I see patches out of 6 and out of 11,
and none of it "threaded" so I don't know what is what to apply :(

Please resend the whole series, correctly, with Guenter's reviewed-by,
so I know what to apply and in what order.



Agreed, I got confused a bit as well. I think Badhri resent patches 1..6
as part of the 1..11 series and marked those as v2, but he did not mark
patches 7..11 as v2.

Badhri, please mark all patches as v3 and indicate the reason in the
changelog (the reason being to add my Reviewed-by: tag and to fix patch
sequence/version numbers). In general, if you add a patch to a series,
please mark the entire series with the same version and provide a changelog
entry indicating that the patch was added in this version.

Thanks,
Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 11/11] staging: typec: tcpm: Switch to PORT_RESET instead of SNK_UNATTACHED

[Guenter Roeck]

On Sat, Aug 26, 2017 at 11:52:33PM -0700, Badhri Jagan Sridharan wrote:
> When VBUS is not discovered within PD_T_PS_SOURCE_ON although Rp
> is detected on CC, TCPM switches the port to SNK_UNATTACHED
> state. SNK_UNATTACHED, however does not force TYPEC_CC_OPEN which
> makes the partner(source) to think that it is connected.
> 
> To overcome this issue, force the port into PORT_RESET state
> to make sure the CC lines are open.
> 
> Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/tcpm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index 9e0111dea7c4..47b8fec5ea36 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -2380,7 +2380,7 @@ static void run_state_machine(struct tcpm_port *port)
>  0);
>   else
>   /* Wait for VBUS, but not forever */
> - tcpm_set_state(port, SNK_UNATTACHED, PD_T_PS_SOURCE_ON);
> + tcpm_set_state(port, PORT_RESET, PD_T_PS_SOURCE_ON);
>   break;
>  
>   case SRC_TRY:
> -- 
> 2.14.1.342.g6490525c54-goog
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 10/11] staging: typec: tcpm: Do not send PING msgs in TCPM

[Guenter Roeck]

On Sat, Aug 26, 2017 at 11:52:00PM -0700, Badhri Jagan Sridharan wrote:
> PING messages are used to monitor the connect/disconnect.
> However, when PD is carried over CC, so this is not required.
> 
> Also, the spec does not clearly say if PD is possible when
> Type-c is connected to Type-A/B. So, removing sending
> PING messages altogether.
> 
> Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/tcpm.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index a7da609006f5..9e0111dea7c4 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -2335,14 +2335,11 @@ static void run_state_machine(struct tcpm_port *port)
>* - The system is not operating in PD mode
>* or
>* - Both partners are connected using a Type-C connector
> -  *   XXX How do we know that ?
> +  *
> +  * There is no actual need to send PD messages since the local
> +  * port type-c and the spec does not clearly say whether PD is
> +  * possible when type-c is connected to Type-A/B
>*/
> - if (port->pwr_opmode == TYPEC_PWR_MODE_PD &&
> - !port->op_vsafe5v) {
> - tcpm_pd_send_control(port, PD_CTRL_PING);
> - tcpm_set_state_cond(port, SRC_READY,
> - PD_T_SOURCE_ACTIVITY);
> - }
>   break;
>   case SRC_WAIT_NEW_CAPABILITIES:
>   /* Nothing to do... */
> -- 
> 2.14.1.342.g6490525c54-goog
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 09/11] staging: typec: tcpm: typec: tcpm: Wait for CC debounce before PD excg

[Guenter Roeck]

On Sat, Aug 26, 2017 at 11:51:27PM -0700, Badhri Jagan Sridharan wrote:
> Once, Rp or Rd is switched, wait for PD_T_CC_DEBOUNCE. If not the
> PS_RDY message transmitted might result in failure.
> Also, Only wait for PD_T_SRCSWAPSTDBY while in
> PR_SWAP_SRC_SNK_TRANSITION_OFF. PD_T_PS_SOURCE_OFF is the overall
> time after which the initial sink would issue hard reset.
> 
> Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/pd.h   |  2 ++
>  drivers/staging/typec/tcpm.c | 25 ++---
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/typec/pd.h b/drivers/staging/typec/pd.h
> index 510ef7279900..30b32ad72acd 100644
> --- a/drivers/staging/typec/pd.h
> +++ b/drivers/staging/typec/pd.h
> @@ -278,6 +278,8 @@ static inline unsigned int rdo_max_power(u32 rdo)
>  #define PD_T_VCONN_SOURCE_ON 100
>  #define PD_T_SINK_REQUEST100 /* 100 ms minimum */
>  #define PD_T_ERROR_RECOVERY  100 /* minimum 25 is insufficient */
> +#define PD_T_SRCSWAPSTDBY  625 /* Maximum of 650ms */
> +#define PD_T_NEWSRC250 /* Maximum of 275ms */
>  
>  #define PD_T_DRP_TRY 100 /* 75 - 150 ms */
>  #define PD_T_DRP_TRYWAIT 600 /* 400 - 800 ms */
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index 1f6827f32b29..a7da609006f5 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -90,9 +90,11 @@
>   S(PR_SWAP_START),   \
>   S(PR_SWAP_SRC_SNK_TRANSITION_OFF),  \
>   S(PR_SWAP_SRC_SNK_SOURCE_OFF),  \
> + S(PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED), \
>   S(PR_SWAP_SRC_SNK_SINK_ON), \
>   S(PR_SWAP_SNK_SRC_SINK_OFF),\
>   S(PR_SWAP_SNK_SRC_SOURCE_ON),   \
> + S(PR_SWAP_SNK_SRC_SOURCE_ON_VBUS_RAMPED_UP),\
>   \
>   S(VCONN_SWAP_ACCEPT),   \
>   S(VCONN_SWAP_SEND), \
> @@ -1395,7 +1397,7 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>  SNK_TRANSITION_SINK_VBUS, 0);
>   }
>   break;
> - case PR_SWAP_SRC_SNK_SOURCE_OFF:
> + case PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED:
>   tcpm_set_state(port, PR_SWAP_SRC_SNK_SINK_ON, 0);
>   break;
>   case PR_SWAP_SNK_SRC_SINK_OFF:
> @@ -2679,11 +2681,17 @@ static void run_state_machine(struct tcpm_port *port)
>   case PR_SWAP_SRC_SNK_TRANSITION_OFF:
>   tcpm_set_vbus(port, false);
>   port->explicit_contract = false;
> + /* allow time for Vbus discharge, must be < tSrcSwapStdby */
>   tcpm_set_state(port, PR_SWAP_SRC_SNK_SOURCE_OFF,
> -PD_T_PS_SOURCE_OFF);
> +PD_T_SRCSWAPSTDBY);
>   break;
>   case PR_SWAP_SRC_SNK_SOURCE_OFF:
>   tcpm_set_cc(port, TYPEC_CC_RD);
> + /* allow CC debounce */
> + tcpm_set_state(port, PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED,
> +PD_T_CC_DEBOUNCE);
> + break;
> + case PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED:
>   /*
>* USB-PD standard, 6.2.1.4, Port Power Role:
>* "During the Power Role Swap Sequence, for the initial Source
> @@ -2709,6 +2717,15 @@ static void run_state_machine(struct tcpm_port *port)
>   case PR_SWAP_SNK_SRC_SOURCE_ON:
>   tcpm_set_cc(port, tcpm_rp_cc(port));
>   tcpm_set_vbus(port, true);
> + /*
> +  * allow time VBUS ramp-up, must be < tNewSrc
> +  * Also, this window overlaps with CC debounce as well.
> +  * So, Wait for the max of two which is PD_T_NEWSRC
> +  */
> + tcpm_set_state(port, PR_SWAP_SNK_SRC_SOURCE_ON_VBUS_RAMPED_UP,
> +PD_T_NEWSRC);
> + break;
> + case PR_SWAP_SNK_SRC_SOURCE_ON_VBUS_RAMPED_UP:
>   /*
>* USB PD standard, 6.2.1.4:
>* "Subsequent Messages initiated by the Policy Engine,
> @@ -2979,8 +2996,10 @@ static void _tcpm_cc_change(struct tcpm_port *port, 
> enum typec_cc_status cc1,
>   case PR_SWAP_SNK_SRC_SINK_OFF:
>   case PR_SWAP_SRC_SNK_TRANSITION_OFF:
>   case PR_SWAP_SRC_SNK_SOURCE_OFF:
> + case PR_SWAP_SRC_SNK_SOURCE_OFF_CC_DEBOUNCED:
> + case PR_SWAP_SNK_SRC_SOURCE_ON:
>  

Re: [PATCH 08/11] staging: typec: tcpm: add cc change handling in src states

[Guenter Roeck]

On Sat, Aug 26, 2017 at 11:50:31PM -0700, Badhri Jagan Sridharan wrote:
> In the case that the lower layer driver reports a cc change directly
> from SINK state to SOURCE state, TCPM doesn't handle these cc change
> in SRC_SEND_CAPABILITIES, SRC_READY states. And with SRC_ATTACHED
> state, the change is not handled as the port is still considered
> connected.
> 
> [49606.131672] state change DRP_TOGGLING -> SRC_ATTACH_WAIT
> [49606.131701] pending state change SRC_ATTACH_WAIT -> SRC_ATTACHED @
> 200 ms
> [49606.329952] state change SRC_ATTACH_WAIT -> SRC_ATTACHED [delayed 200
> ms]
> [49606.329978] polarity 0
> [49606.329989] Requesting mux mode 1, config 0, polarity 0
> [49606.349416] vbus:=1 charge=0
> [49606.372274] pending state change SRC_ATTACHED -> SRC_UNATTACHED @ 480
> ms
> [49606.372431] VBUS on
> [49606.372488] state change SRC_ATTACHED -> SRC_STARTUP
> ...
> (the lower layer driver reports a direct change from source to sink)
> [49606.536927] pending state change SRC_SEND_CAPABILITIES ->
> SRC_SEND_CAPABILITIES @ 150 ms
> [49606.547244] CC1: 2 -> 5, CC2: 0 -> 0 [state SRC_SEND_CAPABILITIES,
> polarity 0, connected]
> 
> This can happen when the lower layer driver and/or the hardware
> handles a portion of the Type-C state machine work, and quietly goes
> through the unattached state.
> 
> Originally-from: Yueyao Zhu <yue...@google.com>
> Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/tcpm.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index 645f43ee83df..1f6827f32b29 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -2874,10 +2874,12 @@ static void _tcpm_cc_change(struct tcpm_port *port, 
> enum typec_cc_status cc1,
>   tcpm_set_state(port, SRC_ATTACH_WAIT, 0);
>   break;
>   case SRC_ATTACHED:
> - if (tcpm_port_is_disconnected(port))
> + case SRC_SEND_CAPABILITIES:
> + case SRC_READY:
> + if (tcpm_port_is_disconnected(port) ||
> + !tcpm_port_is_source(port))
>   tcpm_set_state(port, SRC_UNATTACHED, 0);
>   break;
> -
>   case SNK_UNATTACHED:
>   if (tcpm_port_is_sink(port))
>   tcpm_set_state(port, SNK_ATTACH_WAIT, 0);
> -- 
> 2.14.1.342.g6490525c54-goog
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 06/11 v2] staging: typec: tcpm: Comply with TryWait.SNK State

[Guenter Roeck]

On Sat, Aug 26, 2017 at 11:49:21PM -0700, Badhri Jagan Sridharan wrote:
> According to the spec:
> "4.5.2.2.10.2 Exiting from TryWait.SNK State
> The port shall transition to Attached.SNK after tCCDebounce if or when VBUS
> is detected. Note the Source may initiate USB PD communications which will
> cause brief periods of the SNK.Open state on both the CC1 and CC2 pins,
> but this event will not exceed tPDDebounce. The port shall transition to
> Unattached.SNK when the state of both of the CC1 and CC2 pins is SNK.Open
> for at least tPDDebounce."
> 
> Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>
> ---
> Changelog since v1:
> - Corrected  tag
> 
For v2 up to here:

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

>  drivers/staging/typec/tcpm.c | 58 
> +++-
>  1 file changed, 30 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index fc179bdea7e4..0ae0a5c7 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -2403,26 +2403,24 @@ static void run_state_machine(struct tcpm_port *port)
>   break;
>   case SNK_TRYWAIT:
>   tcpm_set_cc(port, TYPEC_CC_RD);
> - tcpm_set_state(port, SNK_TRYWAIT_DEBOUNCE, PD_T_CC_DEBOUNCE);
> + tcpm_set_state(port, SNK_TRYWAIT_VBUS, PD_T_CC_DEBOUNCE);
>   break;
> - case SNK_TRYWAIT_DEBOUNCE:
> - if (port->vbus_present) {
> + case SNK_TRYWAIT_VBUS:
> + /*
> +  * TCPM stays in this state indefinitely until VBUS
> +  * is detected as long as Rp is not detected for
> +  * more than a time period of tPDDebounce.
> +  */
> + if (port->vbus_present && tcpm_port_is_sink(port)) {
>   tcpm_set_state(port, SNK_ATTACHED, 0);
>   break;
>   }
> - if (tcpm_port_is_disconnected(port)) {
> - tcpm_set_state(port, SNK_UNATTACHED,
> -PD_T_PD_DEBOUNCE);
> - break;
> - }
> - if (tcpm_port_is_source(port))
> - tcpm_set_state(port, SRC_ATTACHED, 0);
> - /* XXX Are we supposed to stay in this state ? */
> + if (!tcpm_port_is_sink(port))
> + tcpm_set_state(port, SNK_TRYWAIT_DEBOUNCE, 0);
>   break;
> - case SNK_TRYWAIT_VBUS:
> - tcpm_set_state(port, SNK_ATTACHED, PD_T_CC_DEBOUNCE);
> + case SNK_TRYWAIT_DEBOUNCE:
> + tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
>   break;
> -
>   case SNK_ATTACHED:
>   ret = tcpm_snk_attach(port);
>   if (ret < 0)
> @@ -2960,19 +2958,16 @@ static void _tcpm_cc_change(struct tcpm_port *port, 
> enum typec_cc_status cc1,
>   tcpm_set_state(port, SRC_TRY_WAIT, 0);
>   break;
>   case SNK_TRYWAIT_DEBOUNCE:
> - if (port->vbus_present) {
> - tcpm_set_state(port, SNK_ATTACHED, 0);
> - break;
> - }
> - if (tcpm_port_is_source(port)) {
> - tcpm_set_state(port, SRC_ATTACHED, 0);
> - break;
> - }
> - if (tcpm_port_is_disconnected(port) &&
> - port->delayed_state != SNK_UNATTACHED)
> + if (tcpm_port_is_sink(port))
> + tcpm_set_state(port, SNK_TRYWAIT_VBUS, 0);
> + break;
> + case SNK_TRYWAIT_VBUS:
> + if (!tcpm_port_is_sink(port))
>   tcpm_set_state(port, SNK_TRYWAIT_DEBOUNCE, 0);
>   break;
> -
> + case SNK_TRYWAIT:
> + /* Do nothing, waiting for tCCDebounce */
> + break;
>   case PR_SWAP_SNK_SRC_SINK_OFF:
>   case PR_SWAP_SRC_SNK_TRANSITION_OFF:
>   case PR_SWAP_SRC_SNK_SOURCE_OFF:
> @@ -3030,7 +3025,14 @@ static void _tcpm_pd_vbus_on(struct tcpm_port *port)
>   /* Do nothing, waiting for PD_DEBOUNCE to do be done */
>   break;
>   case SNK_TRYWAIT:
> - tcpm_set_state(port, SNK_TRYWAIT_VBUS, 0);
> + /* Do nothing, waiting for tCCDebounce */
> + break;
> + case SNK_TRYWAIT_VBUS:
> + if (tcpm_port_is_sink(port))
> + tcpm_set_state(port, SNK_ATTACHED, 0);
> + break;
> + case SNK_TRYWAIT_DEBOUNCE:
> + /* Do nothing, waiting for Rp */
>   break;
>   c

Re: [PATCH 07/11] staging: typec: tcpm: Consider port_type while determining unattached_state

[Guenter Roeck]

On Sat, Aug 26, 2017 at 11:32:54PM -0700, Badhri Jagan Sridharan wrote:
> While performing PORT_RESET, upon receiving the cc disconnect
> signal from the underlaying tcpc device, TCPM transitions into
> unattached state. Consider, the current type of port while determining

stray ,; not really worth a resend.

> the unattached state.
> 
> In the below logs, although the port_type was set to sink, TCPM
> transitioned into SRC_UNATTACHED.
> 
> [  762.290654] state change SRC_READY -> PORT_RESET
> [  762.324531] Setting voltage/current limit 0 mV 0 mA
> [  762.327912] polarity 0
> [  762.334864] cc:=0
> [  762.347193] pending state change PORT_RESET -> PORT_RESET_WAIT_OFF @ 100 ms
> [  762.347200] VBUS off
> [  762.347203] CC1: 2 -> 0, CC2: 0 -> 0 [state PORT_RESET, polarity 0, 
> disconnected]
> [  762.347206] state change PORT_RESET -> SRC_UNATTACHED
> 
> Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/tcpm.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index 0ae0a5c7..645f43ee83df 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -2117,10 +2117,16 @@ static inline enum tcpm_state ready_state(struct 
> tcpm_port *port)
>  
>  static inline enum tcpm_state unattached_state(struct tcpm_port *port)
>  {
> - if (port->pwr_role == TYPEC_SOURCE)
> + if (port->port_type == TYPEC_PORT_DRP) {
> + if (port->pwr_role == TYPEC_SOURCE)
> + return SRC_UNATTACHED;
> + else
> + return SNK_UNATTACHED;
> + } else if (port->port_type == TYPEC_PORT_DFP) {
>   return SRC_UNATTACHED;
> - else
> - return SNK_UNATTACHED;
> + }
> +
> + return SNK_UNATTACHED;
>  }
>  
>  static void tcpm_check_send_discover(struct tcpm_port *port)
> -- 
> 2.14.1.342.g6490525c54-goog
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 5/6] staging: usb: tcpm: usb: typec: tcpm: Follow Try.SRC exit requirements

[Guenter Roeck]

On Sat, Aug 26, 2017 at 10:25:07PM -0700, Badhri Jagan Sridharan wrote:
> According to spec:
> " 4.5.2.2.9.2 Exiting from Try.SRC State:
> The port shall transition to Attached.SRC when the SRC.Rd
> state is detected on exactly one of the CC1 or CC2 pins for
> at least tPDDebounce. The port shall transition to
> TryWait.SNK after tDRPTry and the SRC.Rd state has not been
> detected."
> 
> Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/tcpm.c | 33 -
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index 7eed04698ebe..fc179bdea7e4 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -112,6 +112,7 @@
>   S(SRC_TRYWAIT_UNATTACHED),  \
>   \
>   S(SRC_TRY), \
> + S(SRC_TRY_WAIT),\
>   S(SRC_TRY_DEBOUNCE),\
>   S(SNK_TRYWAIT), \
>   S(SNK_TRYWAIT_DEBOUNCE),\
> @@ -2158,6 +2159,7 @@ static void run_state_machine(struct tcpm_port *port)
>  {
>   int ret;
>   enum typec_pwr_opmode opmode;
> + unsigned int msecs;
>  
>   port->enter_state = port->state;
>   switch (port->state) {
> @@ -2379,7 +2381,22 @@ static void run_state_machine(struct tcpm_port *port)
>   case SRC_TRY:
>   port->try_src_count++;
>   tcpm_set_cc(port, tcpm_rp_cc(port));
> - tcpm_set_state(port, SNK_TRYWAIT, PD_T_DRP_TRY);
> + port->max_wait = 0;
> + tcpm_set_state(port, SRC_TRY_WAIT, 0);
> + break;
> + case SRC_TRY_WAIT:
> + if (port->max_wait == 0) {
> + port->max_wait = jiffies +
> +  msecs_to_jiffies(PD_T_DRP_TRY);
> + msecs = PD_T_DRP_TRY;
> + } else {
> + if (time_is_after_jiffies(port->max_wait))
> + msecs = jiffies_to_msecs(port->max_wait -
> +  jiffies);
> + else
> + msecs = 0;
> + }
> + tcpm_set_state(port, SNK_TRYWAIT, msecs);
>   break;
>   case SRC_TRY_DEBOUNCE:
>   tcpm_set_state(port, SRC_ATTACHED, PD_T_PD_DEBOUNCE);
> @@ -2935,12 +2952,12 @@ static void _tcpm_cc_change(struct tcpm_port *port, 
> enum typec_cc_status cc1,
>   tcpm_set_state(port, SRC_TRYWAIT, 0);
>   }
>   break;
> - case SRC_TRY:
> + case SRC_TRY_WAIT:
>   if (tcpm_port_is_source(port))
>   tcpm_set_state(port, SRC_TRY_DEBOUNCE, 0);
>   break;
>   case SRC_TRY_DEBOUNCE:
> - tcpm_set_state(port, SRC_TRY, 0);
> + tcpm_set_state(port, SRC_TRY_WAIT, 0);
>   break;
>   case SNK_TRYWAIT_DEBOUNCE:
>   if (port->vbus_present) {
> @@ -3015,7 +3032,10 @@ static void _tcpm_pd_vbus_on(struct tcpm_port *port)
>   case SNK_TRYWAIT:
>   tcpm_set_state(port, SNK_TRYWAIT_VBUS, 0);
>   break;
> -
> + case SRC_TRY_WAIT:
> + case SRC_TRY_DEBOUNCE:
> + /* Do nothing, waiting for sink detection */
> + break;
>   default:
>   break;
>   }
> @@ -3069,7 +3089,10 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
>   case PORT_RESET_WAIT_OFF:
>   tcpm_set_state(port, tcpm_default_state(port), 0);
>   break;
> -
> + case SRC_TRY_WAIT:
> + case SRC_TRY_DEBOUNCE:
> + /* Do nothing, waiting for sink detection */
> + break;
>   default:
>   if (port->pwr_role == TYPEC_SINK &&
>   port->attached)
> -- 
> 2.14.1.342.g6490525c54-goog
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/6] staging: usb: tcpm: usb: type-c: tcpm: Check for Rp for tPDDebounce

[Guenter Roeck]

On Sat, Aug 26, 2017 at 10:24:49PM -0700, Badhri Jagan Sridharan wrote:
> According the spec, the following is the conditions for exiting Try.SNK
> state:
> "The port shall wait for tDRPTry and only then begin monitoring the CC1 and
> CC2 pins for the SNK.Rp state. The port shall then transition to
> Attached.SNK when the SNK.Rp state is detected on exactly one of the CC1
> or CC2 pins for at least tPDDebounce and V BUS is detected. Alternatively,
> the port shall transition to TryWait.SRC if SNK.Rp state is not detected
> for tPDDebounce."
> 
> Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/tcpm.c | 68 
> 
>  1 file changed, 24 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index d45ffa8f2cfd..7eed04698ebe 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -105,6 +105,8 @@
>   \
>   S(SNK_TRY), \
>   S(SNK_TRY_WAIT),\
> + S(SNK_TRY_WAIT_DEBOUNCE),   \
> + S(SNK_TRY_WAIT_DEBOUNCE_CHECK_VBUS),\
>   S(SRC_TRYWAIT), \
>   S(SRC_TRYWAIT_DEBOUNCE),\
>   S(SRC_TRYWAIT_UNATTACHED),  \
> @@ -2202,18 +2204,24 @@ static void run_state_machine(struct tcpm_port *port)
>   tcpm_set_state(port, SNK_TRY_WAIT, PD_T_DRP_TRY);
>   break;
>   case SNK_TRY_WAIT:
> + if (tcpm_port_is_sink(port)) {
> + tcpm_set_state(port, SNK_TRY_WAIT_DEBOUNCE, 0);
> + } else {
> + tcpm_set_state(port, SRC_TRYWAIT, 0);
> + port->max_wait = 0;
> + }
> + break;
> + case SNK_TRY_WAIT_DEBOUNCE:
> + tcpm_set_state(port, SNK_TRY_WAIT_DEBOUNCE_CHECK_VBUS,
> +PD_T_PD_DEBOUNCE);
> + break;
> + case SNK_TRY_WAIT_DEBOUNCE_CHECK_VBUS:
>   if (port->vbus_present && tcpm_port_is_sink(port)) {
>   tcpm_set_state(port, SNK_ATTACHED, 0);
> - break;
> - }
> - if (!tcpm_port_is_sink(port)) {
> - tcpm_set_state(port, SRC_TRYWAIT,
> -PD_T_PD_DEBOUNCE);
> + } else {
> + tcpm_set_state(port, SRC_TRYWAIT, 0);
>   port->max_wait = 0;
> - break;
>   }
> - /* No vbus, cc state is sink or open */
> - tcpm_set_state(port, SRC_TRYWAIT_UNATTACHED, PD_T_DRP_TRYWAIT);
>   break;
>   case SRC_TRYWAIT:
>   tcpm_set_cc(port, tcpm_rp_cc(port));
> @@ -2921,20 +2929,12 @@ static void _tcpm_cc_change(struct tcpm_port *port, 
> enum typec_cc_status cc1,
>   if (port->vbus_present || !tcpm_port_is_source(port))
>   tcpm_set_state(port, SRC_TRYWAIT, 0);
>   break;
> - case SNK_TRY_WAIT:
> - if (port->vbus_present && tcpm_port_is_sink(port)) {
> - tcpm_set_state(port, SNK_ATTACHED, 0);
> - break;
> + case SNK_TRY_WAIT_DEBOUNCE:
> + if (!tcpm_port_is_sink(port)) {
> + port->max_wait = 0;
> + tcpm_set_state(port, SRC_TRYWAIT, 0);
>   }
> - if (!tcpm_port_is_sink(port))
> - new_state = SRC_TRYWAIT;
> - else
> - new_state = SRC_TRYWAIT_UNATTACHED;
> -
> - if (new_state != port->delayed_state)
> - tcpm_set_state(port, SNK_TRY_WAIT, 0);
>   break;
> -
>   case SRC_TRY:
>   if (tcpm_port_is_source(port))
>   tcpm_set_state(port, SRC_TRY_DEBOUNCE, 0);
> @@ -2974,8 +2974,6 @@ static void _tcpm_cc_change(struct tcpm_port *port, 
> enum typec_cc_status cc1,
>  
>  static void _tcpm_pd_vbus_on(struct tcpm_port *port)
>  {
> - enum tcpm_state new_state;
> -
>   tcpm_log_force(port, "VBUS on");
>   port->vbus_present = true;
>   switch (port->state) {
> @@ -3011,18 +3009,8 @@ static void _tcpm_pd_vbus_on(struct tcpm_port *port)
>   case SRC_TRYWAIT_DEBOUNCE:
>   tcpm_set_state(port, SRC_TRYWAIT, 0);
>   break;
> - case SNK_TRY_WAIT:
> - if (tcpm_port_is_sink(port)) {
> 

Re: [PATCH 3/6] staging: usb: tcpm: usb: typec: tcpm: Prevent TCPM from looping in SRC_TRYWAIT

[Guenter Roeck]

On Sat, Aug 26, 2017 at 10:24:31PM -0700, Badhri Jagan Sridharan wrote:
> According to the spec the following is the condition
> for exiting TryWait.SRC:
> 
> "The port shall transition to Attached.SRC when V BUS is at vSafe0V
> and the SRC.Rd state is detected on exactly one of the CC pins for at
> least tCCDebounce. The port shall transition to Unattached.SNK after
> tDRPTry if neither of the CC1 or CC2 pins are in the SRC.Rd state"
> 
> TCPM at present keeps re-entering the SRC_TRYWAIT and keeps restarting
> tDRPTry if the CC presents Rp and disconnects within tCCDebounce.
> 
> For example:
> [  447.164308] pending state change SRC_TRYWAIT -> SRC_ATTACHED @ 200 ms
> [  447.164386] CC1: 2 -> 0, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, 
> disconnected]
> [  447.164406] state change SRC_TRYWAIT -> SRC_TRYWAIT
> [  447.164573] cc:=3
> [  447.191408] pending state change SRC_TRYWAIT -> SRC_TRYWAIT_UNATTACHED @ 
> 100 ms
> [  447.191478] CC1: 0 -> 0, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, 
> disconnected]
> [  447.207261] CC1: 0 -> 2, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, 
> connected]
> [  447.207306] state change SRC_TRYWAIT -> SRC_TRYWAIT
> [  447.207485] cc:=3
> [  447.237283] pending state change SRC_TRYWAIT -> SRC_ATTACHED @ 200 ms
> [  447.237357] CC1: 2 -> 0, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, 
> disconnected]
> [  447.237379] state change SRC_TRYWAIT -> SRC_TRYWAIT
> [  447.237532] cc:=3
> [  447.263219] pending state change SRC_TRYWAIT -> SRC_TRYWAIT_UNATTACHED @ 
> 100 ms
> [  447.263289] CC1: 0 -> 0, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, 
> disconnected]
> [  447.280926] CC1: 0 -> 2, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, 
> connected]
> [  447.280970] state change SRC_TRYWAIT -> SRC_TRYWAIT
> [  447.281158] cc:=3
> [  447.307767] pending state change SRC_TRYWAIT -> SRC_ATTACHED @ 200 ms
> [  447.307838] CC1: 2 -> 0, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, 
> disconnected]
> [  447.307858] state change SRC_TRYWAIT -> SRC_TRYWAIT
> 
> In TCPM, tDRPTry is set tp 100ms (min 75ms and max 150ms)
> and tCCdebounce is set to 200ms (min 100ms and max 200ms).
> To overcome the issue, record the time at which the port
> enters TryWait.SRC(SRC_TRYWAIT) and re-enter SRC_TRYWAIT
> only when CC keeps debouncing within tDRPTry.
> 
> Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/tcpm.c | 45 
> 
>  1 file changed, 29 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index 1219e3bc13ef..d45ffa8f2cfd 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -105,6 +106,7 @@
>   S(SNK_TRY), \
>   S(SNK_TRY_WAIT),\
>   S(SRC_TRYWAIT), \
> + S(SRC_TRYWAIT_DEBOUNCE),\
>   S(SRC_TRYWAIT_UNATTACHED),  \
>   \
>   S(SRC_TRY), \
> @@ -284,6 +286,9 @@ struct tcpm_port {
>   struct typec_altmode *partner_altmode[SVID_DISCOVERY_MAX];
>   struct typec_altmode *port_altmode[SVID_DISCOVERY_MAX];
>  
> + /* Deadline in jiffies to exit src_try_wait state */
> + unsigned long max_wait;
> +
>  #ifdef CONFIG_DEBUG_FS
>   struct dentry *dentry;
>   struct mutex logbuffer_lock;/* log buffer access lock */
> @@ -2204,6 +2209,7 @@ static void run_state_machine(struct tcpm_port *port)
>   if (!tcpm_port_is_sink(port)) {
>   tcpm_set_state(port, SRC_TRYWAIT,
>  PD_T_PD_DEBOUNCE);
> + port->max_wait = 0;
>   break;
>   }
>   /* No vbus, cc state is sink or open */
> @@ -2211,11 +2217,22 @@ static void run_state_machine(struct tcpm_port *port)
>   break;
>   case SRC_TRYWAIT:
>   tcpm_set_cc(port, tcpm_rp_cc(port));
> - if (!port->vbus_present && tcpm_port_is_source(port))
> - tcpm_set_state(port, SRC_ATTACHED, PD_T_CC_DEBOUNCE);
> - else
> + if (port->max_wait == 0) {
> + port->max_wait = jiffies +
> +  msecs_to_jiffies(PD_T_DRP_TRY);
> 

Re: [PATCH 2/6] staging: usb: typec: Check for port type for Try.SRC/Try.SNK

[Guenter Roeck]

On Sat, Aug 26, 2017 at 10:24:12PM -0700, Badhri Jagan Sridharan wrote:
> Enable Try.SRC or Try.SNK only when port_type is
> DRP. Try.SRC or Try.SNK state machines are not
> valid for SRC only or SNK only ports.
> 
> Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/tcpm.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index 6c045ac9c42a..1219e3bc13ef 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -328,10 +328,12 @@ struct pd_rx_event {
>(tcpm_cc_is_audio((port)->cc2) && tcpm_cc_is_open((port)->cc1)))
>  
>  #define tcpm_try_snk(port) \
> - ((port)->try_snk_count == 0 && (port)->try_role == TYPEC_SINK)
> + ((port)->try_snk_count == 0 && (port)->try_role == TYPEC_SINK && \
> + (port)->port_type == TYPEC_PORT_DRP)
>  
>  #define tcpm_try_src(port) \
> - ((port)->try_src_count == 0 && (port)->try_role == TYPEC_SOURCE)
> + ((port)->try_src_count == 0 && (port)->try_role == TYPEC_SOURCE && \
> + (port)->port_type == TYPEC_PORT_DRP)
>  
>  static enum tcpm_state tcpm_default_state(struct tcpm_port *port)
>  {
> -- 
> 2.14.1.342.g6490525c54-goog
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/6] staging: usb: typec: tcpm set port type callback

[Guenter Roeck]

On Sat, Aug 26, 2017 at 10:23:24PM -0700, Badhri Jagan Sridharan wrote:
> The port type callback call enquires the tcpc_dev if
> the requested port type is supported. If supported, then
> performs a tcpm reset if required after setting the tcpm
> internal port_type variable.
> 
> Check against the tcpm port_type instead of checking
> against caps.type as port_type reflects the current
> configuration.
> 
> Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/tcpm.c | 52 
> ++--
>  1 file changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index a911cad41a59..6c045ac9c42a 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -197,6 +197,7 @@ struct tcpm_port {
>  
>   bool attached;
>   bool connected;
> + enum typec_port_type port_type;
>   bool vbus_present;
>   bool vbus_never_low;
>   bool vbus_source;
> @@ -334,7 +335,7 @@ struct pd_rx_event {
>  
>  static enum tcpm_state tcpm_default_state(struct tcpm_port *port)
>  {
> - if (port->typec_caps.type == TYPEC_PORT_DRP) {
> + if (port->port_type == TYPEC_PORT_DRP) {
>   if (port->try_role == TYPEC_SINK)
>   return SNK_UNATTACHED;
>   else if (port->try_role == TYPEC_SOURCE)
> @@ -342,7 +343,7 @@ static enum tcpm_state tcpm_default_state(struct 
> tcpm_port *port)
>   else if (port->tcpc->config->default_role == TYPEC_SINK)
>   return SNK_UNATTACHED;
>   /* Fall through to return SRC_UNATTACHED */
> - } else if (port->typec_caps.type == TYPEC_PORT_UFP) {
> + } else if (port->port_type == TYPEC_PORT_UFP) {
>   return SNK_UNATTACHED;
>   }
>   return SRC_UNATTACHED;
> @@ -1458,7 +1459,7 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>   tcpm_set_state(port, SOFT_RESET, 0);
>   break;
>   case PD_CTRL_DR_SWAP:
> - if (port->typec_caps.type != TYPEC_PORT_DRP) {
> + if (port->port_type != TYPEC_PORT_DRP) {
>   tcpm_queue_message(port, PD_MSG_CTRL_REJECT);
>   break;
>   }
> @@ -1478,7 +1479,7 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>   }
>   break;
>   case PD_CTRL_PR_SWAP:
> - if (port->typec_caps.type != TYPEC_PORT_DRP) {
> + if (port->port_type != TYPEC_PORT_DRP) {
>   tcpm_queue_message(port, PD_MSG_CTRL_REJECT);
>   break;
>   }
> @@ -1853,7 +1854,7 @@ static bool tcpm_start_drp_toggling(struct tcpm_port 
> *port)
>   int ret;
>  
>   if (port->tcpc->start_drp_toggling &&
> - port->typec_caps.type == TYPEC_PORT_DRP) {
> + port->port_type == TYPEC_PORT_DRP) {
>   tcpm_log_force(port, "Start DRP toggling");
>   ret = port->tcpc->start_drp_toggling(port->tcpc,
>tcpm_rp_cc(port));
> @@ -2163,7 +2164,7 @@ static void run_state_machine(struct tcpm_port *port)
>   break;
>   }
>   tcpm_set_cc(port, tcpm_rp_cc(port));
> - if (port->typec_caps.type == TYPEC_PORT_DRP)
> + if (port->port_type == TYPEC_PORT_DRP)
>   tcpm_set_state(port, SNK_UNATTACHED, PD_T_DRP_SNK);
>   break;
>   case SRC_ATTACH_WAIT:
> @@ -2320,7 +2321,7 @@ static void run_state_machine(struct tcpm_port *port)
>   break;
>   }
>   tcpm_set_cc(port, TYPEC_CC_RD);
> - if (port->typec_caps.type == TYPEC_PORT_DRP)
> + if (port->port_type == TYPEC_PORT_DRP)
>   tcpm_set_state(port, SRC_UNATTACHED, PD_T_DRP_SRC);
>   break;
>   case SNK_ATTACH_WAIT:
> @@ -2411,7 +2412,7 @@ static void run_state_machine(struct tcpm_port *port)
>* see USB power delivery specification, section 8.3.3.6.1.5.1).
>*/
>   tcpm_set_state(port, hard_reset_state(port),
> -port->typec_caps.type == TYPEC_PORT_DRP ?
> +port->port_type == TYPEC_PORT_DRP ?
>   PD_T_DB_DETECT : PD_T_NO_RESPONSE);
>   break;
>   case SNK_DISCOVERY_DEBOUNCE:
> @@ -3167,7 +3168,7 @@ stati

Re: [PATCH][staging-next] staging: typec: tcpm: make function tcpm_get_pwr_opmode

[Guenter Roeck]

On Tue, Aug 22, 2017 at 05:02:18PM +0100, Colin King wrote:
> From: Colin Ian King <colin.k...@canonical.com>
> 
> The function pointer tcpm_get_pwr_opmode is local to the source and does
> not need to be in global scope, so make it static.
> 
> Cleans up sparse warning:
> symbol 'tcpm_get_pwr_opmode' was not declared. Should it be static?
> 
> Signed-off-by: Colin Ian King <colin.k...@canonical.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/tcpm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index 7b5ba27dd935..a911cad41a59 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -2131,7 +2131,7 @@ static void tcpm_swap_complete(struct tcpm_port *port, 
> int result)
>   }
>  }
>  
> -enum typec_pwr_opmode tcpm_get_pwr_opmode(enum typec_cc_status cc)
> +static enum typec_pwr_opmode tcpm_get_pwr_opmode(enum typec_cc_status cc)
>  {
>   switch (cc) {
>   case TYPEC_CC_RP_1_5:
> -- 
> 2.14.1
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 02/14] staging: typec: tcpm: Add get_current_limit tcpc_dev callback

[Guenter Roeck]

On 08/15/2017 01:04 PM, Hans de Goede wrote:

A Rp signalling the default current limit indicates that we're possibly
connected to an USB2 power-source. In some cases the type-c port-controller
may provide the capability to detect the current-limit in this case,
through e.g. BC1.2 detection.

This commit adds an optional get_current_limit tcpc_dev callback which
allows the port-controller to provide current-limit detection for when
the CC pin is pulled up with Rp.

Signed-off-by: Hans de Goede <hdego...@redhat.com>


Reviewed-by: Guenter Roeck <li...@roeck-us.net>


---
Changes in v2:
-s/get_usb2_current_limit/get_current_limit/
---
  drivers/staging/typec/tcpm.c | 5 -
  drivers/staging/typec/tcpm.h | 7 +++
  2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 20eb4eb..8e823af 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -660,7 +660,10 @@ static u32 tcpm_get_current_limit(struct tcpm_port *port)
break;
case TYPEC_CC_RP_DEF:
default:
-   limit = 0;
+   if (port->tcpc->get_current_limit)
+   limit = port->tcpc->get_current_limit(port->tcpc);
+   else
+   limit = 0;
break;
}
  
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h

index 19c307d..1465668 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -108,6 +108,13 @@ struct tcpc_dev {
  
  	int (*init)(struct tcpc_dev *dev);

int (*get_vbus)(struct tcpc_dev *dev);
+   /*
+* This optional callback gets called by the tcpm core when configured
+* as a snk and cc=Rp-def. This allows the tcpm to provide a fallback
+* current-limit detection method for the cc=Rp-def case. E.g. some
+* tcpcs may include BC1.2 charger detection and use that in this case.
+*/
+   int (*get_current_limit)(struct tcpc_dev *dev);
int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc);
int (*get_cc)(struct tcpc_dev *dev, enum typec_cc_status *cc1,
  enum typec_cc_status *cc2);



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] staging: typec: tcpm: explicit_contract is always established

[Guenter Roeck]

On 08/15/2017 04:23 PM, Badhri Jagan Sridharan wrote:

While in SNK_READY state, the explicit_contract seems to be
set to true irrespective of whether an explicit contract
was established for the current connection. TCPM also seems
to report the pwr_opmode as TYPEC_PWR_MODE_PD always once
the port gets into SNK_READY state. This isn't completely
true as port gets into the SNK_READY state for non-pd
type-c ports as well.

This patch sets the explicit_contract flag only when
the PS_READY message is received and the vbus has been
detected by the port controller.

Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>


Reviewed-by: Guenter Roeck <li...@roeck-us.net>


---
  drivers/staging/typec/tcpm.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index a24e6bbb909c..3e12cf101311 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -1367,6 +1367,7 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
tcpm_set_current_limit(port,
   port->current_limit,
   port->supply_voltage);
+   port->explicit_contract = true;
tcpm_set_state(port, SNK_READY, 0);
} else {
/*
@@ -2458,10 +2459,11 @@ static void run_state_machine(struct tcpm_port *port)
break;
case SNK_READY:
port->try_snk_count = 0;
-   port->explicit_contract = true;
-   typec_set_pwr_opmode(port->typec_port, TYPEC_PWR_MODE_PD);
-   port->pwr_opmode = TYPEC_PWR_MODE_PD;
-
+   if (port->explicit_contract) {
+   typec_set_pwr_opmode(port->typec_port,
+TYPEC_PWR_MODE_PD);
+   port->pwr_opmode = TYPEC_PWR_MODE_PD;
+   }
tcpm_typec_connect(port);
  
  		tcpm_check_send_discover(port);

@@ -2951,6 +2953,7 @@ static void _tcpm_pd_vbus_on(struct tcpm_port *port)
port->vbus_present = true;
switch (port->state) {
case SNK_TRANSITION_SINK_VBUS:
+   port->explicit_contract = true;
tcpm_set_state(port, SNK_READY, 0);
break;
case SNK_DISCOVERY:



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: typec: tcpm: Report right typec_pwr_opmode

[Guenter Roeck]

On 08/15/2017 04:22 PM, Badhri Jagan Sridharan wrote:

At present, TCPM does not take into account the actual resistor
value presented in the CC line and therefore reports TYPEC_PWR_MODE_USB
irrespective of the power_op_mode it is in.
This patch makes TCPM consider the actual value of Rp.

Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>


Reviewed-by: Guenter Roeck <li...@roeck-us.net>


---
  drivers/staging/typec/tcpm.c | 21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 20eb4ebcf8c3..a24e6bbb909c 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -2123,9 +2123,23 @@ static void tcpm_swap_complete(struct tcpm_port *port, 
int result)
}
  }
  
+enum typec_pwr_opmode tcpm_get_pwr_opmode(enum typec_cc_status cc)

+{
+   switch (cc) {
+   case TYPEC_CC_RP_1_5:
+   return TYPEC_PWR_MODE_1_5A;
+   case TYPEC_CC_RP_3_0:
+   return TYPEC_PWR_MODE_3_0A;
+   case TYPEC_CC_RP_DEF:
+   default:
+   return TYPEC_PWR_MODE_USB;
+   }
+}
+
  static void run_state_machine(struct tcpm_port *port)
  {
int ret;
+   enum typec_pwr_opmode opmode;
  
  	port->enter_state = port->state;

switch (port->state) {
@@ -2201,7 +2215,8 @@ static void run_state_machine(struct tcpm_port *port)
   ret < 0 ? 0 : PD_T_PS_SOURCE_ON);
break;
case SRC_STARTUP:
-   typec_set_pwr_opmode(port->typec_port, TYPEC_PWR_MODE_USB);
+   opmode =  tcpm_get_pwr_opmode(tcpm_rp_cc(port));
+   typec_set_pwr_opmode(port->typec_port, opmode);
port->pwr_opmode = TYPEC_PWR_MODE_USB;
port->caps_count = 0;
port->message_id = 0;
@@ -2362,7 +2377,9 @@ static void run_state_machine(struct tcpm_port *port)
break;
case SNK_STARTUP:
/* XXX: callback into infrastructure */
-   typec_set_pwr_opmode(port->typec_port, TYPEC_PWR_MODE_USB);
+   opmode =  tcpm_get_pwr_opmode(port->polarity ?
+ port->cc2 : port->cc1);
+   typec_set_pwr_opmode(port->typec_port, opmode);
port->pwr_opmode = TYPEC_PWR_MODE_USB;
port->message_id = 0;
port->rx_msgid = -1;



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 7/7] staging: typec: tcpm: Check cc status before entering SRC_TRY_DEBOUCE

[Guenter Roeck]

From: Badhri Jagan Sridharan <bad...@google.com>

[  130.893355] state change SNK_DEBOUNCED -> SRC_TRY
[  130.893363] cc:=3
[  130.893490] pending state change SRC_TRY -> SNK_TRYWAIT @ 100 ms
[  130.895602] CC1: 3 -> 0, CC2: 0 -> 0 [state SRC_TRY, polarity 0, 
disconnected]
[  130.895613] state change SRC_TRY -> SRC_TRY_DEBOUNCE
[  130.895621] pending state change SRC_TRY_DEBOUNCE -> SRC_ATTACHED @ 20 ms
[  130.916843] state change SRC_TRY_DEBOUNCE -> SRC_ATTACHED [delayed 20 ms]

Although the CC state was changing to TYPEC_CC_OPEN, the port entered
SRC_TRY_DEBOUNCE from SRC_TRY. The port must enter SRC_TRY_DEBOUNCE only
if the CC state is TYPEC_CC_RD.

Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>
[groeck: Wording]
Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
 drivers/staging/typec/tcpm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index e0f71aeb4ed3..e72286464164 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -2898,7 +2898,8 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum 
typec_cc_status cc1,
break;
 
case SRC_TRY:
-   tcpm_set_state(port, SRC_TRY_DEBOUNCE, 0);
+   if (tcpm_port_is_source(port))
+   tcpm_set_state(port, SRC_TRY_DEBOUNCE, 0);
break;
case SRC_TRY_DEBOUNCE:
tcpm_set_state(port, SRC_TRY, 0);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/7] staging: typec: tcpm: Constify alternate modes

[Guenter Roeck]

Constify alternate mode configuration data which won't be touched
by the driver.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
This series has been in my queue and kind of got lost. Sorry for the delay.

 drivers/staging/typec/tcpm.c | 2 +-
 drivers/staging/typec/tcpm.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 20eb4ebcf8c3..06c49873df8e 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -3485,7 +3485,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, 
struct tcpc_dev *tcpc)
}
 
if (tcpc->config->alt_modes) {
-   struct typec_altmode_desc *paltmode = tcpc->config->alt_modes;
+   const struct typec_altmode_desc *paltmode = 
tcpc->config->alt_modes;
 
i = 0;
while (paltmode->svid && i < ARRAY_SIZE(port->port_altmode)) {
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 19c307d31a5a..4c6b38cb2c8a 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -72,7 +72,7 @@ struct tcpc_config {
enum typec_role default_role;
bool try_role_hw;   /* try.{src,snk} implemented in hardware */
 
-   struct typec_altmode_desc *alt_modes;
+   const struct typec_altmode_desc *alt_modes;
 };
 
 enum tcpc_usb_switch {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 5/7] staging: typec: tcpm: Add timeout when waiting for role swap completion

[Guenter Roeck]

The Type-C protocol manager state machine could fail, which might result
in role swap requests from user space to hang forever. Add a generous
timeout when waiting for role swaps to complete to avoid this situation.

Originally-from: Badhri Jagan Sridharan <bad...@google.com>
Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
 drivers/staging/typec/tcpm.c | 21 +++--
 drivers/staging/typec/tcpm.h |  3 ++-
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index cf498c68c4af..515bec1807d7 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -3166,9 +3166,12 @@ static int tcpm_dr_set(const struct typec_capability 
*cap,
tcpm_set_state(port, DR_SWAP_SEND, 0);
mutex_unlock(>lock);
 
-   wait_for_completion(>swap_complete);
+   if (!wait_for_completion_timeout(>swap_complete,
+   msecs_to_jiffies(PD_ROLE_SWAP_TIMEOUT)))
+   ret = -ETIMEDOUT;
+   else
+   ret = port->swap_status;
 
-   ret = port->swap_status;
goto swap_unlock;
 
 port_unlock:
@@ -3223,9 +3226,12 @@ static int tcpm_pr_set(const struct typec_capability 
*cap,
tcpm_set_state(port, PR_SWAP_SEND, 0);
mutex_unlock(>lock);
 
-   wait_for_completion(>swap_complete);
+   if (!wait_for_completion_timeout(>swap_complete,
+   msecs_to_jiffies(PD_ROLE_SWAP_TIMEOUT)))
+   ret = -ETIMEDOUT;
+   else
+   ret = port->swap_status;
 
-   ret = port->swap_status;
goto swap_unlock;
 
 port_unlock:
@@ -3260,9 +3266,12 @@ static int tcpm_vconn_set(const struct typec_capability 
*cap,
tcpm_set_state(port, VCONN_SWAP_SEND, 0);
mutex_unlock(>lock);
 
-   wait_for_completion(>swap_complete);
+   if (!wait_for_completion_timeout(>swap_complete,
+   msecs_to_jiffies(PD_ROLE_SWAP_TIMEOUT)))
+   ret = -ETIMEDOUT;
+   else
+   ret = port->swap_status;
 
-   ret = port->swap_status;
goto swap_unlock;
 
 port_unlock:
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 4c6b38cb2c8a..374cea44a84a 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -34,7 +34,8 @@ enum typec_cc_polarity {
 };
 
 /* Time to wait for TCPC to complete transmit */
-#define PD_T_TCPC_TX_TIMEOUT  100
+#define PD_T_TCPC_TX_TIMEOUT   100 /* in ms*/
+#define PD_ROLE_SWAP_TIMEOUT   (MSEC_PER_SEC * 10)
 
 enum tcpm_transmit_status {
TCPC_TX_SUCCESS = 0,
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/7] staging: typec: tcpm: Select default state based on port type

[Guenter Roeck]

From: Badhri Jagan Sridharan <bad...@google.com>

tcpm_default_state wasn't considering the port type when determining the
default role. This change makes tcpm_default_state to consider port type
as well.

tcpm_default_state would return the following based on the port type:
TYPEC_PORT_UFP - SNK_UNATTACHED
TYPEC_PORT_DFP - SRC_UNATTACHED
TYPEC_PORT_DRP - based on the preferred_role setting

Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>
[groeck: Reworded description; minor formatting changes]
Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
 drivers/staging/typec/tcpm.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index e7873286b5ea..cf498c68c4af 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -332,12 +332,17 @@ struct pd_rx_event {
 
 static enum tcpm_state tcpm_default_state(struct tcpm_port *port)
 {
-   if (port->try_role == TYPEC_SINK)
-   return SNK_UNATTACHED;
-   else if (port->try_role == TYPEC_SOURCE)
-   return SRC_UNATTACHED;
-   else if (port->tcpc->config->default_role == TYPEC_SINK)
+   if (port->typec_caps.type == TYPEC_PORT_DRP) {
+   if (port->try_role == TYPEC_SINK)
+   return SNK_UNATTACHED;
+   else if (port->try_role == TYPEC_SOURCE)
+   return SRC_UNATTACHED;
+   else if (port->tcpc->config->default_role == TYPEC_SINK)
+   return SNK_UNATTACHED;
+   /* Fall through to return SRC_UNATTACHED */
+   } else if (port->typec_caps.type == TYPEC_PORT_UFP) {
return SNK_UNATTACHED;
+   }
return SRC_UNATTACHED;
 }
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 6/7] staging: typec: tcpm: Improve role swap with non PD capable partners

[Guenter Roeck]

If the partner is not PD capable, we can not use a power role set request
to swap roles. Use the data role set request instead.

Also, if a partner is not PD capable, it does not really make sense to send
a PD message to trigger a role swap. On top of that, we should really wait
for the attempted role change to complete. Otherwise, it may well be that
user space requests another role change immediately afterwards which will
fail because the port is not yet in ready state.

Trigger the role swap from data role change requests and introduce new
state PORT_RESET and use it to solve the problem. This new state is
mostly identical to ERROR_RECOVERY, only it does not cause a pending
role change to fail. Use this new state also when initializing the driver.
Rename ERROR_RECOVERY_WAIT_OFF to PORT_RESET_WAIT_OFF to better reflect
its new meaning.

Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
 drivers/staging/typec/tcpm.c | 60 +---
 1 file changed, 34 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 515bec1807d7..e0f71aeb4ed3 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -115,7 +115,8 @@
S(BIST_RX), \
\
S(ERROR_RECOVERY),  \
-   S(ERROR_RECOVERY_WAIT_OFF)
+   S(PORT_RESET),  \
+   S(PORT_RESET_WAIT_OFF)
 
 #define GENERATE_ENUM(e)   e
 #define GENERATE_STRING(s) #s
@@ -230,6 +231,7 @@ struct tcpm_port {
 
struct mutex swap_lock; /* swap command lock */
bool swap_pending;
+   bool non_pd_role_swap;
struct completion swap_complete;
int swap_status;
 
@@ -2124,6 +2126,7 @@ static void tcpm_swap_complete(struct tcpm_port *port, 
int result)
if (port->swap_pending) {
port->swap_status = result;
port->swap_pending = false;
+   port->non_pd_role_swap = false;
complete(>swap_complete);
}
 }
@@ -2138,7 +2141,8 @@ static void run_state_machine(struct tcpm_port *port)
break;
/* SRC states */
case SRC_UNATTACHED:
-   tcpm_swap_complete(port, -ENOTCONN);
+   if (!port->non_pd_role_swap)
+   tcpm_swap_complete(port, -ENOTCONN);
tcpm_src_detach(port);
if (tcpm_start_drp_toggling(port)) {
tcpm_set_state(port, DRP_TOGGLING, 0);
@@ -2293,7 +2297,8 @@ static void run_state_machine(struct tcpm_port *port)
 
/* SNK states */
case SNK_UNATTACHED:
-   tcpm_swap_complete(port, -ENOTCONN);
+   if (!port->non_pd_role_swap)
+   tcpm_swap_complete(port, -ENOTCONN);
tcpm_snk_detach(port);
if (tcpm_start_drp_toggling(port)) {
tcpm_set_state(port, DRP_TOGGLING, 0);
@@ -2704,13 +2709,15 @@ static void run_state_machine(struct tcpm_port *port)
break;
case ERROR_RECOVERY:
tcpm_swap_complete(port, -EPROTO);
+   tcpm_set_state(port, PORT_RESET, 0);
+   break;
+   case PORT_RESET:
tcpm_reset_port(port);
-
tcpm_set_cc(port, TYPEC_CC_OPEN);
-   tcpm_set_state(port, ERROR_RECOVERY_WAIT_OFF,
+   tcpm_set_state(port, PORT_RESET_WAIT_OFF,
   PD_T_ERROR_RECOVERY);
break;
-   case ERROR_RECOVERY_WAIT_OFF:
+   case PORT_RESET_WAIT_OFF:
tcpm_set_state(port,
   tcpm_default_state(port),
   port->vbus_present ? PD_T_PS_SOURCE_OFF : 0);
@@ -3042,7 +3049,7 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
/* Do nothing, expected */
break;
 
-   case ERROR_RECOVERY_WAIT_OFF:
+   case PORT_RESET_WAIT_OFF:
tcpm_set_state(port, tcpm_default_state(port), 0);
break;
 
@@ -3139,7 +3146,7 @@ static int tcpm_dr_set(const struct typec_capability *cap,
mutex_lock(>swap_lock);
mutex_lock(>lock);
 
-   if (port->typec_caps.type != TYPEC_PORT_DRP || !port->pd_capable) {
+   if (port->typec_caps.type != TYPEC_PORT_DRP) {
ret = -EINVAL;
goto port_unlock;
}
@@ -3160,10 +3167,26 @@ static int tcpm_dr_set(const struct typec_capability 
*cap,
 * Reject data role swap request in this case.
 */
 
+   if (!port->pd_capable) {
+   /*
+* If the partner is not PD capable, reset the port to
+* trigger a role change. This can only work if a preferred
+* role is configured, and if it matches the requested role.
+ 

[PATCH 2/7] staging: typec: tcpm: Report role swap complete after entering READY state

[Guenter Roeck]

Role swap requests fail unless the current role is either SRC_READY or
SNK_READY. This works fine for VCONN and data role swaps, where we
immediately enter READY state after reporting a successful role swap
to user space. However, on power role changes, the role swap is currently
reported as successful while power negotiation is still in process.
User space does not know this, and may request another role swap
immediately after a power role swap is reported to be complete.
This second role swap will fail with -EAGAIN.

To fix the problem, report role swap completion after power negotiation
is complete and the state machine enters SRC_READY or SNK_READY state.
This is better anyway since it captures errors due to failed power
negotiations. It also simplifies the code since the number of calls
needed to report successful role swaps is reduced.

Reported-by: Howard Yen <howard_...@htc.com>
Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
 drivers/staging/typec/tcpm.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 06c49873df8e..be2a91b52d69 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -2262,8 +2262,8 @@ static void run_state_machine(struct tcpm_port *port)
 #endif
port->try_src_count = 0;
 
+   tcpm_swap_complete(port, 0);
tcpm_typec_connect(port);
-
tcpm_check_send_discover(port);
/*
 * 6.3.5
@@ -2445,8 +2445,8 @@ static void run_state_machine(struct tcpm_port *port)
typec_set_pwr_opmode(port->typec_port, TYPEC_PWR_MODE_PD);
port->pwr_opmode = TYPEC_PWR_MODE_PD;
 
+   tcpm_swap_complete(port, 0);
tcpm_typec_connect(port);
-
tcpm_check_send_discover(port);
break;
 
@@ -2574,7 +2574,6 @@ static void run_state_machine(struct tcpm_port *port)
   TYPEC_HOST);
port->send_discover = true;
}
-   tcpm_swap_complete(port, 0);
tcpm_set_state(port, ready_state(port), 0);
break;
 
@@ -2622,7 +2621,6 @@ static void run_state_machine(struct tcpm_port *port)
tcpm_set_state_cond(port, SNK_UNATTACHED, PD_T_PS_SOURCE_ON);
break;
case PR_SWAP_SRC_SNK_SINK_ON:
-   tcpm_swap_complete(port, 0);
tcpm_set_state(port, SNK_STARTUP, 0);
break;
case PR_SWAP_SNK_SRC_SINK_OFF:
@@ -2642,7 +2640,6 @@ static void run_state_machine(struct tcpm_port *port)
 */
tcpm_set_pwr_role(port, TYPEC_SOURCE);
tcpm_pd_send_control(port, PD_CTRL_PS_RDY);
-   tcpm_swap_complete(port, 0);
tcpm_set_state(port, SRC_STARTUP, 0);
break;
 
@@ -2672,12 +2669,10 @@ static void run_state_machine(struct tcpm_port *port)
case VCONN_SWAP_TURN_ON_VCONN:
tcpm_set_vconn(port, true);
tcpm_pd_send_control(port, PD_CTRL_PS_RDY);
-   tcpm_swap_complete(port, 0);
tcpm_set_state(port, ready_state(port), 0);
break;
case VCONN_SWAP_TURN_OFF_VCONN:
tcpm_set_vconn(port, false);
-   tcpm_swap_complete(port, 0);
tcpm_set_state(port, ready_state(port), 0);
break;
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/7] staging: typec: tcpm: Set default state after error recovery based on port type

[Guenter Roeck]

From: Badhri Jagan Sridharan <bad...@google.com>

While exiting ERROR_RECOVERY, choose default state based on the port
type instead of current power role.

Quoting from specification:

4.5.2.2.2 ErrorRecovery State
This state appears in Figure 4-12, Figure 4-13, Figure 4-14, Figure 4-15,
Figure 4-16 and Figure 4-17.
The ErrorRecovery state is where the port removes the terminations from
the CC1 and CC2 pins for tErrorRecovery followed by transitioning to the
appropriate Unattached.SNK or Unattached.SRC state based on port type.
This is the equivalent of forcing a detach event and looking for a new
attach.

Signed-off-by: Badhri Jagan Sridharan <bad...@google.com>
Signed-off-by: Guenter Roeck <li...@roeck-us.net>
---
 drivers/staging/typec/tcpm.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index be2a91b52d69..e7873286b5ea 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -3038,10 +3038,7 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
break;
 
case ERROR_RECOVERY_WAIT_OFF:
-   tcpm_set_state(port,
-  port->pwr_role == TYPEC_SOURCE ?
-   SRC_UNATTACHED : SNK_UNATTACHED,
-  0);
+   tcpm_set_state(port, tcpm_default_state(port), 0);
break;
 
default:
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 10/18] staging: typec: fusb302: Add support for fcs,vbus-regulator-name device-property

[Guenter Roeck]

On 08/06/2017 07:52 AM, Hans de Goede wrote:

Hi,

On 06-08-17 16:30, Guenter Roeck wrote:

On 08/06/2017 05:35 AM, Hans de Goede wrote:

On devicetree platforms the fusb302 dt-node will have a vbus regulator
property with a phandle to the regulator.

On ACPI platforms, there are no phandles and we need to get the vbus by a
system wide unique name. Add support for a new "fcs,vbus-regulator-name"
device-property which ACPI platform code can set to pass the name.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
  drivers/staging/typec/fusb302/fusb302.c | 28 ++--
  1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index e1e08f57af99..c3bcc5484ade 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1722,6 +1722,28 @@ static int fusb302_probe(struct i2c_client *client,
  return -EPROBE_DEFER;
  }
+/*
+ * Devicetree platforms should get vbus from their dt-node.
+ * On ACPI platforms, we need to get the vbus by a system wide unique
+ * name, which is set in a device prop by the platform code.
+ */
+if (device_property_read_string(dev, "fcs,vbus-regulator-name",
+) == 0) {


Another property to be documented and approved.


Again this is for kernel internal use on non-dt platforms only, so documenting
it in the devicetree bindings is not necessary.


Ok.


Also, isn't there a better way to get regulator names for dt- and non-dt 
systems ?
This would apply to every driver supporting both and using regulators, which 
seems
awkward.


While working on this I noticed that it is possible to add a regulator_match
table entry when registering a regulator, but that requires describing this
in regulator_init_data. Which would mean passing regulator_init_data from the
place where it is instantiated to where it gets registered, which would
mean passing a pointer through a device-property, given that this is purely 
kernel
internal that is possible, but not really how device-props are supposed to be 
used.

Also since the regulator-core only adds the mapping when registering the
regulator, this means that if we try to get the regulator before it has been
registered; and there is another regulator with the rather generic "vbus"
name then that will be returned instead.

Basically regulators are practically almost unused on x86 systems. I had to
add CONFIG_REGULATOR=y to my .config which is based on the Fedora 26 kernel
.config, so it has pretty much everything under the sun enabled. So it seems
that we are covering new ground here.



We have some in hwmon, but they get by with using devm_regulator_get_optional()
for both dt and non-dt systems. Only problem with that is that it returns
-ENODEV if regulators are not configured, which by itself is weird/odd
(and there have been endless discussions about it).


An alternative would be to not use the regulator subsys for this at all, but
it does seem the logical thing to use and using get-by-name is no different
then what we've doing for setting the the "fusb302-typec-source" psy as supplier
for the charger psy class device registered by the bq24190_charger driver.

TL;DR: It seems that on x86, at least for existing devices where we cannot
control the ACPI tables that getting things by name is the thing to do.



Messy :-(. I don't have a better idea, unfortunately.


+/*
+ * Use regulator_get_optional so that we can detect if we need
+ * to defer the probe rather then getting the dummy-regulator.
+ */


Wouldn't this apply to dt systems as well ?


No because there will be a property named "vbus-supply" in the fusb302
node containing a phandle to the regulator, if the regulator to which the 
phandle
points has not been registered yet regulator_get will automatically return
-EPROBE_DEFER because there is a "vbus-supply" property, only if there is
no such property at all will it return a dummy regulator.



More messy. Again, I don't have a better idea, but it is really weird that we
need all this code. There should really be some generic code handling all those
differences.


+chip->vbus = devm_regulator_get_optional(dev, name);
+if (IS_ERR(chip->vbus)) {
+ret = PTR_ERR(chip->vbus);
+return (ret == -ENODEV) ? -EPROBE_DEFER : ret;


This will be stuck in returning -EPROBE_DEFER if the regulator subsystem
is disabled. Is this acceptable ?


+}
+} else {
+chip->vbus = devm_regulator_get(dev, "vbus");
+if (IS_ERR(chip->vbus))
+return PTR_ERR(chip->vbus);
+}
+


You might also want to explain why you moved this code.


Right, I did that because it may fail with -EPROBE_DEFER and
I wanted to do that before the register_psy. But as I just

Re: [PATCH 08/18] staging: typec: fusb302: Add support for USB2 charger detection through extcon

[Guenter Roeck]

On 08/06/2017 07:36 AM, Hans de Goede wrote:

Hi,

On 06-08-17 16:22, Guenter Roeck wrote:

On 08/06/2017 05:35 AM, Hans de Goede wrote:

The fusb302 port-controller relies on an external device doing USB2
charger-type detection.

The Intel Whiskey Cove PMIC with which the fusb302 is combined on some
X86/ACPI platforms already has a charger-type detection driver which
uses extcon to communicate the detected charger-type.

This commit uses the tcpm_get_usb2_current_limit_extcon helper to enable
USB2 charger detection on these systems. Note that the "fcs,extcon-name"
property name is only for kernel internal use by X86/ACPI platform code
and as such is NOT documented in the devicetree bindings.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
  drivers/staging/typec/fusb302/fusb302.c | 16 
  1 file changed, 16 insertions(+)

diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index be454b5ff6c7..1d8c9b66df2f 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1201,6 +1201,8 @@ static void init_tcpc_dev(struct tcpc_dev 
*fusb302_tcpc_dev)
  {
  fusb302_tcpc_dev->init = tcpm_init;
  fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
+fusb302_tcpc_dev->get_usb2_current_limit =
+tcpm_get_usb2_current_limit_extcon;
  fusb302_tcpc_dev->set_cc = tcpm_set_cc;
  fusb302_tcpc_dev->get_cc = tcpm_get_cc;
  fusb302_tcpc_dev->set_polarity = tcpm_set_polarity;
@@ -1685,6 +1687,7 @@ static int fusb302_probe(struct i2c_client *client,
  struct fusb302_chip *chip;
  struct i2c_adapter *adapter;
  struct device *dev = >dev;
+const char *name;
  int ret = 0;
  u32 val;
@@ -1717,6 +1720,19 @@ static int fusb302_probe(struct i2c_client *client,
  if (device_property_read_u32(dev, "fcs,operating-snk-mw", ) == 0)
  chip->tcpc_config.operating_snk_mw = val;
+/*
+ * Devicetree platforms should get extcon via phandle (not yet
+ * supported). On ACPI platforms, we get the name from a device prop.
+ * This device prop is for kernel internal use only and is expected
+ * to be set by the platform code which also registers the i2c client
+ * for the fusb302.
+ */
+if (device_property_read_string(dev, "fcs,extcon-name", ) == 0) {


Those new devicetree properties need to be documented and approved by dt 
maintainers.


As indicated by the comment, this property should not be used in the devicetree
case, notice this is a device-property and not an of property and since it is
not intended to be used with devicetree at all (in devicetree a phandle rather
then a name would be used), it has no place under Documentation/devicetree at 
all.



Ok. I thought device properties were supposed to unify dt and non-dt situations,
but apparently not. Oh well :-(.


Also there is no current binding documentation for the "fcs,fusb302" compatible
and the weird "fcs,int_n" gpio which really is an irq which it already uses.



Yes, one of those TODO items for moving the code out of staging. "fcs," should
possibly be "fusb302,", and int_n _is_ weird.






+chip->tcpc_dev.usb2_extcon = extcon_get_extcon_dev(name);
+if (!chip->tcpc_dev.usb2_extcon)


extcon_get_extcon_dev() can also return an ERR_PTR.

As before, I am not really happy typing this into tcpm via tcpc_dev.
Until we have a better solution, I would prefer for that code to stay with the 
fusb302
code.


Ok, I tried to make this all re-usable since I think other port-controller 
drivers
will need something similar, but I can kill the entire tcpm-helpers.c file in v2
and then put everything in fusb302.c. I take it that that is what you want ?


Yes, please.

Also, please copy Yueyao Zhu for the fusb302 changes in v2.

Thanks,
Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/18] staging: typec: tcpm: Add get_usb2_current_limit tcpc_dev callback

[Guenter Roeck]

On 08/06/2017 07:29 AM, Hans de Goede wrote:

Hi,

On 06-08-17 16:18, Guenter Roeck wrote:

On 08/06/2017 05:35 AM, Hans de Goede wrote:

A Rp signalling the default current limit indicates that we're possibly
connected to an USB2 power-source. In some cases the type-c
port-controller may provide the capability to detect the current-limit
for USB2 power-sources (through e.g. BC1.2 detection).

This commit adds an optional get_usb2_current_limit tcpc_dev callback
which allows the port-controller to return the detected current-limit
if the CC pin is pulled up with Rp.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
  drivers/staging/typec/tcpm.c | 5 -
  drivers/staging/typec/tcpm.h | 1 +
  2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 20eb4ebcf8c3..9f5adace4309 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -660,7 +660,10 @@ static u32 tcpm_get_current_limit(struct tcpm_port *port)
  break;
  case TYPEC_CC_RP_DEF:
  default:
-limit = 0;
+if (port->tcpc->get_usb2_current_limit)


I think this callback should just be get_current_limit.


This is only used in the Rp-def case, which usually indicates being connected
with an A->C cable to a USB-2 device and the intend is for the callback to
implement some USB-2 specific method to detect the supported current-limit,
hence the name. If you prefer to name it just get_current_limit I can
change it for v2, but IMHO the usb2 part of the name is important as this
will not get called when USB-PD negotiation is used, nor when Rp has the Rp15
or Rp30 values.



As you say, it gets called in the Rp-def case, which can be set anytime.
I'd rather have a generic name and explain in tcpm.h as part of the API
description when it is used/called.

Note that it is also called in SNK_DISCOVERY if cc=Rp-def to calculate
and set the initial current limit, even with pd.

Thanks,
Guenter


Regards,

Hans






+limit = port->tcpc->get_usb2_current_limit(port->tcpc);
+else
+limit = 0;
  break;
  }
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 19c307d31a5a..01b7d89379a3 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -108,6 +108,7 @@ struct tcpc_dev {
  int (*init)(struct tcpc_dev *dev);
  int (*get_vbus)(struct tcpc_dev *dev);
+int (*get_usb2_current_limit)(struct tcpc_dev *dev); /* Optional */
  int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc);
  int (*get_cc)(struct tcpc_dev *dev, enum typec_cc_status *cc1,
enum typec_cc_status *cc2);







___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 04/18] staging: typec: tcpm: Add helpers for exporting current-limit through a psy

[Guenter Roeck]

On 08/06/2017 07:21 AM, Hans de Goede wrote:

Hi,

On 06-08-17 16:13, Guenter Roeck wrote:

On 08/06/2017 05:35 AM, Hans de Goede wrote:

Not all type-c port-controller can control the current-limit directly,
in cases where the current limit can not be controlled directly it needs
to be exported so that another driver (e.g. the charger driver) can pick
the limit up and configure the system accordingly.

The power-supply subsys already provides infrastructure for this,
power-supply devices have the notion of being supplied by another
power-supply and have properties through which we can export the
current-limit.

This commits adds 2 helper functions for use by port-controller drivers
which want to export the current-limit info in this way:

int tcpm_register_psy(struct device *dev, struct tcpc_dev *tcpc,
  const char *name);
int tcpm_set_current_limit_psy(struct tcpc_dev *tcpc, u32 max_ma, u32 mv);

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
  drivers/staging/typec/tcpm-helpers.c | 66 
  drivers/staging/typec/tcpm.h |  9 +
  2 files changed, 75 insertions(+)

diff --git a/drivers/staging/typec/tcpm-helpers.c 
b/drivers/staging/typec/tcpm-helpers.c
index 0c87ec9936e1..8f7699576878 100644
--- a/drivers/staging/typec/tcpm-helpers.c
+++ b/drivers/staging/typec/tcpm-helpers.c
@@ -20,6 +20,60 @@
  #include "tcpm.h"
+static int tcpm_psy_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+struct tcpc_dev *tcpc = power_supply_get_drvdata(psy);
+
+switch (psp) {
+case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+val->intval = tcpc->supply_voltage * 1000; /* mV -> µV */
+break;
+case POWER_SUPPLY_PROP_CURRENT_MAX:
+val->intval = tcpc->current_limit * 1000; /* mA -> µA */
+break;
+default:
+return -ENODATA;
+}
+
+return 0;
+}
+
+static enum power_supply_property tcpm_psy_properties[] = {
+POWER_SUPPLY_PROP_VOLTAGE_NOW,
+POWER_SUPPLY_PROP_CURRENT_MAX,
+};
+
+int tcpm_register_psy(struct device *dev, struct tcpc_dev *tcpc,
+  const char *name)


This is misleading since it relies on devm functions, and there is no 
unregister function.


The reliance on devm functions is intentional, that makes cleanup on probe() 
failure for users
of this a lot easier. I guess we could rename this tcpm_initialize_psy(), 
although it does
actually register a psy, so maybe: devm_tcpm_register_psy() to explain why 
there is no
unregister counter-part ?



I don't question the use of devm functions. Yes, I think 
devm_tcpm_register_psy() would be better.


Overall I am not too happy with tying this all into tcpm. The functions are not 
really tcpm
related. Not that I have a better idea how to handle this right now, but 
conceptually it
doesn't seem correct.


Note that no changes are made to the tcpm core in this entire patch-set, with 
the exception
of the first patch which adds the get_usb2_current_limit callback.


Yes, but you are using tcpm data structures. Of this entire series, I think 
only the callback
(renamed to get_usb_current_limit) should really be in tcpm code. Maybe it 
makes sense to find
a way to provide helpers in a generic way, if it turns out that the same code 
is needed by multiple
drivers, but right now they are only used by fusb302 and might as well stay 
there.

Until other drivers need them, we don't really know if the helpers are useful 
for multiple drivers.
I would prefer to add such helpers if and when we have more than one driver 
using them.

Thanks,
Guenter


This really only ties into the port-controller driver, and as such uses 
tcpc_dev to store some
stuff. I could make this more clear by prefixing the helper function names with 
tcpc instead of
tcpm if you think that is better ?

Regards,

Hans





+{
+struct power_supply_config psy_cfg = {};
+struct power_supply_desc *desc;
+int ret = 0;
+
+desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
+if (!desc)
+return -ENOMEM;
+
+desc->name= name;
+desc->type= POWER_SUPPLY_TYPE_USB;
+desc->properties= tcpm_psy_properties;
+desc->num_properties= ARRAY_SIZE(tcpm_psy_properties);
+desc->get_property= tcpm_psy_get_property;
+psy_cfg.drv_data= tcpc;
+
+tcpc->psy = devm_power_supply_register(dev, desc, _cfg);
+if (IS_ERR(tcpc->psy)) {
+ret = PTR_ERR(tcpc->psy);
+tcpc->psy = NULL;
+dev_err(dev, "Error registering power-supply: %d\n", ret);
+}
+
+return ret;
+}
+EXPORT_SYMBOL_GPL(tcpm_register_psy);
+
  /* Generic (helper) implementations for some tcpc_dev callbacks */
  int tcpm_get_usb2_current_limit_extcon(struct tcpc_dev *tcpc)
  {
@@ -58,3 +112,15 @@ int tcpm_get_usb2_current_limit_extcon(struct tcpc_dev 
*tcpc)
  return cu

Re: [PATCH 18/18] i2c-cht-wc: Add device-properties for fusb302 integration

[Guenter Roeck]

On 08/06/2017 05:35 AM, Hans de Goede wrote:

Add device-properties to make the bq24292i controller connected to
the bus get its input-current-limit from the fusb302 Type-C port
controller which is used on boards with the cht-wc PMIC.

Signed-off-by: Hans de Goede 
---
  drivers/i2c/busses/Kconfig  | 5 +
  drivers/i2c/busses/i2c-cht-wc.c | 5 -
  2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index f20b1f84013a..6de21a81b00b 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -197,6 +197,11 @@ config I2C_CHT_WC
  SMBus controller found in the Intel Cherry Trail Whiskey Cove PMIC
  found on some Intel Cherry Trail systems.
  
+	  Note this controller is hooked up to a TI bq24292i charger-IC,

+ combined with a FUSB302 Type-C port-controller as such it is advised
+ to also select CONFIG_CHARGER_BQ24190=m and CONFIG_TYPEC_FUSB302=m
+ (the fusb302 driver currently is in drivers/staging).
+


Just wondering - is this always the case ? What if someone builds a system with
different charger and port controller ICs ?


  config I2C_NFORCE2
tristate "Nvidia nForce2, nForce3 and nForce4"
depends on PCI
diff --git a/drivers/i2c/busses/i2c-cht-wc.c b/drivers/i2c/busses/i2c-cht-wc.c
index ccf0785bcb75..08229fb12615 100644
--- a/drivers/i2c/busses/i2c-cht-wc.c
+++ b/drivers/i2c/busses/i2c-cht-wc.c
@@ -211,8 +211,11 @@ static const struct irq_chip cht_wc_i2c_irq_chip = {
.name   = "cht_wc_ext_chrg_irq_chip",
  };
  
+static const char * const bq24190_suppliers[] = { "fusb302-typec-source" };

+
  static const struct property_entry bq24190_props[] = {
-   PROPERTY_ENTRY_STRING("extcon-name", "cht_wcove_pwrsrc"),
+   PROPERTY_ENTRY_STRING_ARRAY("supplied-from", bq24190_suppliers),
+   PROPERTY_ENTRY_BOOL("input-current-limit-from-supplier"),
PROPERTY_ENTRY_BOOL("omit-battery-class"),
PROPERTY_ENTRY_BOOL("disable-reset"),
{ }



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 11/18] power: supply: Fix power_supply_am_i_supplied to return -ENODEV when apropriate

[Guenter Roeck]

On 08/06/2017 05:35 AM, Hans de Goede wrote:

Commit 2848e039c562 ("power: supply: Make power_supply_am_i_supplied return
-ENODEV if there are no suppliers") was supposed to make
power_supply_am_i_supplied() return -ENODEV when there are no supplies
which supply the supply passed to it.

But instead it will only return -ENODEV when there are no supplies at
all as data->count++; is incremented on every call of the iterator, rather
then only when __power_supply_is_supplied_by returns true. This commit
fixes this.

Fixes: 2848e039c562 ("power: supply: Make power_supply_am_i_supplied ...")
Signed-off-by: Hans de Goede 


Independent of this series ?


---
  drivers/power/supply/power_supply_core.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c 
b/drivers/power/supply/power_supply_core.c
index 540d3e0aa011..0741fcef3b44 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -314,11 +314,12 @@ static int __power_supply_am_i_supplied(struct device 
*dev, void *_data)
struct power_supply *epsy = dev_get_drvdata(dev);
struct psy_am_i_supplied_data *data = _data;
  
-	data->count++;

-   if (__power_supply_is_supplied_by(epsy, data->psy))
+   if (__power_supply_is_supplied_by(epsy, data->psy)) {
+   data->count++;
if (!epsy->desc->get_property(epsy, POWER_SUPPLY_PROP_ONLINE,
))
return ret.intval;
+   }
  
  	return 0;

  }



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 10/18] staging: typec: fusb302: Add support for fcs,vbus-regulator-name device-property

[Guenter Roeck]

On 08/06/2017 05:35 AM, Hans de Goede wrote:

On devicetree platforms the fusb302 dt-node will have a vbus regulator
property with a phandle to the regulator.

On ACPI platforms, there are no phandles and we need to get the vbus by a
system wide unique name. Add support for a new "fcs,vbus-regulator-name"
device-property which ACPI platform code can set to pass the name.

Signed-off-by: Hans de Goede 
---
  drivers/staging/typec/fusb302/fusb302.c | 28 ++--
  1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index e1e08f57af99..c3bcc5484ade 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1722,6 +1722,28 @@ static int fusb302_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
  
+	/*

+* Devicetree platforms should get vbus from their dt-node.
+* On ACPI platforms, we need to get the vbus by a system wide unique
+* name, which is set in a device prop by the platform code.
+*/
+   if (device_property_read_string(dev, "fcs,vbus-regulator-name",
+   ) == 0) {


Another property to be documented and approved.

Also, isn't there a better way to get regulator names for dt- and non-dt 
systems ?
This would apply to every driver supporting both and using regulators, which 
seems
awkward.


+   /*
+* Use regulator_get_optional so that we can detect if we need
+* to defer the probe rather then getting the dummy-regulator.
+*/


Wouldn't this apply to dt systems as well ?


+   chip->vbus = devm_regulator_get_optional(dev, name);
+   if (IS_ERR(chip->vbus)) {
+   ret = PTR_ERR(chip->vbus);
+   return (ret == -ENODEV) ? -EPROBE_DEFER : ret;
+   }
+   } else {
+   chip->vbus = devm_regulator_get(dev, "vbus");
+   if (IS_ERR(chip->vbus))
+   return PTR_ERR(chip->vbus);
+   }
+


You might also want to explain why you moved this code.


ret = tcpm_register_psy(chip->dev, >tcpc_dev,
"fusb302-typec-source");
if (ret < 0)
@@ -1739,12 +1761,6 @@ static int fusb302_probe(struct i2c_client *client,
INIT_DELAYED_WORK(>bc_lvl_handler, fusb302_bc_lvl_handler_work);
init_tcpc_dev(>tcpc_dev);
  
-	chip->vbus = devm_regulator_get(chip->dev, "vbus");

-   if (IS_ERR(chip->vbus)) {
-   ret = PTR_ERR(chip->vbus);
-   goto destroy_workqueue;
-   }
-
if (client->irq) {
chip->gpio_int_n_irq = client->irq;
} else {



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 09/18] staging: typec: fusb302: Use tcpm_set_current_limit_psy

[Guenter Roeck]

On 08/06/2017 05:35 AM, Hans de Goede wrote:

Register a power_supply and use tcpm_set_current_limit_psy as
set_current_limit so that another driver (e.g. the charger driver) can
pick the limit up and configure the system accordingly.

Signed-off-by: Hans de Goede 
---
  drivers/staging/typec/fusb302/fusb302.c | 18 ++
  1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index 1d8c9b66df2f..e1e08f57af99 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -854,17 +854,6 @@ static int tcpm_set_vbus(struct tcpc_dev *dev, bool on, 
bool charge)
return ret;
  }
  
-static int tcpm_set_current_limit(struct tcpc_dev *dev, u32 max_ma, u32 mv)

-{
-   struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
-tcpc_dev);
-
-   fusb302_log(chip, "current limit: %d ma, %d mv (not implemented)",
-   max_ma, mv);
-
-   return 0;
-}
-


Same comment as before. I think the fusb code should do what 
tcpm_set_current_limit_psy
is doing, ie register with a power supply.


  static int fusb302_pd_tx_flush(struct fusb302_chip *chip)
  {
return fusb302_i2c_set_bits(chip, FUSB_REG_CONTROL0,
@@ -1208,7 +1197,7 @@ static void init_tcpc_dev(struct tcpc_dev 
*fusb302_tcpc_dev)
fusb302_tcpc_dev->set_polarity = tcpm_set_polarity;
fusb302_tcpc_dev->set_vconn = tcpm_set_vconn;
fusb302_tcpc_dev->set_vbus = tcpm_set_vbus;
-   fusb302_tcpc_dev->set_current_limit = tcpm_set_current_limit;
+   fusb302_tcpc_dev->set_current_limit = tcpm_set_current_limit_psy;
fusb302_tcpc_dev->set_pd_rx = tcpm_set_pd_rx;
fusb302_tcpc_dev->set_roles = tcpm_set_roles;
fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
@@ -1733,6 +1722,11 @@ static int fusb302_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
  
+	ret = tcpm_register_psy(chip->dev, >tcpc_dev,

+   "fusb302-typec-source");
+   if (ret < 0)
+   return ret;
+
ret = fusb302_debugfs_init(chip);
if (ret < 0)
return ret;



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 08/18] staging: typec: fusb302: Add support for USB2 charger detection through extcon

[Guenter Roeck]

On 08/06/2017 05:35 AM, Hans de Goede wrote:

The fusb302 port-controller relies on an external device doing USB2
charger-type detection.

The Intel Whiskey Cove PMIC with which the fusb302 is combined on some
X86/ACPI platforms already has a charger-type detection driver which
uses extcon to communicate the detected charger-type.

This commit uses the tcpm_get_usb2_current_limit_extcon helper to enable
USB2 charger detection on these systems. Note that the "fcs,extcon-name"
property name is only for kernel internal use by X86/ACPI platform code
and as such is NOT documented in the devicetree bindings.

Signed-off-by: Hans de Goede 
---
  drivers/staging/typec/fusb302/fusb302.c | 16 
  1 file changed, 16 insertions(+)

diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index be454b5ff6c7..1d8c9b66df2f 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1201,6 +1201,8 @@ static void init_tcpc_dev(struct tcpc_dev 
*fusb302_tcpc_dev)
  {
fusb302_tcpc_dev->init = tcpm_init;
fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
+   fusb302_tcpc_dev->get_usb2_current_limit =
+   tcpm_get_usb2_current_limit_extcon;
fusb302_tcpc_dev->set_cc = tcpm_set_cc;
fusb302_tcpc_dev->get_cc = tcpm_get_cc;
fusb302_tcpc_dev->set_polarity = tcpm_set_polarity;
@@ -1685,6 +1687,7 @@ static int fusb302_probe(struct i2c_client *client,
struct fusb302_chip *chip;
struct i2c_adapter *adapter;
struct device *dev = >dev;
+   const char *name;
int ret = 0;
u32 val;
  
@@ -1717,6 +1720,19 @@ static int fusb302_probe(struct i2c_client *client,

if (device_property_read_u32(dev, "fcs,operating-snk-mw", ) == 0)
chip->tcpc_config.operating_snk_mw = val;
  
+	/*

+* Devicetree platforms should get extcon via phandle (not yet
+* supported). On ACPI platforms, we get the name from a device prop.
+* This device prop is for kernel internal use only and is expected
+* to be set by the platform code which also registers the i2c client
+* for the fusb302.
+*/
+   if (device_property_read_string(dev, "fcs,extcon-name", ) == 0) {


Those new devicetree properties need to be documented and approved by dt 
maintainers.


+   chip->tcpc_dev.usb2_extcon = extcon_get_extcon_dev(name);
+   if (!chip->tcpc_dev.usb2_extcon)


extcon_get_extcon_dev() can also return an ERR_PTR.

As before, I am not really happy typing this into tcpm via tcpc_dev.
Until we have a better solution, I would prefer for that code to stay with the 
fusb302
code.


+   return -EPROBE_DEFER;
+   }
+
ret = fusb302_debugfs_init(chip);
if (ret < 0)
return ret;



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/18] staging: typec: tcpm: Add get_usb2_current_limit tcpc_dev callback

[Guenter Roeck]

On 08/06/2017 05:35 AM, Hans de Goede wrote:

A Rp signalling the default current limit indicates that we're possibly
connected to an USB2 power-source. In some cases the type-c
port-controller may provide the capability to detect the current-limit
for USB2 power-sources (through e.g. BC1.2 detection).

This commit adds an optional get_usb2_current_limit tcpc_dev callback
which allows the port-controller to return the detected current-limit
if the CC pin is pulled up with Rp.

Signed-off-by: Hans de Goede 
---
  drivers/staging/typec/tcpm.c | 5 -
  drivers/staging/typec/tcpm.h | 1 +
  2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 20eb4ebcf8c3..9f5adace4309 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -660,7 +660,10 @@ static u32 tcpm_get_current_limit(struct tcpm_port *port)
break;
case TYPEC_CC_RP_DEF:
default:
-   limit = 0;
+   if (port->tcpc->get_usb2_current_limit)


I think this callback should just be get_current_limit.


+   limit = port->tcpc->get_usb2_current_limit(port->tcpc);
+   else
+   limit = 0;
break;
}
  
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h

index 19c307d31a5a..01b7d89379a3 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -108,6 +108,7 @@ struct tcpc_dev {
  
  	int (*init)(struct tcpc_dev *dev);

int (*get_vbus)(struct tcpc_dev *dev);
+   int (*get_usb2_current_limit)(struct tcpc_dev *dev); /* Optional */
int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc);
int (*get_cc)(struct tcpc_dev *dev, enum typec_cc_status *cc1,
  enum typec_cc_status *cc2);



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 04/18] staging: typec: tcpm: Add helpers for exporting current-limit through a psy

[Guenter Roeck]

On 08/06/2017 05:35 AM, Hans de Goede wrote:

Not all type-c port-controller can control the current-limit directly,
in cases where the current limit can not be controlled directly it needs
to be exported so that another driver (e.g. the charger driver) can pick
the limit up and configure the system accordingly.

The power-supply subsys already provides infrastructure for this,
power-supply devices have the notion of being supplied by another
power-supply and have properties through which we can export the
current-limit.

This commits adds 2 helper functions for use by port-controller drivers
which want to export the current-limit info in this way:

int tcpm_register_psy(struct device *dev, struct tcpc_dev *tcpc,
  const char *name);
int tcpm_set_current_limit_psy(struct tcpc_dev *tcpc, u32 max_ma, u32 mv);

Signed-off-by: Hans de Goede 
---
  drivers/staging/typec/tcpm-helpers.c | 66 
  drivers/staging/typec/tcpm.h |  9 +
  2 files changed, 75 insertions(+)

diff --git a/drivers/staging/typec/tcpm-helpers.c 
b/drivers/staging/typec/tcpm-helpers.c
index 0c87ec9936e1..8f7699576878 100644
--- a/drivers/staging/typec/tcpm-helpers.c
+++ b/drivers/staging/typec/tcpm-helpers.c
@@ -20,6 +20,60 @@
  
  #include "tcpm.h"
  
+static int tcpm_psy_get_property(struct power_supply *psy,

+enum power_supply_property psp,
+union power_supply_propval *val)
+{
+   struct tcpc_dev *tcpc = power_supply_get_drvdata(psy);
+
+   switch (psp) {
+   case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+   val->intval = tcpc->supply_voltage * 1000; /* mV -> µV */
+   break;
+   case POWER_SUPPLY_PROP_CURRENT_MAX:
+   val->intval = tcpc->current_limit * 1000; /* mA -> µA */
+   break;
+   default:
+   return -ENODATA;
+   }
+
+   return 0;
+}
+
+static enum power_supply_property tcpm_psy_properties[] = {
+   POWER_SUPPLY_PROP_VOLTAGE_NOW,
+   POWER_SUPPLY_PROP_CURRENT_MAX,
+};
+
+int tcpm_register_psy(struct device *dev, struct tcpc_dev *tcpc,
+ const char *name)


This is misleading since it relies on devm functions, and there is no 
unregister function.
Overall I am not too happy with tying this all into tcpm. The functions are not 
really tcpm
related. Not that I have a better idea how to handle this right now, but 
conceptually it
doesn't seem correct.


+{
+   struct power_supply_config psy_cfg = {};
+   struct power_supply_desc *desc;
+   int ret = 0;
+
+   desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
+   if (!desc)
+   return -ENOMEM;
+
+   desc->name   = name;
+   desc->type   = POWER_SUPPLY_TYPE_USB;
+   desc->properties = tcpm_psy_properties;
+   desc->num_properties = ARRAY_SIZE(tcpm_psy_properties);
+   desc->get_property   = tcpm_psy_get_property;
+   psy_cfg.drv_data= tcpc;
+
+   tcpc->psy = devm_power_supply_register(dev, desc, _cfg);
+   if (IS_ERR(tcpc->psy)) {
+   ret = PTR_ERR(tcpc->psy);
+   tcpc->psy = NULL;
+   dev_err(dev, "Error registering power-supply: %d\n", ret);
+   }
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(tcpm_register_psy);
+
  /* Generic (helper) implementations for some tcpc_dev callbacks */
  int tcpm_get_usb2_current_limit_extcon(struct tcpc_dev *tcpc)
  {
@@ -58,3 +112,15 @@ int tcpm_get_usb2_current_limit_extcon(struct tcpc_dev 
*tcpc)
return current_limit;
  }
  EXPORT_SYMBOL_GPL(tcpm_get_usb2_current_limit_extcon);
+
+int tcpm_set_current_limit_psy(struct tcpc_dev *tcpc, u32 max_ma, u32 mv)
+{
+   tcpc->supply_voltage = mv;
+   tcpc->current_limit = max_ma;
+
+   if (tcpc->psy)
+   power_supply_changed(tcpc->psy);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(tcpm_set_current_limit_psy);
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 35e8c1e7dba0..1b1475b487b5 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -17,6 +17,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include "pd.h"
  
@@ -129,6 +130,10 @@ struct tcpc_dev {

struct tcpc_mux_dev *mux;
/* Used by tcpm_get_usb2_current_limit_extcon helpers */
struct extcon_dev *usb2_extcon;
+   /* Used by tcpm_set_current_limit_psy helpers */
+   struct power_supply *psy;
+   u32 current_limit;
+   u32 supply_voltage;
  };
  
  struct tcpm_port;

@@ -154,7 +159,11 @@ void tcpm_pd_transmit_complete(struct tcpm_port *port,
  void tcpm_pd_hard_reset(struct tcpm_port *port);
  void tcpm_tcpc_reset(struct tcpm_port *port);
  
+int tcpm_register_psy(struct device *dev, struct tcpc_dev *tcpc,

+ const char *name);
+
  /* Generic (helper) implementations for some tcpc_dev callbacks */
  int 

Re: [PATCH 02/18] staging: typec: tcpm: Add extcon helper functions for USB2 current limit detect

[Guenter Roeck]

On 08/06/2017 05:35 AM, Hans de Goede wrote:

Some type-c port-controllers, such as the fusb302 port-controller, rely
on an external device doing USB2 charger-type detection.

Existing PMIC (and charger) drivers already use extcon to communicate the
detected charger-type from the PMIC (extcon) driver to the charger driver.

Rather then inventing a new API for USB2 charger-type detection
specifically for use with the tcpm code, lets simply re-use the existing
support. This will also allow re-using existing PMIC extcon drivers such
as the axp288 and Intel Whiskey Cove drivers as is on devices where these
are combined with a fusb302 (or in the future another port-controller
which relies on external USB2 charger-type detection).

This commit adds a helper function which tcpc drivers can use to easily
hook into existing PMIC extcon drivers for USB2 charger-type detection:

int tcpm_get_usb2_current_limit_extcon(struct tcpc_dev *tcpc);

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
  drivers/staging/typec/tcpm.c | 40 
  drivers/staging/typec/tcpm.h |  6 ++
  2 files changed, 46 insertions(+)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 9f5adace4309..06bb0e640bcf 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -16,6 +16,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -3532,6 +3533,45 @@ void tcpm_unregister_port(struct tcpm_port *port)
  }
  EXPORT_SYMBOL_GPL(tcpm_unregister_port);
  
+/* Generic (helper) implementations for some tcpc_dev callbacks */

+int tcpm_get_usb2_current_limit_extcon(struct tcpc_dev *tcpc)
+{
+   struct extcon_dev *extcon = tcpc->usb2_extcon;
+   int current_limit = 0;
+   unsigned long timeout;
+
+   if (!extcon)
+   return 0;
+
+   /*
+* USB2 Charger detection may still be in progress when we get here,
+* this can take upto 600ms, wait 800ms max.
+*/
+   timeout = jiffies + msecs_to_jiffies(800);
+   do {
+   if (extcon_get_state(extcon, EXTCON_CHG_USB_SDP) == 1) {
+   current_limit = 500;
+   break;
+   }
+
+   if (extcon_get_state(extcon, EXTCON_CHG_USB_CDP) == 1 ||
+   extcon_get_state(extcon, EXTCON_CHG_USB_ACA) == 1) {
+   current_limit = 1500;
+   break;
+   }
+
+   if (extcon_get_state(extcon, EXTCON_CHG_USB_DCP) == 1) {
+   current_limit = 2000;
+   break;
+   }
+
+   msleep(50);
+   } while (time_before(jiffies, timeout));
+
+   return current_limit;
+}
+EXPORT_SYMBOL_GPL(tcpm_get_usb2_current_limit_extcon);
+


Not really sure about this one. Should it be part of low level drivers ?

Guenter


  MODULE_AUTHOR("Guenter Roeck <gro...@chromium.org>");
  MODULE_DESCRIPTION("USB Type-C Port Manager");
  MODULE_LICENSE("GPL");
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 01b7d89379a3..35e8c1e7dba0 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -16,6 +16,7 @@
  #define __LINUX_USB_TCPM_H
  
  #include 

+#include 
  #include 
  #include "pd.h"
  
@@ -126,6 +127,8 @@ struct tcpc_dev {

int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
   const struct pd_message *msg);
struct tcpc_mux_dev *mux;
+   /* Used by tcpm_get_usb2_current_limit_extcon helpers */
+   struct extcon_dev *usb2_extcon;
  };
  
  struct tcpm_port;

@@ -151,4 +154,7 @@ void tcpm_pd_transmit_complete(struct tcpm_port *port,
  void tcpm_pd_hard_reset(struct tcpm_port *port);
  void tcpm_tcpc_reset(struct tcpm_port *port);
  
+/* Generic (helper) implementations for some tcpc_dev callbacks */

+int tcpm_get_usb2_current_limit_extcon(struct tcpc_dev *tcpc);
+
  #endif /* __LINUX_USB_TCPM_H */



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 06/18] staging: typec: fusb302: Get max snk mv/ma/mw from device-properties

[Guenter Roeck]

On 08/06/2017 05:35 AM, Hans de Goede wrote:

This is board specific info so it should come from board config, such
as devicetree.

I've chosen to prefix these with "fcs," treating them as fusb302 driver
specific. We may want to revisit to replace these with properties which
are part of a (to be written) generic type-c controller devicetree
binding.


You might want to add dt maintainers here.


Signed-off-by: Hans de Goede <hdego...@redhat.com> > ---
  drivers/staging/typec/TODO  |  5 +
  drivers/staging/typec/fusb302/fusb302.c | 18 +-
  2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/typec/TODO b/drivers/staging/typec/TODO
index bc1f97a2d1bf..98b0dbd5752b 100644
--- a/drivers/staging/typec/TODO
+++ b/drivers/staging/typec/TODO
@@ -11,5 +11,10 @@ tcpm:
  tcpci:
  - Test with real hardware
  
+fusb302:

+- We may want to replace the  "fcs,max-snk-mv", "fcs,max-snk-ma",
+  "fcs,max-snk-mw" and "fcs,operating-snk-mw" device(tree) properties with
+  properties which are part of a generic type-c controller devicetree binding.
+

I think preferred units would be -microvolt, -,icroamp, and -microwatt.


  Please send patches to Guenter Roeck <li...@roeck-us.net> and copy
  Heikki Krogerus <heikki.kroge...@linux.intel.com>.
diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index 6baed06a3c0d..8ceaef8ecf17 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -90,6 +90,7 @@ struct fusb302_chip {
struct i2c_client *i2c_client;
struct tcpm_port *tcpm_port;
struct tcpc_dev tcpc_dev;
+   struct tcpc_config tcpc_config;
  
  	struct regulator *vbus;
  
@@ -1198,7 +1199,6 @@ static const struct tcpc_config fusb302_tcpc_config = {
  
  static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)

  {
-   fusb302_tcpc_dev->config = _tcpc_config;
fusb302_tcpc_dev->init = tcpm_init;
fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
fusb302_tcpc_dev->set_cc = tcpm_set_cc;
@@ -1684,7 +1684,9 @@ static int fusb302_probe(struct i2c_client *client,
  {
struct fusb302_chip *chip;
struct i2c_adapter *adapter;
+   struct device *dev = >dev;
int ret = 0;
+   u32 val;
  
  	adapter = to_i2c_adapter(client->dev.parent);

if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
@@ -1699,8 +1701,22 @@ static int fusb302_probe(struct i2c_client *client,
chip->i2c_client = client;
i2c_set_clientdata(client, chip);
chip->dev = >dev;
+   chip->tcpc_config = fusb302_tcpc_config;
+   chip->tcpc_dev.config = >tcpc_config;
mutex_init(>lock);
  
+	if (device_property_read_u32(dev, "fcs,max-snk-mv", ) == 0)

+   chip->tcpc_config.max_snk_mv = val;
+
+   if (device_property_read_u32(dev, "fcs,max-snk-ma", ) == 0)
+   chip->tcpc_config.max_snk_ma = val;
+
+   if (device_property_read_u32(dev, "fcs,max-snk-mw", ) == 0)
+   chip->tcpc_config.max_snk_mw = val;
+
+   if (device_property_read_u32(dev, "fcs,operating-snk-mw", ) == 0)
+   chip->tcpc_config.operating_snk_mw = val;
+
ret = fusb302_debugfs_init(chip);
if (ret < 0)
return ret;



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: New Driver for electrical energy measurement

[Guenter Roeck]

On 07/14/2017 01:07 AM, Jonathan Cameron wrote:

On Wed, 12 Jul 2017 15:19:40 +0200
Christian Gromm  wrote:


On Wed, 12 Jul 2017 14:51:01 +0200
Greg KH  wrote:


On Wed, Jul 12, 2017 at 02:18:54PM +0200, Christian Gromm wrote:


Hi,

Microchip is planning to introduce a driver for a new companion
chip series capable of electical energy measurement. However, we
are not sure which location in the source tree is the most
suitable. First thought was to put it into /drivers/powercap. But
now we are in discussions whether it would make even more sense to
introduce a new driver for electrical energy measuring or
monitoring.


Why not just have it be an IIO device, I think they have energy
sensors already (adding the iio mailing list to find out...)


We thought about hwmon, but not IIO.

   


Following is a rough overview of the part we want the driver for.
Any input on this will be appreciated.

thanks,
Chris

=== Introduction

Following the recent focus on low-power systems, Microchip has
designed a companion chip series capable of measuring the
electrical energy flow of an electrical system.  The series in
question, EM Chip  is capable of measuring the energy flow (in or
out) of an electrical system by monitoring one or more power
rails.  The 1st chip of the series, EM Chip is able to measure the
net energy flow over 4 different power rails.  The chip itself is a
small QFN16 (4mm x 4mm x 0.5mm) or WLCSP16 (2.25mm x 2.2mm) package.

=== Theory of Operation

In order to measure the amount of energy entering/exiting a system,
a small shunt resistor is interleaved on a power rail. By measuring
the small voltage drop across the shunt resistor (a known value),
the electrical current is measured.

I = U/R

Depending on whether the system is consuming or producing energy,
the measured current value can be either a positive or negative
number.

To get the instantaneous power figure we will have to measure the
power rail’s voltage and multiply it with the measured value for
the current

P = V * I = V * U_shunt/R

Knowing the instantaneous power and by making a high enough sample
rate (for measuring the current and the power rail voltage), we can
assume the measured value for power is equal to the average power
figure for the amount of time between 2 sampling moments.

Now that we also know the average power for a given time interval
(dt_x = time between 2 sampling moments; sampling speed is known),
we can measure the energy amount entering or exiting the system
between 2 sampling moments

E_partial_x = P * dt_x

Having the energy information available, we can continue to add the
subsequent energy values for as long as the system is active. The
amount of energy is the sum of all the partial energy values
measured for each time interval E = Sum (E_partial_x), where x can
take values from 0 till infinite.


=== Chip Operation

The EM Chip chip uses the same principles of operation as presented
in the “Theory of Operation” and it does so for a number of 4
channels. 4 independent power rails can be energy monitored in the
same time.  The chip is controlled over I2C/SMBus by an I2C/SMBus
master.

In order to reduce the amount of information traffic between the EM
Chip and the entity asking for its information, the chip features
large accumulator energy registers for the accumulated energy
values. It can accumulate up to 2^24 power values. Depending on the
selected chip’s sampling speed, such a number of samples are
produced in about 4.5 hours (fastest sampling speed) up to 582.5
hours or more than 24 days (lowest sampling speed).

EM Chip measures the power as a 28-bits number. The 28-bits number
is the multiplication result of the 16-bits number used to measure
the power rail voltage and the 12-bits number used for measuring
the voltage drop across the power rail shunt resistor.

If bidirectional energy flow is required, the power is measured as a
27-bits number and 1 bit for the sign. When no bidirectional flow is
needed, the power value is measured as an unsigned 28-bits number.

Due to the relatively large size of the accumulated energy
registers inside the chip (48 bits), in the worst case scenario
(power values are full scale numbers), a number of 2^20 full-scale
power values can be measured before energy register’s overflow.
Using the fastest sampling speed, the accumulated energy registers
overflows only after a bit over 17 minutes, while at the lowest
sampling speed, it would overflow in over 36 hours.

Thus, the chip requires infrequent reads of the accumulated energy
registers. Even in the worst case scenario, the time between 2
consecutive energy accumulator reads can be over 17 minutes !

In order to keep a longer history of energy measurements, an
I2C/SMBus master (e.g. SoC) would read the accumulated energy
register values and then use larger “soft” accumulated energy
registers to extend the maximum overflow period.

The EM 

Re: [PATCH v3] staging: typec: Fix endianness warning discovered by sparse

[Guenter Roeck]

On Fri, Jul 07, 2017 at 10:24:32AM +1000, Thomas Gardner wrote:
> The below warning is resolved by removing the cpu_to_le32() call. This
> call was redundant; vdm_run_state_machine() ensures that SVDM responses
> have the correct endianness before sending.
> 
> typec/tcpm.c:1019:49: warning: incorrect type in assignment (different base 
> types)
> typec/tcpm.c:1019:49:expected unsigned int [unsigned] [usertype] 
> typec/tcpm.c:1019:49:got restricted __le32 [usertype] 
> 
> Signed-off-by: Thomas Gardner <t...@fastmail.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/tcpm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index 20eb4ebcf8c3..2195c80235a1 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -1015,8 +1015,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const 
> __le32 *payload, int cnt,
>   if (port->data_role == TYPEC_DEVICE &&
>   port->nr_snk_vdo) {
>   for (i = 0; i <  port->nr_snk_vdo; i++)
> - response[i + 1]
> - = cpu_to_le32(port->snk_vdo[i]);
> + response[i + 1] = port->snk_vdo[i];
>   rlen = port->nr_snk_vdo + 1;
>   }
>   break;
> -- 
> 2.13.2
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: typec: Fix endianness warning discovered by sparse

[Guenter Roeck]

On 07/06/2017 03:54 PM, Thomas Gardner wrote:

The below warning is resolved by removing the cpu_to_le32() call. This
call was redundant; vdm_run_state_machine() ensures that SVDM responses
have the correct endianness before sending.

typec/tcpm.c:1019:49: warning: incorrect type in assignment (different base 
types)
typec/tcpm.c:1019:49:expected unsigned int [unsigned] [usertype] 
typec/tcpm.c:1019:49:got restricted __le32 [usertype] 

Signed-off-by: Thomas Gardner 
---
  drivers/staging/typec/tcpm.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 20eb4ebcf8c3..0b596f56d18c 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -1015,8 +1015,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const 
__le32 *payload, int cnt,
if (port->data_role == TYPEC_DEVICE &&
port->nr_snk_vdo) {
for (i = 0; i <  port->nr_snk_vdo; i++)
-   response[i + 1]
-   = cpu_to_le32(port->snk_vdo[i]);
+   response[i + 1] = (port->snk_vdo[i]);


Unnecessary ( ).

Guenter


rlen = port->nr_snk_vdo + 1;
}
break;



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: typec: Fix type mismatch found with sparse

[Guenter Roeck]

On 07/05/2017 07:57 PM, Thomas Gardner wrote:

Guenter Roeck<li...@roeck-us.net> / 2017-07-05T07:25-0700

On 07/05/2017 07:00 AM, Thomas Gardner wrote:

The warning below is resolved by casting the LHS to __le32.

typec/tcpm.c:1019:49: warning: incorrect type in assignment (different base 
types)
typec/tcpm.c:1019:49:expected unsigned int [unsigned] [usertype] 
typec/tcpm.c:1019:49:got restricted __le32 [usertype] 

Signed-off-by: Thomas Gardner <t...@fastmail.com>
---
   drivers/staging/typec/tcpm.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 20eb4ebcf8c3..7699bb27a4d9 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -1015,7 +1015,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const 
__le32 *payload, int cnt,
if (port->data_role == TYPEC_DEVICE &&
port->nr_snk_vdo) {
for (i = 0; i <  port->nr_snk_vdo; i++)
-   response[i + 1]
+   ((__le32 *)response)[i + 1]
= cpu_to_le32(port->snk_vdo[i]);
rlen = port->nr_snk_vdo + 1;
}


I think this would just hide a number of at least potential endianness issues
in the driver. response[] should be of type __le32 instead, with everything
that comes with it.


Greetings Guenter, is this conversion actually necessary? The state
machine tries to fix endianness of the response before sending at,

for (i = 0; i < port->vdo_count; i++)
msg.payload[i] = cpu_to_le32(port->vdo_data[i]);

Perhaps the patch should be,

-   response[i + 1]
-   = cpu_to_le32(port->snk_vdo[i]);
+   response[i + 1] = port->snk_vdo[i];

which also clears the warning.



Yes, looking through the code, that is correct.

Thanks for tracking this down. Can you send an updated patch ?

Guenter


Which makes me wonder, since I don't see any of those warnings - what
does it take to see them ?


I can see them by running,

make C=2 ./drivers/staging/typec/


Guenter







___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: typec: Fix type mismatch found with sparse

[Guenter Roeck]

On 07/05/2017 07:00 AM, Thomas Gardner wrote:

The warning below is resolved by casting the LHS to __le32.

typec/tcpm.c:1019:49: warning: incorrect type in assignment (different base 
types)
typec/tcpm.c:1019:49:expected unsigned int [unsigned] [usertype] 
typec/tcpm.c:1019:49:got restricted __le32 [usertype] 

Signed-off-by: Thomas Gardner 
---
  drivers/staging/typec/tcpm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 20eb4ebcf8c3..7699bb27a4d9 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -1015,7 +1015,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const 
__le32 *payload, int cnt,
if (port->data_role == TYPEC_DEVICE &&
port->nr_snk_vdo) {
for (i = 0; i <  port->nr_snk_vdo; i++)
-   response[i + 1]
+   ((__le32 *)response)[i + 1]
= cpu_to_le32(port->snk_vdo[i]);
rlen = port->nr_snk_vdo + 1;
}


I think this would just hide a number of at least potential endianness issues
in the driver. response[] should be of type __le32 instead, with everything
that comes with it.

Which makes me wonder, since I don't see any of those warnings - what does it 
take
to see them ?

Guenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] staging: typec: add tcpci_read16_le.

[Guenter Roeck]

On 06/19/2017 03:11 AM, ? ? wrote:

From: Pan Li 

Add tcpci_read16_le for pd_message header type __le16.
> Signed-off-by: Pan Li 
---
  drivers/staging/typec/tcpci.c | 15 ++-
  1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index 2adb543..8da4a38 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -46,12 +46,17 @@ static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev 
*tcpc)
return container_of(tcpc, struct tcpci, tcpc);
  }
  
-static int tcpci_read16(struct tcpci *tcpci, unsigned int reg,

-   unsigned int *val)
+static int tcpci_read16(struct tcpci *tcpci, unsigned int reg, u16 *val)
+


This is a separate fix which probably warrants a separate patch, or at the very 
least
should be mentioned in the subject/description.

Wonder if there is a valid use case for this function, though. Isn't status 
really
also little endian ? I'll have to look into the code.

Thanks,
Guenter


  {
return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u16));
  }
  
+static int tcpci_read16_le(struct tcpci *tcpci, unsigned int reg, __le16 *val)

+{
+   return regmap_raw_read(tcpci->regmap, reg, val, sizeof(__le16));
+}
+
  static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
  {
return regmap_raw_write(tcpci->regmap, reg, , sizeof(u16));
@@ -355,7 +360,8 @@ static int tcpci_init(struct tcpc_dev *tcpc)
  static irqreturn_t tcpci_irq(int irq, void *dev_id)
  {
struct tcpci *tcpci = dev_id;
-   unsigned int status, reg;
+   unsigned int reg;
+   u16 status;
  
  	tcpci_read16(tcpci, TCPC_ALERT, );
  
@@ -389,8 +395,7 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id)
  
  		regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, );
  
-		tcpci_read16(tcpci, TCPC_RX_HDR, );

-   msg.header = reg;
+   tcpci_read16_le(tcpci, TCPC_RX_HDR, );
  
  		if (WARN_ON(cnt > sizeof(msg.payload)))

cnt = sizeof(msg.payload);



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: fusb302: don't bitshift __le16 type

[Guenter Roeck]

On Fri, Jun 16, 2017 at 07:45:56PM +0200, Frans Klaver wrote:
> The header field in struct pd_message is declared as an __le16 type. The
> data in the message is supposed to be little endian. This means we don't
> have to go and shift the individual bytes into position when we're
> filling the buffer, we can just copy the contents right away. As an
> added benefit we don't get fishy results on big endian systems anymore.
> 
> Signed-off-by: Frans Klaver <franskla...@gmail.com>

Reviewed-by: Guenter Roeck <li...@roeck-us.net>

> ---
>  drivers/staging/typec/fusb302/fusb302.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/typec/fusb302/fusb302.c 
> b/drivers/staging/typec/fusb302/fusb302.c
> index 4a356e509fe4..03a3809d18f0 100644
> --- a/drivers/staging/typec/fusb302/fusb302.c
> +++ b/drivers/staging/typec/fusb302/fusb302.c
> @@ -1039,8 +1039,8 @@ static int fusb302_pd_send_message(struct fusb302_chip 
> *chip,
>   }
>   /* packsym tells the FUSB302 chip that the next X bytes are payload */
>   buf[pos++] = FUSB302_TKN_PACKSYM | (len & 0x1F);
> - buf[pos++] = msg->header & 0xFF;
> - buf[pos++] = (msg->header >> 8) & 0xFF;
> + memcpy([pos], >header, sizeof(msg->header));
> + pos += sizeof(msg->header);
>  
>   len -= 2;
>   memcpy([pos], msg->payload, len);
> -- 
> 2.13.0
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel