R: [PATCH] arm64: fix missing include in asm uaccess.h
> On Wed, Nov 11, 2020 at 12:58:26AM +, Al Viro wrote:
> > On Wed, Nov 11, 2020 at 01:44:38AM +0100, Ansuel Smith wrote:
> > > Fix a compilation error as PF_KTHREAD is defined in linux/sched.h and
> > > this is missing.
> > >
> > > Fixes: df325e05a682 ("arm64: Validate tagged addresses in access_ok()
> > > called from kernel threads")
> > > Signed-off-by: Ansuel Smith
> > > ---
> > > arch/arm64/include/asm/uaccess.h | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/arch/arm64/include/asm/uaccess.h
> b/arch/arm64/include/asm/uaccess.h
> > > index 991dd5f031e4..51a4f63f464a 100644
> > > --- a/arch/arm64/include/asm/uaccess.h
> > > +++ b/arch/arm64/include/asm/uaccess.h
> > > @@ -7,6 +7,8 @@
> > > #ifndef __ASM_UACCESS_H
> > > #define __ASM_UACCESS_H
> > >
> > > +#include
> > > +
> > > #include
> > > #include
> > > #include
> >
> > NAK. The real bug is in arch/arm64/include/asm/asm-prototypes.h -
> > it has no business pulling asm/uaccess.h
> >
> > Just include linux/uaccess.h instead.
>
> BTW,
> $ grep -n uaccess.h `find -name asm-prototypes.h`
> ./arch/alpha/include/asm/asm-prototypes.h:7:#include
> ./arch/arm64/include/asm/asm-prototypes.h:18:#include
> ./arch/ia64/include/asm/asm-prototypes.h:12:#include
> ./arch/mips/include/asm/asm-prototypes.h:6:#include
> ./arch/powerpc/include/asm/asm-prototypes.h:14:#include
> ./arch/sparc/include/asm/asm-prototypes.h:9:#include
> ./arch/x86/include/asm/asm-prototypes.h:3:#include
>
> Spot the irregularity...
>
> While we are at it,
> $ git grep -n -w '#.*include.*asm/uaccess.h'
> arch/arm64/include/asm/asm-prototypes.h:18:#include
> arch/nds32/math-emu/fpuemu.c:5:#include
> arch/powerpc/kvm/book3s_xive_native.c:15:#include
> arch/powerpc/mm/book3s64/radix_pgtable.c:30:#include
> drivers/s390/net/ctcm_mpc.c:50:#include/* instead
of
> ok ? */
> include/linux/uaccess.h:11:#include
>
> The last one is the only such include that should exist; drivers/s390 one
> is obviously a false positive. And IMO the right thing to do is to
> replace the remaining arch/* instances with includes of linux/uaccess.h.
>
> All of those are asking for trouble; any change moving e.g. a common
> variant of some primitive into linux/uaccess.h might end up breaking
> those.
Thx for the quick response. I found this error while working on a qcom
driver that
use this include. I can confirm that by using linux/uaccess.h the problem is
solved.
RE: [PATCH v2 1/2] devfreq: qcom: Add L2 Krait Cache devfreq scaling driver
> On Wed, Sep 30, 2020 at 01:56:01PM +0200, ansuels...@gmail.com > wrote: > > > Subject: Re: [PATCH v2 1/2] devfreq: qcom: Add L2 Krait Cache devfreq > > > scaling driver > > > > > > On Tue, Sep 29, 2020 at 06:29:24PM +0200, Ansuel Smith wrote: > > > > Qcom L2 Krait CPUs use the generic cpufreq-dt driver and doesn't > > actually > > > > scale the Cache frequency when the CPU frequency is changed. This > > > > devfreq driver register with the cpu notifier and scale the Cache > > > > based on the max Freq across all core as the CPU cache is shared > across > > > > all of them. If provided this also scale the voltage of the regulator > > > > attached to the CPU cache. The scaling logic is based on the CPU freq > > > > and the 3 scaling interval are set by the device dts. > > > > > > > > > > I have raised this concern before. I am worried this kind of independent > > > CPU and cache frequency controls make way for clkscrew kind of > attacks. > > > Why can't the clocks be made parent/child or secondary and > automatically > > > updated when CPU clocks are changed. > > > > > > > I don't think I understand this fully. Anyway about the clkscrew attack, > the > > range are set on the dts so unless someone actually wants to have a > > vulnerable system, the range can't be changes at runtime. The devfreq > > governor is set to immutable and can't be changes AFAIK. > > > > I don't think that matters, we are talking about Secure/Non-secure > boundary > here. DT can be modified or simple a rogue devfreq module can do all the > bad things. > Well it's what is happening right now (cpu at max + l2 at low) and from my test the system is just slowed down. But you are right about the security concerns. > > About 'automatically updated when CPU changes', the cache is shared > across 2 > > core and they scale independently. We can be in situation where one cpu > is > > at max and one at idle freq and the cache is set to idle. To fix this at > > every change the clk should find the max value and I think this would > make > > all the clk scaling very slow. > > This sounds like we are going back to coupled idle states kind of setup. > Sorry we don't want those anymore. > > > If you have any suggestion on how I can implement this better, I'm > > more than happy to address them. For now, the lack of this kind of cache > > scale, make the system really slow since by default the init of the cpu and > > cache clks put them at the lowest frequency and nobody changes that. > > (we have cpufreq scaling support but the cache is never actually scaled) > > As I mentioned, if this needs to be done in OSPM, then hide it in the clock > building right dependencies. Clk will even have refcount so that one idle > CPU won't bring the cache down to idle frequency. > What I really can't understand is how I can describe the CPU freq interval. Since I can't use dts should I hardcode them? (cpu have more opp than the l2 cache, they are not mapped 1:1) > -- > Regards, > Sudeep
RE: [PATCH v2 1/2] devfreq: qcom: Add L2 Krait Cache devfreq scaling driver
> Subject: Re: [PATCH v2 1/2] devfreq: qcom: Add L2 Krait Cache devfreq > scaling driver > > On Tue, Sep 29, 2020 at 06:29:24PM +0200, Ansuel Smith wrote: > > Qcom L2 Krait CPUs use the generic cpufreq-dt driver and doesn't actually > > scale the Cache frequency when the CPU frequency is changed. This > > devfreq driver register with the cpu notifier and scale the Cache > > based on the max Freq across all core as the CPU cache is shared across > > all of them. If provided this also scale the voltage of the regulator > > attached to the CPU cache. The scaling logic is based on the CPU freq > > and the 3 scaling interval are set by the device dts. > > > > I have raised this concern before. I am worried this kind of independent > CPU and cache frequency controls make way for clkscrew kind of attacks. > Why can't the clocks be made parent/child or secondary and automatically > updated when CPU clocks are changed. > > -- > Regards, > Sudeep I don't think I understand this fully. Anyway about the clkscrew attack, the range are set on the dts so unless someone actually wants to have a vulnerable system, the range can't be changes at runtime. The devfreq governor is set to immutable and can't be changes AFAIK. About 'automatically updated when CPU changes', the cache is shared across 2 core and they scale independently. We can be in situation where one cpu is at max and one at idle freq and the cache is set to idle. To fix this at every change the clk should find the max value and I think this would make all the clk scaling very slow. If you have any suggestion on how I can implement this better, I'm more than happy to address them. For now, the lack of this kind of cache scale, make the system really slow since by default the init of the cpu and cache clks put them at the lowest frequency and nobody changes that. (we have cpufreq scaling support but the cache is never actually scaled)
RE: [PATCH 2/2] dt-bindings: devfreq: Document L2 Krait CPU Cache devfreq driver
> -Original Message-
> From: Rob Herring
> Sent: Monday, September 28, 2020 8:09 PM
> To: Ansuel Smith
> Cc: MyungJoo Ham ; Kyungmin Park
> ; Chanwoo Choi
> ; linux...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] dt-bindings: devfreq: Document L2 Krait CPU
> Cache devfreq driver
>
> On Sun, Sep 27, 2020 at 06:05:13PM +0200, Ansuel Smith wrote:
> > Document dedicated L2 Krait CPU Cache devfreq scaling driver.
> >
> > Signed-off-by: Ansuel Smith
> > ---
> > .../bindings/devfreq/krait-cache-devfreq.yaml | 77
> +++
> > 1 file changed, 77 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/devfreq/krait-
> cache-devfreq.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/devfreq/krait-cache-
> devfreq.yaml b/Documentation/devicetree/bindings/devfreq/krait-cache-
> devfreq.yaml
> > new file mode 100644
> > index ..099ed978e022
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/devfreq/krait-cache-
> devfreq.yaml
> > @@ -0,0 +1,77 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/cpufreq/krait-cache-devfreq.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: DEVFREQ driver for Krait L2 Cpu Cache Frequency Scaling
>
> Bindings are for h/w devices, not collections of properties for some
> driver. Define a binding for L2 cache and add on to it what you need.
>
Should I still document it in the devfreq directory or somewhere else?
> > +
> > +maintainers:
> > + - Ansuel Smith
> > +
> > +description: |
> > + This Scale the Krait CPU L2 Cache Frequency and optionally voltage
> > + when the Cpu Frequency is changed (using the cpufreq notifier).
> > +
> > + Cache is scaled with the max frequency across all core and the cache
> > + frequency will scale based on the configured threshold in the dts.
> > +
> > + The cache thresholds can be set to 3+ frequency bin, idle, nominal
and
> > + high.
> > +
> > +properties:
> > + compatible:
> > +const: qcom,krait-cache
> > +
> > + clocks:
> > +$ref: "/schemas/types.yaml#/definitions/phandle"
>
> 'clocks' already has a type defined. You just need how many and what
> each entry is.
>
> > +description: Phandle to the L2 CPU clock
> > +
> > + clock-names:
> > +const: "l2"
> > +
> > + voltage-tolerance:
> > +description: Same voltage tolerance of the Krait CPU
>
> Needs a vendor prefix and unit suffix.
>
> > +
> > + l2-cpufreq:
> > +description: |
> > + Threshold used by the driver to scale the L2 cache.
> > + If the max CPU Frequency is more than the set frequency,
> > + the driver will transition to the next frequency bin.
> > + Value is in kHz
> > +$ref: /schemas/types.yaml#/definitions/uint32-array
> > +minItems: 3
> > +items:
> > + - description: idle
> > + - description: nominal
> > + - description: high
> > +
> > + l2-supply:
> > +$ref: "/schemas/types.yaml#/definitions/phandle"
> > +description: Phandle to the L2 regulator supply.
> > +
> > + opp-table: true
> > +
> > +required:
> > + - compatible
> > + - clocks
> > + - clock-names
> > + - voltage-tolerance
> > + - l2-cpufreq
> > +
> > +examples:
> > + - |
> > +qcom-krait-cache {
> > + compatible = "qcom,krait-cache";
> > + clocks = < 4>;
> > + clock-names = "l2";
> > + l2-cpufreq = <384000 60 120>;
> > + l2-supply = <_s1a>;
> > +
> > + operating-points = <
>
> Not documented and generally deprecated.
>
Ok will change to v2.
> > +/* kHzuV */
> > +384000 110
> > +100 110
> > +120 115
> > + >;
> > +};
> > --
> > 2.27.0
> >
RE: [RFC PATCH v6 0/8] Add support for ipq8064 tsens
> -Original Message- > From: Amit Kucheria > Sent: Monday, September 28, 2020 1:33 PM > To: Ansuel Smith > Cc: Andy Gross ; Bjorn Andersson > ; Zhang Rui ; Daniel > Lezcano ; Rob Herring ; > linux-arm-msm ; Linux PM list p...@vger.kernel.org>; open list:OPEN FIRMWARE AND FLATTENED DEVICE > TREE BINDINGS ; LKML ker...@vger.kernel.org> > Subject: Re: [RFC PATCH v6 0/8] Add support for ipq8064 tsens > > Hi Ansuel, > > Just a quick note to say that I'm not ignoring this, just on > vacations. I'll be back to review this in a few days. > > Regards, > Amit > Thx a lot. Was thinking of resending this but I will wait.
RE: [PATCH v3 3/4] of_net: add mac-address-increment support
> -Original Message- > From: Rob Herring > Sent: Friday, September 25, 2020 8:24 PM > To: Ansuel Smith > Cc: Miquel Raynal ; Richard Weinberger > ; Vignesh Raghavendra ; David S. > Miller ; Jakub Kicinski ; > Andrew Lunn ; Heiner Kallweit > ; Russell King ; Frank > Rowand ; Boris Brezillon > ; MTD Maling List ; > devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; netdev > > Subject: Re: [PATCH v3 3/4] of_net: add mac-address-increment support > > On Sun, Sep 20, 2020 at 3:57 AM Ansuel Smith > wrote: > > > > Lots of embedded devices use the mac-address of other interface > > extracted from nvmem cells and increments it by one or two. Add two > > bindings to integrate this and directly use the right mac-address for > > the interface. Some example are some routers that use the gmac > > mac-address stored in the art partition and increments it by one for the > > wifi. mac-address-increment-byte bindings is used to tell what byte of > > the mac-address has to be increased (if not defined the last byte is > > increased) and mac-address-increment tells how much the byte decided > > early has to be increased. > > I'm inclined to say if there's a platform specific way to transform > MAC addresses, then there should be platform specific code to do that > which then stuffs the DT using standard properties. Otherwise, we have > a never ending stream of 'generic' properties to try to handle > different platforms' cases. > > Rob I agree about the 'never ending stream'... But I think the increment feature is not that platform specific. I will quote some number by another patch that tried to implement the same feature in a different way, [1] * mtd-mac-addressused 497 times in 357 device tree files * mtd-mac-address-increment used 74 times in 58 device tree files * mtd-mac-address-increment-byte used 1 time in 1 device tree file The mtd-mac-address is what this patchset is trying to fix with the nvmem support. The increment is much more than 74 times since it doesn't count SoC that have wifi integrated (it's common practice for SoC with integrated wifi to take the switch mac and use it to set the wifi mac) Actually what is really specific is the increment-byte that can be dropped if we really want to. I still think the increment feature would be very useful to add full support for mac-address extracted from nvmem cell. [1] https://patchwork.ozlabs.org/project/netdev/patch/1555445100-30936-1-git-send-email-yn...@true.cz/
RE: [PATCH v2 2/2] dt: bindings: ath10k: Document qcom,ath10k-pre-calibration-data-mtd
> -Original Message- > From: Rob Herring > Sent: Wednesday, September 23, 2020 10:58 PM > To: Ansuel Smith > Cc: Kalle Valo ; David S. Miller > ; Jakub Kicinski ; linux- > wirel...@vger.kernel.org; net...@vger.kernel.org; > devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; > ath...@lists.infradead.org > Subject: Re: [PATCH v2 2/2] dt: bindings: ath10k: Document qcom,ath10k- > pre-calibration-data-mtd > > On Fri, Sep 18, 2020 at 08:11:03PM +0200, Ansuel Smith wrote: > > Document use of qcom,ath10k-pre-calibration-data-mtd bindings used to > > define from where the driver will load the pre-cal data in the defined > > mtd partition. > > > > Signed-off-by: Ansuel Smith > > --- > > .../devicetree/bindings/net/wireless/qcom,ath10k.txt | 12 +--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git > a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > > index b61c2d5a0..568364243 100644 > > --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > > +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > > @@ -15,9 +15,9 @@ and also uses most of the properties defined in this > doc (except > > "qcom,ath10k-calibration-data"). It uses "qcom,ath10k-pre-calibration- > data" > > to carry pre calibration data. > > > > -In general, entry "qcom,ath10k-pre-calibration-data" and > > -"qcom,ath10k-calibration-data" conflict with each other and only one > > -can be provided per device. > > +In general, entry "qcom,ath10k-pre-calibration-data", > > +"qcom,ath10k-calibration-data-mtd" and "qcom,ath10k-calibration- > data" conflict with > > +each other and only one can be provided per device. > > > > SNOC based devices (i.e. wcn3990) uses compatible string > "qcom,wcn3990-wifi". > > > > @@ -63,6 +63,12 @@ Optional properties: > > hw versions. > > - qcom,ath10k-pre-calibration-data : pre calibration data as an array, > > the length can vary between hw versions. > > +- qcom,ath10k-pre-calibration-data-mtd : > > mtd is a Linuxism. > > > + Usage: optional > > + Value type: > > + Definition: pre calibration data read from mtd partition. Take 3 > value, the > > + mtd to read data from, the offset in the mtd partition and > the > > The phandle is the mtd or partition? > > Maybe you should be using nvmem binding here. > The phandle is to the mtd. You are right about nvmem... Problem is that nvmem for mtd is still not supported. I already sent a patch to fix this in the mtd mailing list but I'm waiting for review... If that will be accepted, I can convert this patch to use nvmem api. > > + size of data to read. > > - -supply: handle to the regulator device tree node > >optional "supply-name" are "vdd-0.8-cx-mx", > >"vdd-1.8-xo", "vdd-1.3-rfa", "vdd-3.3-ch0", > > -- > > 2.27.0 > >
RE: R: [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
> On Sun, Sep 20, 2020 at 02:39:39AM +0200, ansuels...@gmail.com > wrote: > > > > > > > -Messaggio originale- > > > Da: Andrew Lunn > > > Inviato: domenica 20 settembre 2020 02:31 > > > A: Ansuel Smith > > > Cc: Miquel Raynal ; Richard Weinberger > > > ; Vignesh Raghavendra ; Rob > Herring > > > ; David S. Miller ; Jakub > > > Kicinski ; Heiner Kallweit ; > > > Russell King ; Frank Rowand > > > ; Boris Brezillon ; > linux- > > > m...@lists.infradead.org; devicet...@vger.kernel.org; linux- > > > ker...@vger.kernel.org; net...@vger.kernel.org > > > Oggetto: Re: [PATCH v2 4/4] dt-bindings: net: Document use of mac- > > > address-increment > > > > > > > + mac-address-increment: > > > > +description: > > > > + The MAC address can optionally be increased (or decreased using > > > > + negative values) from the original value readed (from a nvmem > > cell > > > > > > Read is irregular, there is no readed, just read. > > > > > > > + for example). This can be used if the mac is readed from a > > dedicated > > > > + partition and must be increased based on the number of device > > > > + present in the system. > > > > > > You should probably add there is no underflow/overflow to other bytes > > > of the MAC address. 00:01:02:03:04:ff + 1 == 00:01:02:03:04:00. > > > > > > > +minimum: -255 > > > > +maximum: 255 > > > > + > > > > + mac-address-increment-byte: > > > > +description: > > > > + If 'mac-address-increment' is defined, this will tell what byte > > of > > > > + the mac-address will be increased. If 'mac-address-increment' is > > > > + not defined, this option will do nothing. > > > > +default: 5 > > > > +minimum: 0 > > > > +maximum: 5 > > > > > > Is there a real need for this? A value of 0 seems like a bad idea, > > > since a unicast address could easily become a multicast address, which > > > is not valid for an interface address. It also does not seem like a > > > good idea to allow the OUI to be changed. So i think only bytes 3-5 > > > should be allowed, but even then, i don't think this is needed, unless > > > you do have a clear use case. > > > > > > Andrew > > > > Honestly the mac-address-increment-byte is added to give user some > control > > but I > > don't really have a use case for it. Should I limit it to 3 or just remove > > the function? > > If you have no use case, just remove it and document that last byte > will be incremented. I somebody does need it, it can be added in a > backwards compatible way. > > Andrew I just rechecked mac-address-increment-byte and we have one device that use it and would benefits from this. I will change the Documentation to min 3 and leave it.
R: [PATCH v2 4/4] dt-bindings: net: Document use of mac-address-increment
> -Messaggio originale- > Da: Andrew Lunn > Inviato: domenica 20 settembre 2020 02:31 > A: Ansuel Smith > Cc: Miquel Raynal ; Richard Weinberger > ; Vignesh Raghavendra ; Rob Herring > ; David S. Miller ; Jakub > Kicinski ; Heiner Kallweit ; > Russell King ; Frank Rowand > ; Boris Brezillon ; linux- > m...@lists.infradead.org; devicet...@vger.kernel.org; linux- > ker...@vger.kernel.org; net...@vger.kernel.org > Oggetto: Re: [PATCH v2 4/4] dt-bindings: net: Document use of mac- > address-increment > > > + mac-address-increment: > > +description: > > + The MAC address can optionally be increased (or decreased using > > + negative values) from the original value readed (from a nvmem cell > > Read is irregular, there is no readed, just read. > > > + for example). This can be used if the mac is readed from a dedicated > > + partition and must be increased based on the number of device > > + present in the system. > > You should probably add there is no underflow/overflow to other bytes > of the MAC address. 00:01:02:03:04:ff + 1 == 00:01:02:03:04:00. > > > +minimum: -255 > > +maximum: 255 > > + > > + mac-address-increment-byte: > > +description: > > + If 'mac-address-increment' is defined, this will tell what byte of > > + the mac-address will be increased. If 'mac-address-increment' is > > + not defined, this option will do nothing. > > +default: 5 > > +minimum: 0 > > +maximum: 5 > > Is there a real need for this? A value of 0 seems like a bad idea, > since a unicast address could easily become a multicast address, which > is not valid for an interface address. It also does not seem like a > good idea to allow the OUI to be changed. So i think only bytes 3-5 > should be allowed, but even then, i don't think this is needed, unless > you do have a clear use case. > > Andrew Honestly the mac-address-increment-byte is added to give user some control but I don't really have a use case for it. Should I limit it to 3 or just remove the function? Will address the other 2 comment in v3. Thx for review.
R: [PATCH 2/2] dt: bindings: ath10k: Document qcom, ath10k-pre-calibration-data-mtd
> -Messaggio originale- > Da: Christian Lamparter > Inviato: venerdì 18 settembre 2020 18:54 > A: Ansuel Smith ; Kalle Valo > > Cc: devicet...@vger.kernel.org; net...@vger.kernel.org; linux- > wirel...@vger.kernel.org; linux-kernel@vger.kernel.org; > ath...@lists.infradead.org; David S. Miller ; Rob > Herring ; Jakub Kicinski ; linux- > m...@lists.infradead.org; Srinivas Kandagatla > ; Bartosz Golaszewski > > Oggetto: Re: [PATCH 2/2] dt: bindings: ath10k: Document qcom, ath10k- > pre-calibration-data-mtd > > On 2020-09-18 18:29, Ansuel Smith wrote: > > Document use of qcom,ath10k-pre-calibration-data-mtd bindings used to > > define from where the driver will load the pre-cal data in the defined > > mtd partition. > > > > Signed-off-by: Ansuel Smith > > Q: Doesn't mtd now come with nvmem support from the get go? So > the MAC-Addresses and pre-caldata could be specified as a > nvmem-node in the devicetree? I remember seeing that this was > worked on or was this mtd->nvmem dropped? > > Cheers, > Christian Sorry a lot for the double email... I think I found what you are talking about. It looks like the code was merged but not the documentation. Will do some test and check if this works. This should be the related patch. https://patchwork.ozlabs.org/project/linux-mtd/patch/1521933899-362-4-git-send-email-al...@free.fr/
R: [PATCH 2/2] dt: bindings: ath10k: Document qcom, ath10k-pre-calibration-data-mtd
> -Messaggio originale- > Da: Christian Lamparter > Inviato: venerdì 18 settembre 2020 18:54 > A: Ansuel Smith ; Kalle Valo > > Cc: devicet...@vger.kernel.org; net...@vger.kernel.org; linux- > wirel...@vger.kernel.org; linux-kernel@vger.kernel.org; > ath...@lists.infradead.org; David S. Miller ; Rob > Herring ; Jakub Kicinski ; linux- > m...@lists.infradead.org; Srinivas Kandagatla > ; Bartosz Golaszewski > > Oggetto: Re: [PATCH 2/2] dt: bindings: ath10k: Document qcom, ath10k- > pre-calibration-data-mtd > > On 2020-09-18 18:29, Ansuel Smith wrote: > > Document use of qcom,ath10k-pre-calibration-data-mtd bindings used to > > define from where the driver will load the pre-cal data in the defined > > mtd partition. > > > > Signed-off-by: Ansuel Smith > > Q: Doesn't mtd now come with nvmem support from the get go? So > the MAC-Addresses and pre-caldata could be specified as a > nvmem-node in the devicetree? I remember seeing that this was > worked on or was this mtd->nvmem dropped? > > Cheers, > Christian Can you give me some example where this is used? I can't find any reference.
R: R: [RFC PATCH v3 0/2] Add Krait Cache Scaling support
> -Messaggio originale- > Da: sibis=codeaurora@mg.codeaurora.org > Per conto di Sibi Sankar > Inviato: giovedì 3 settembre 2020 09:13 > A: Viresh Kumar > Cc: ansuels...@gmail.com; vincent.guit...@linaro.org; > sarava...@google.com; 'Sudeep Holla' ; 'Rafael J. > Wysocki' ; 'Rob Herring' ; linux- > p...@vger.kernel.org; devicet...@vger.kernel.org; linux- > ker...@vger.kernel.org > Oggetto: Re: R: [RFC PATCH v3 0/2] Add Krait Cache Scaling support > > On 2020-09-03 12:23, Viresh Kumar wrote: > > On 31-08-20, 09:41, ansuels...@gmail.com wrote: > >> On 31-08-20, Sibi wrote: > >> > On 2020-08-24 16:10, Viresh Kumar wrote: > >> > > +Vincent/Saravana/Sibi > >> > > > >> > > On 21-08-20, 16:00, Ansuel Smith wrote: > >> > >> This adds Krait Cache scaling support using the cpufreq notifier. > >> > >> I have some doubt about where this should be actually placed (clk > or > >> > >> cpufreq)? > >> > >> Also the original idea was to create a dedicated cpufreq driver (like > >> > >> it's done in > >> > >> the codeaurora qcom repo) by copying the cpufreq-dt driver and > adding > >> > >> the cache > >> > >> scaling logic but i still don't know what is better. Have a very > >> > >> similar driver or > >> > >> add a dedicated driver only for the cache using the cpufreq notifier > >> > >> and do the > >> > >> scale on every freq transition. > >> > >> Thanks to everyone who will review or answer these questions. > >> > > > >> > > Saravana was doing something with devfreq to solve such issues if I > >> > > wasn't mistaken. > >> > > > >> > > Sibi ? > >> > > >> > IIRC the final plan was to create a devfreq device > >> > and devfreq-cpufreq based governor to scale them, this > >> > way one can switch to a different governor if required. > >> > >> So in this case I should convert this patch to a devfreq driver- > > > > I think this should happen nevertheless. You are doing DVFS for a > > device which isn't a CPU and devfreq looks to be the right place of > > doing so. > > > >> Isn't overkill to use a governor for such a task? > >> (3 range based on the cpufreq?) > > > > I am not sure about the governor part here, maybe it won't be required > > ? > > Yeah I don't see it being needed in ^^ > case as well. I just mentioned them as > an advantage in case you wanted to switch > to a different governor in the future. > > https://lore.kernel.org/lkml/d0bc8877-6d41-f54e-1c4c- > 2fadbb9dc...@samsung.com/ > > A devfreq governor tracking cpufreq was > generally accepted but using a cpufreq > notifier to achieve that was discouraged. > I read the patch discussion and it looks like at the very end they lost interest in pushing it. That would very fit what I need here so I'm asking how should I proceed? Keep the cpufreq notifier? Introduce a dedicated governor? Ask them to resume the pushing or try to include the changes to the passive governor by myself? > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project.
R: [RFC PATCH v3 0/2] Add Krait Cache Scaling support
> -Messaggio originale- > Da: sibis=codeaurora@mg.codeaurora.org > Per conto di Sibi Sankar > Inviato: lunedì 31 agosto 2020 07:46 > A: Viresh Kumar > Cc: Ansuel Smith ; vincent.guit...@linaro.org; > sarava...@google.com; Sudeep Holla ; Rafael J. > Wysocki ; Rob Herring ; linux- > p...@vger.kernel.org; devicet...@vger.kernel.org; linux- > ker...@vger.kernel.org > Oggetto: Re: [RFC PATCH v3 0/2] Add Krait Cache Scaling support > > On 2020-08-24 16:10, Viresh Kumar wrote: > > +Vincent/Saravana/Sibi > > > > On 21-08-20, 16:00, Ansuel Smith wrote: > >> This adds Krait Cache scaling support using the cpufreq notifier. > >> I have some doubt about where this should be actually placed (clk or > >> cpufreq)? > >> Also the original idea was to create a dedicated cpufreq driver (like > >> it's done in > >> the codeaurora qcom repo) by copying the cpufreq-dt driver and adding > >> the cache > >> scaling logic but i still don't know what is better. Have a very > >> similar driver or > >> add a dedicated driver only for the cache using the cpufreq notifier > >> and do the > >> scale on every freq transition. > >> Thanks to everyone who will review or answer these questions. > > > > Saravana was doing something with devfreq to solve such issues if I > > wasn't mistaken. > > > > Sibi ? > > IIRC the final plan was to create a devfreq device > and devfreq-cpufreq based governor to scale them, this > way one can switch to a different governor if required. So in this case I should convert this patch to a devfreq driver- Isn't overkill to use a governor for such a task? (3 range based on the cpufreq?) > (I don't see if ^^ applies well for l2 though). In the > interim until such a solution is acked on the list we > just scale the resources directly from the cpufreq In this case for this SoC we can't really scale the L2 freq with the cpu since we observed a bug and we need to switch back to the idle freq sometimes. Also this SoC use the generic cpufreq-dt driver and doesn't have a dedicated driver. So we must use a notifier. > driver. On SDM845/SC7180 SoCs, L3 is modeled as a > interconnect provider and is directly scaled from the > cpufreq-hw driver. > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project.
R: [RFC PATCH v5 1/7] drivers: thermal: tsens: Add VER_0 tsens version
> -Messaggio originale-
> Da: Amit Kucheria
> Inviato: martedì 11 agosto 2020 14:58
> A: Ansuel Smith
> Cc: Andy Gross ; Bjorn Andersson
> ; Zhang Rui ; Daniel
> Lezcano ; Rob Herring ;
> Linux PM list ; linux-arm-msm m...@vger.kernel.org>; open list:OPEN FIRMWARE AND FLATTENED DEVICE
> TREE BINDINGS ; LKML ker...@vger.kernel.org>
> Oggetto: Re: [RFC PATCH v5 1/7] drivers: thermal: tsens: Add VER_0 tsens
> version
>
> On Sat, Jul 25, 2020 at 11:44 PM Ansuel Smith
> wrote:
> >
> > VER_0 is used to describe device based on tsens version before v0.1.
> > These device are devices based on msm8960 for example apq8064 or
> > ipq806x.
> >
> > Signed-off-by: Ansuel Smith
> > ---
> > drivers/thermal/qcom/tsens.c | 160 +++--
> --
> > drivers/thermal/qcom/tsens.h | 7 +-
> > 2 files changed, 132 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> > index 9fe9a2b26705..78840c1bc5d2 100644
> > --- a/drivers/thermal/qcom/tsens.c
> > +++ b/drivers/thermal/qcom/tsens.c
> > @@ -516,6 +516,15 @@ static irqreturn_t tsens_irq_thread(int irq, void
> *data)
> > dev_dbg(priv->dev, "[%u] %s: no violation: %d\n",
> > hw_id, __func__, temp);
> > }
> > +
> > + if (tsens_version(priv) < VER_0_1) {
> > + /* Constraint: There is only 1 interrupt control
> > register for
> all
> > +* 11 temperature sensor. So monitoring more than 1
> sensor based
> > +* on interrupts will yield inconsistent result. To
> > overcome
> this
> > +* issue we will monitor only sensor 0 which is the
> > master
> sensor.
> > +*/
> > + break;
> > + }
> > }
> >
> > return IRQ_HANDLED;
> > @@ -531,6 +540,13 @@ static int tsens_set_trips(void *_sensor, int low,
> int high)
> > int high_val, low_val, cl_high, cl_low;
> > u32 hw_id = s->hw_id;
> >
> > + if (tsens_version(priv) < VER_0_1) {
> > + /* Pre v0.1 IP had a single register for each type of
> > interrupt
> > +* and thresholds
> > +*/
> > + hw_id = 0;
> > + }
> > +
> > dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",
> > hw_id, __func__, low, high);
> >
> > @@ -550,6 +566,12 @@ static int tsens_set_trips(void *_sensor, int low,
> int high)
> > tsens_set_interrupt(priv, hw_id, LOWER, true);
> > tsens_set_interrupt(priv, hw_id, UPPER, true);
> >
> > + /* VER_0 require to set MIN and MAX THRESH */
> > + if (tsens_version(priv) < VER_0_1) {
> > + regmap_field_write(priv->rf[MIN_THRESH_0], 0);
> > + regmap_field_write(priv->rf[MAX_THRESH_0], 12);
>
> Since MIN_THRESH_0 and MAX_THRESH_0 are the only two threshold on
> pre
> 0.1 IP, just (mis)use the already predefined LOW_THRESH_0 and
> UP_THRESH_0 in regfield_ids in init_common() below? Then we won't need
> this special casing here. All the special casing ugliness can then
> stay in init_common() with comments.
>
> > + }
> > +
> > spin_unlock_irqrestore(>ul_lock, flags);
> >
> > dev_dbg(dev, "[%u] %s: (%d:%d)->(%d:%d)\n",
> > @@ -584,18 +606,21 @@ int get_temp_tsens_valid(const struct
> tsens_sensor *s, int *temp)
> > u32 valid;
> > int ret;
> >
> > - ret = regmap_field_read(priv->rf[valid_idx], );
> > - if (ret)
> > - return ret;
> > - while (!valid) {
> > - /* Valid bit is 0 for 6 AHB clock cycles.
> > -* At 19.2MHz, 1 AHB clock is ~60ns.
> > -* We should enter this loop very, very rarely.
> > -*/
> > - ndelay(400);
> > + /* VER_0 doesn't have VALID bit */
> > + if (tsens_version(priv) >= VER_0_1) {
>
> Since 8960 needs a custom get_temp function, is this change really
> needed?
>
The get_temp_tsens_valid function is used to setup interrupt, think this is
the best way instead of add an if and call get_temp in the setup interrupt
function (instead of using get_temp_valid)
> > ret = regmap_field_read(priv->rf[valid_idx], );
> > if (ret)
> > return ret;
> > + while (!valid) {
> > + /* Valid bit is 0 for 6 AHB clock cycles.
> > +* At 19.2MHz, 1 AHB clock is ~60ns.
> > +* We should enter this loop very, very rarely.
> > +*/
> > + ndelay(400);
> > + ret = regmap_field_read(priv->rf[valid_idx],
> > );
> > + if (ret)
> > + return ret;
> > + }
> > }
> >
> > /* Valid bit is set, OK to
R: R: [RFC PATCH v2 2/2] dt-bindings: cpufreq: Document Krait CPU Cache scaling
> -Messaggio originale-
> Da: Sudeep Holla
> Inviato: lunedì 10 agosto 2020 14:45
> A: ansuels...@gmail.com
> Cc: 'Viresh Kumar' ; 'Rafael J. Wysocki'
> ; 'Rob Herring' ; linux-
> p...@vger.kernel.org; devicet...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Oggetto: Re: R: [RFC PATCH v2 2/2] dt-bindings: cpufreq: Document Krait
> CPU Cache scaling
>
> On Mon, Aug 10, 2020 at 01:15:24PM +0200, ansuels...@gmail.com
> wrote:
> >
> >
> > > -Messaggio originale-
> > > Da: Sudeep Holla
> > > Inviato: lunedì 10 agosto 2020 10:02
> > > A: Ansuel Smith
> > > Cc: Viresh Kumar ; Rafael J. Wysocki
> > > ; Rob Herring ; linux-
> > > p...@vger.kernel.org; devicet...@vger.kernel.org; linux-
> > > ker...@vger.kernel.org
> > > Oggetto: Re: [RFC PATCH v2 2/2] dt-bindings: cpufreq: Document Krait
> CPU
> > > Cache scaling
> > >
> > > On Sat, Aug 08, 2020 at 01:49:12AM +0200, Ansuel Smith wrote:
> > > > Document dedicated Krait CPU Cache Scaling driver.
> > > >
> > > > Signed-off-by: Ansuel Smith
> > > > ---
> > > > .../bindings/cpufreq/krait-cache-scale.yaml | 92
> > > +++
> > > > 1 file changed, 92 insertions(+)
> > > > create mode 100644
> Documentation/devicetree/bindings/cpufreq/krait-
> > > cache-scale.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/cpufreq/krait-cache-
> > > scale.yaml b/Documentation/devicetree/bindings/cpufreq/krait-cache-
> > > scale.yaml
> > > > new file mode 100644
> > > > index ..f10b1f386a99
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/cpufreq/krait-cache-
> > > scale.yaml
> > > > @@ -0,0 +1,92 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/cpufreq/krait-cache-scale.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Krait Cpu Cache Frequency Scaling dedicated driver
> > > > +
> > > > +maintainers:
> > > > + - Ansuel Smith
> > > > +
> > > > +description: |
> > > > + This Scale the Krait CPU Cache Frequency and optionally voltage
> > > > + when the Cpu Frequency is changed (using the cpufreq notifier).
> > > > +
> > > > + Cache is scaled with the max frequency across all core and the
cache
> > > > + frequency will scale based on the configured threshold in the
dts.
> > > > +
> > > > + The cache is hardcoded to 3 frequency bin, idle, nominal and
high.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > +const: qcom,krait-cache
> > > > +
> > >
> > > How does this fit in the standard cache hierarchy nodes ? Extend the
> > > example to cover that.
> > >
> >
> > I think i didn't understand this question. You mean that I should put
> > in the example how the standard l2 cache nodes are defined?
> >
>
> I was referring to something like below which I found now in
> arch/arm/boot/dts/qcom-msm8974.dtsi:
> L2: l2-cache {
> compatible = "cache";
> cache-level = <2>;
> qcom,saw = <_l2>;
> };
>
> > > > + clocks:
> > > > +description: Phandle to the L2 CPU clock
> > > > +
> > > > + clock-names:
> > > > +const: "l2"
> > > > +
> > > > + voltage-tolerance:
> > > > +description: Same voltage tollerance of the Krait CPU
> > > > +
> > > > + l2-rates:
> > > > +description: |
> > > > + Frequency the L2 cache will be scaled at.
> > > > + Value is in Hz.
> > > > +$ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +items:
> > > > + - description: idle
> > > > + - description: nominal
> > > > + - description: high
> > > > +
> > >
> > > Why can't you re-use the standard OPP v2 bindings ?
> > >
> >
> > Isn't overkill to use the OPP v2 bindings to represent the the microvolt
> > related to the le freq? Is the OPP v1 sufficient?
>
> Should be fine if it is allowed. v2 came out in the flow of my thought
> and was not intentional.
>
> > Also I can't find a way to reflect this specific case where the l2 rates
> > are changed based on the cpu freq value? Any idea about that?
> >
>
> OK, I am always opposed to giving such independent controls in the kernel
> as one can play around say max cpu freq and lowest cache or vice-versa
> and create instabilities. IMO this should be completely hidden from OS.
> But I know these are old platforms, so I will shut my mouth ;)
>
If we really want to deny this practice, I can add a check in the probe
function to fail if the l2 freq threshold is less than the cpu freq.
> --
> Regards,
> Sudeep
R: [RFC PATCH v2 2/2] dt-bindings: cpufreq: Document Krait CPU Cache scaling
> -Messaggio originale- > Da: Sudeep Holla > Inviato: lunedì 10 agosto 2020 10:02 > A: Ansuel Smith > Cc: Viresh Kumar ; Rafael J. Wysocki > ; Rob Herring ; linux- > p...@vger.kernel.org; devicet...@vger.kernel.org; linux- > ker...@vger.kernel.org > Oggetto: Re: [RFC PATCH v2 2/2] dt-bindings: cpufreq: Document Krait CPU > Cache scaling > > On Sat, Aug 08, 2020 at 01:49:12AM +0200, Ansuel Smith wrote: > > Document dedicated Krait CPU Cache Scaling driver. > > > > Signed-off-by: Ansuel Smith > > --- > > .../bindings/cpufreq/krait-cache-scale.yaml | 92 > +++ > > 1 file changed, 92 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/cpufreq/krait- > cache-scale.yaml > > > > diff --git a/Documentation/devicetree/bindings/cpufreq/krait-cache- > scale.yaml b/Documentation/devicetree/bindings/cpufreq/krait-cache- > scale.yaml > > new file mode 100644 > > index ..f10b1f386a99 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/cpufreq/krait-cache- > scale.yaml > > @@ -0,0 +1,92 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/cpufreq/krait-cache-scale.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Krait Cpu Cache Frequency Scaling dedicated driver > > + > > +maintainers: > > + - Ansuel Smith > > + > > +description: | > > + This Scale the Krait CPU Cache Frequency and optionally voltage > > + when the Cpu Frequency is changed (using the cpufreq notifier). > > + > > + Cache is scaled with the max frequency across all core and the cache > > + frequency will scale based on the configured threshold in the dts. > > + > > + The cache is hardcoded to 3 frequency bin, idle, nominal and high. > > + > > +properties: > > + compatible: > > +const: qcom,krait-cache > > + > > How does this fit in the standard cache hierarchy nodes ? Extend the > example to cover that. > I think i didn't understand this question. You mean that I should put in the example how the standard l2 cache nodes are defined? > > + clocks: > > +description: Phandle to the L2 CPU clock > > + > > + clock-names: > > +const: "l2" > > + > > + voltage-tolerance: > > +description: Same voltage tollerance of the Krait CPU > > + > > + l2-rates: > > +description: | > > + Frequency the L2 cache will be scaled at. > > + Value is in Hz. > > +$ref: /schemas/types.yaml#/definitions/uint32-array > > +items: > > + - description: idle > > + - description: nominal > > + - description: high > > + > > Why can't you re-use the standard OPP v2 bindings ? > Isn't overkill to use the OPP v2 bindings to represent the the microvolt related to the le freq? Is the OPP v1 sufficient? Also I can't find a way to reflect this specific case where the l2 rates are changed based on the cpu freq value? Any idea about that? > -- > Regards, > Sudeep
R: [PATCH v4 5/7] drivers: thermal: tsens: add interrupt support for 9860 driver
> -Messaggio originale- > Da: Amit Kucheria > Inviato: lunedì 20 luglio 2020 11:41 > A: Ansuel Smith > Cc: Rob Herring ; Andy Gross ; > Bjorn Andersson ; Zhang Rui > ; Daniel Lezcano ; > Michael Turquette ; Stephen Boyd > ; Linux PM list ; linux-arm- > msm ; DTML > ; Linux Kernel Mailing List ker...@vger.kernel.org>; linux-clk > Oggetto: Re: [PATCH v4 5/7] drivers: thermal: tsens: add interrupt support > for 9860 driver > > Hi Ansuel, > > Thanks for this patch. > > On Thu, Jul 16, 2020 at 7:58 AM Ansuel Smith > wrote: > > > > Add interrupt support for 9860 tsens driver used to set thermal trip > > point for the system. > > typo: 8960 > > You've used the names 8960 and ipq8064 interchangeably throughout the > series. AFAICT, msm8960, ipq8064 and apq8064 use the same IP version > of tsens. Please use 8960 in all patches, descriptions and dt-binding. > to reflect the filename for the driver. > Then add ipq8064 and apq8064 in a comment in the driver like here to > show that the driver also supports these other SoCs: > https://elixir.bootlin.com/linux/v5.8- > rc4/source/drivers/thermal/qcom/tsens-v0_1.c#L328 > > You can also add a new compatible string for ipq8064 as a separate > patch at the end of the series. > > > Signed-off-by: Ansuel Smith > > --- > > drivers/thermal/qcom/tsens-8960.c | 197 > +++--- > > drivers/thermal/qcom/tsens.h | 3 + > > 2 files changed, 186 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/thermal/qcom/tsens-8960.c > b/drivers/thermal/qcom/tsens-8960.c > > index 45788eb3c666..20d0bfb10f1f 100644 > > --- a/drivers/thermal/qcom/tsens-8960.c > > +++ b/drivers/thermal/qcom/tsens-8960.c > > @@ -8,6 +8,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include "tsens.h" > > > > @@ -27,7 +28,6 @@ > > /* CNTL_ADDR bitmasks */ > > #define EN BIT(0) > > #define SW_RST BIT(1) > > -#define SENSOR0_EN BIT(3) > > #define SLP_CLK_ENABIT(26) > > #define SLP_CLK_ENA_8660 BIT(24) > > #define MEASURE_PERIOD 1 > > @@ -41,14 +41,26 @@ > > > > #define THRESHOLD_ADDR 0x3624 > > /* THRESHOLD_ADDR bitmasks */ > > +#define THRESHOLD_MAX_CODE 0x2 > > +#define THRESHOLD_MIN_CODE 0 > > #define THRESHOLD_MAX_LIMIT_SHIFT 24 > > #define THRESHOLD_MIN_LIMIT_SHIFT 16 > > #define THRESHOLD_UPPER_LIMIT_SHIFT8 > > #define THRESHOLD_LOWER_LIMIT_SHIFT0 > > +#define THRESHOLD_MAX_LIMIT_MASK (THRESHOLD_MAX_CODE > << \ > > + THRESHOLD_MAX_LIMIT_SHIFT) > > +#define THRESHOLD_MIN_LIMIT_MASK (THRESHOLD_MAX_CODE << > \ > > + THRESHOLD_MIN_LIMIT_SHIFT) > > +#define THRESHOLD_UPPER_LIMIT_MASK (THRESHOLD_MAX_CODE > << \ > > + THRESHOLD_UPPER_LIMIT_SHIFT) > > +#define THRESHOLD_LOWER_LIMIT_MASK (THRESHOLD_MAX_CODE > << \ > > + THRESHOLD_LOWER_LIMIT_SHIFT) > > > > /* Initial temperature threshold values */ > > -#define LOWER_LIMIT_TH 0x50 > > -#define UPPER_LIMIT_TH 0xdf > > +#define LOWER_LIMIT_TH_89600x50 > > +#define UPPER_LIMIT_TH_89600xdf > > +#define LOWER_LIMIT_TH_80640x9d /* 95C */ > > +#define UPPER_LIMIT_TH_80640xa6 /* 105C */ > > #define MIN_LIMIT_TH 0x0 > > #define MAX_LIMIT_TH 0xff > > > > @@ -57,6 +69,170 @@ > > #define TRDY_MASK BIT(7) > > #define TIMEOUT_US 100 > > > > +#define TSENS_EN BIT(0) > > +#define TSENS_SW_RST BIT(1) > > +#define TSENS_ADC_CLK_SEL BIT(2) > > +#define SENSOR0_EN BIT(3) > > +#define SENSOR1_EN BIT(4) > > +#define SENSOR2_EN BIT(5) > > +#define SENSOR3_EN BIT(6) > > +#define SENSOR4_EN BIT(7) > > +#define SENSORS_EN (SENSOR0_EN | SENSOR1_EN | \ > > + SENSOR2_EN | SENSOR3_EN | SENSOR4_EN) > > +#define TSENS_8064_SENSOR5_EN BIT(8) > > +#define TSENS_8064_SENSOR6_EN BIT(9) > > +#define TSENS_8064_SENSOR7_EN BIT(10) > > +#define TSENS_8064_SENSOR8_EN BIT(11) > > +#define TSENS_8064_SENSOR9_EN BIT(12) > > +#define TSENS_8064_SENSOR10_EN BIT(13) > > +#define TSENS_8064_SENSORS_EN (SENSORS_EN | \ > > + TSENS_8064_SENSOR5_EN | \ > > + TSENS_8064_SENSOR6_EN | \ > > + TSENS_8064_SENSOR7_EN | \ > > + TSENS_8064_SENSOR8_EN | \ > > + TSENS_8064_SENSOR9_EN | \ > > +
R: [PATCH v10 1/2] phy: qualcomm: add qcom ipq806x dwc usb phy driver
> -Messaggio originale- > Da: Vinod Koul > Inviato: lunedì 20 luglio 2020 08:26 > A: Ansuel Smith > Cc: Andy Gross ; Jonathan McDowell > ; Andy Gross ; Bjorn Andersson > ; Kishon Vijay Abraham I ; > Rob Herring ; Mark Rutland > ; linux-arm-...@vger.kernel.org; linux- > ker...@vger.kernel.org; devicet...@vger.kernel.org > Oggetto: Re: [PATCH v10 1/2] phy: qualcomm: add qcom ipq806x dwc usb > phy driver > > On 17-07-20, 15:16, Ansuel Smith wrote: > > This has lost in the original push for the dwc3 qcom driver. > > This is needed for ipq806x SoC as without this the usb ports > > doesn't work at all. > > Applied both, thanks > > My script found below errors with W=1, can you please send fixes for > these > Since you applied them should I send a new patch or a v11 of this patchset? > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:140: warning: Function > parameter or member 'phy_dwc3' not described in > 'usb_phy_write_readback' > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:140: warning: Function > parameter or member 'offset' not described in 'usb_phy_write_readback' > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:140: warning: Function > parameter or member 'mask' not described in 'usb_phy_write_readback' > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:140: warning: Function > parameter or member 'val' not described in 'usb_phy_write_readback' > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:182: warning: Function > parameter or member 'phy_dwc3' not described in 'usb_ss_write_phycreg' > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:182: warning: Function > parameter or member 'addr' not described in 'usb_ss_write_phycreg' > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:182: warning: Function > parameter or member 'val' not described in 'usb_ss_write_phycreg' > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:219: warning: Function > parameter or member 'phy_dwc3' not described in 'usb_ss_read_phycreg' > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:219: warning: Function > parameter or member 'addr' not described in 'usb_ss_read_phycreg' > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:219: warning: Function > parameter or member 'val' not described in 'usb_ss_read_phycreg' > > -- > ~Vinod
R: [RESEND PATCH v7 1/2] phy: qualcomm: add qcom ipq806x dwc usb phy driver
> -Messaggio originale-
> Da: Vinod Koul
> Inviato: lunedì 13 luglio 2020 07:33
> A: Ansuel Smith
> Cc: Andy Gross ; Jonathan McDowell
> ; Andy Gross ; Bjorn Andersson
> ; Kishon Vijay Abraham I ;
> Rob Herring ; Mark Rutland
> ; linux-arm-...@vger.kernel.org; linux-
> ker...@vger.kernel.org; devicet...@vger.kernel.org
> Oggetto: Re: [RESEND PATCH v7 1/2] phy: qualcomm: add qcom ipq806x
> dwc usb phy driver
>
> On 15-06-20, 22:53, Ansuel Smith wrote:
>
> > @@ -0,0 +1,593 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright (c) 2014-2015, Code Aurora Forum. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
>
> You have SPDX tag, so we dont need the license text, please remove this.
> Also we are in 2020 now so Copyright looks incorrect
>
> > +static int qcom_ipq806x_usb_ss_phy_init(struct phy *phy)
> > +{
> > + struct usb_phy *phy_dwc3 = phy_get_drvdata(phy);
> > + int ret;
> > + u32 data = 0;
>
> Superfluous init
>
> > +static int qcom_ipq806x_usb_phy_probe(struct platform_device *pdev)
> > +{
> > + struct usb_phy *phy_dwc3;
> > + struct phy_provider *phy_provider;
> > + struct phy *generic_phy;
> > + const struct of_device_id *match;
> > + const struct phy_drvdata *data;
> > + struct resource *res;
> > + resource_size_t size;
>
> Pls pick one, tabs or single spaces, not both. and reverse christmas
> looks better :)
>
> > + struct device_node *np;
> > +
> > + phy_dwc3 = devm_kzalloc(>dev, sizeof(*phy_dwc3),
> GFP_KERNEL);
> > + if (!phy_dwc3)
> > + return -ENOMEM;
> > +
> > + match = of_match_node(qcom_ipq806x_usb_phy_table, pdev-
> >dev.of_node);
> > + data = match->data;
>
> How about using of_device_get_match_data() instead?
> --
> ~Vinod
match is also used in the function to compare compatible.
R: [PATCH 3/6] dt-bindings: thermal: tsens: document ipq8064 bindings
> -Messaggio originale- > Da: Rob Herring > Inviato: venerdì 10 luglio 2020 18:27 > A: Ansuel Smith > Cc: Amit Kucheria ; Andy Gross > ; Bjorn Andersson ; > Zhang Rui ; Daniel Lezcano > ; linux...@vger.kernel.org; linux-arm- > m...@vger.kernel.org; devicet...@vger.kernel.org; linux- > ker...@vger.kernel.org > Oggetto: Re: [PATCH 3/6] dt-bindings: thermal: tsens: document ipq8064 > bindings > > On Thu, Jul 09, 2020 at 11:51:33PM +0200, Ansuel Smith wrote: > > Document the use of regmap phandle for ipq8064 SoCs > > > > Signed-off-by: Ansuel Smith > > --- > > .../bindings/thermal/qcom-tsens.yaml | 51 --- > > 1 file changed, 44 insertions(+), 7 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml > b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml > > index d7be931b42d2..5ceb5d720e16 100644 > > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml > > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml > > @@ -24,6 +24,7 @@ properties: > >- enum: > >- qcom,msm8916-tsens > >- qcom,msm8974-tsens > > + - qcom,ipq8064-tsens > >- const: qcom,tsens-v0_1 > > > >- description: v1 of TSENS > > @@ -47,6 +48,11 @@ properties: > >- description: TM registers > >- description: SROT registers > > > > + regmap: > > +description: > > + Phandle to the gcc. On ipq8064 SoCs gcc and tsense share the same > regs. > > +$ref: /schemas/types.yaml#/definitions/phandle > > Can't you make this a child of the gcc and drop this property? > Make the thermal a child of the gcc would be a little confusing. Anyway making this a child of gcc cause the not probing of the thermal driver as it's ignored any child of gcc. I pushed v2 with the fixed problem. > > + > >interrupts: > > minItems: 1 > > items: > > @@ -111,17 +117,48 @@ allOf: > > interrupt-names: > >minItems: 2 > > > > -required: > > - - compatible > > - - reg > > - - "#qcom,sensors" > > - - interrupts > > - - interrupt-names > > - - "#thermal-sensor-cells" > > + - if: > > + properties: > > +compatible: > > + contains: > > +enum: > > + - qcom,ipq8064-tsens > > +then: > > + required: > > +- compatible > > +- regmap > > +- "#qcom,sensors" > > +- interrupts > > +- interrupt-names > > +- "#thermal-sensor-cells" > > + > > +else: > > + required: > > +- compatible > > +- reg > > +- "#qcom,sensors" > > +- interrupts > > +- interrupt-names > > +- "#thermal-sensor-cells" > > Keep all the common required properties and just put reg/regmap in the > if/then if this ends up staying. > > Rob
R: R: [PATCH v5 11/11] PCI: qcom: Add Force GEN1 support
> -Messaggio originale-
> Da: Bjorn Helgaas
> Inviato: martedì 2 giugno 2020 19:28
> A: ansuels...@gmail.com
> Cc: 'Rob Herring' ; 'Sham Muthayyan'
> ; 'Rob Herring' ; 'Andy
> Gross' ; 'Bjorn Andersson'
> ; 'Bjorn Helgaas' ;
> 'Mark Rutland' ; 'Stanimir Varbanov'
> ; 'Lorenzo Pieralisi'
> ; 'Andrew Murray'
> ; 'Philipp Zabel'
> ; linux-arm-...@vger.kernel.org; linux-
> p...@vger.kernel.org; devicet...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Varadarajan Narayanan
> Oggetto: Re: R: [PATCH v5 11/11] PCI: qcom: Add Force GEN1 support
>
> [+cc Varada]
>
> On Tue, Jun 02, 2020 at 07:07:27PM +0200, ansuels...@gmail.com
> wrote:
> > > On Tue, Jun 02, 2020 at 01:53:52PM +0200, Ansuel Smith wrote:
> > > > From: Sham Muthayyan
> > > >
> > > > Add Force GEN1 support needed in some ipq8064 board that needs to
> > > limit
> > > > some PCIe line to gen1 for some hardware limitation. This is set by
the
> > > > max-link-speed binding and needed by some soc based on ipq8064.
> (for
> > > > example Netgear R7800 router)
> > > >
> > > > Signed-off-by: Sham Muthayyan
> > > > Signed-off-by: Ansuel Smith
> > > > Reviewed-by: Rob Herring
> > > > ---
> > > > drivers/pci/controller/dwc/pcie-qcom.c | 13 +
> > > > 1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> > > b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > index 259b627bf890..0ce15d53c46e 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > @@ -27,6 +27,7 @@
> > > > #include
> > > > #include
> > > >
> > > > +#include "../../pci.h"
> > > > #include "pcie-designware.h"
> > > >
> > > > #define PCIE20_PARF_SYS_CTRL 0x00
> > > > @@ -99,6 +100,8 @@
> > > > #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE 0x358
> > > > #define SLV_ADDR_SPACE_SZ 0x1000
> > > >
> > > > +#define PCIE20_LNK_CONTROL2_LINK_STATUS2 0xa0
> > > > +
> > > > #define DEVICE_TYPE_RC 0x4
> > > >
> > > > #define QCOM_PCIE_2_1_0_MAX_SUPPLY 3
> > > > @@ -195,6 +198,7 @@ struct qcom_pcie {
> > > > struct phy *phy;
> > > > struct gpio_desc *reset;
> > > > const struct qcom_pcie_ops *ops;
> > > > + int gen;
> > > > };
> > > >
> > > > #define to_qcom_pcie(x)dev_get_drvdata((x)->dev)
> > > > @@ -395,6 +399,11 @@ static int qcom_pcie_init_2_1_0(struct
> > > qcom_pcie *pcie)
> > > > /* wait for clock acquisition */
> > > > usleep_range(1000, 1500);
> > > >
> > > > + if (pcie->gen == 1) {
> > > > + val = readl(pci->dbi_base +
> > > PCIE20_LNK_CONTROL2_LINK_STATUS2);
> > > > + val |= 1;
> > >
> > > Is this the same bit that's visible in config space as
> > > PCI_EXP_LNKSTA_CLS_2_5GB? Why not use that #define?
> > >
> > > There are a bunch of other #defines in this file that look like
> > > redefinitions of standard things:
> > >
> > > #define PCIE20_COMMAND_STATUS 0x04
> > > Looks like PCI_COMMAND
> > >
> > > #define CMD_BME_VAL 0x4
> > > Looks like PCI_COMMAND_MASTER
> > >
> > > #define PCIE20_DEVICE_CONTROL2_STATUS2 0x98
> > > Looks like (PCIE20_CAP + PCI_EXP_DEVCTL2)
> > >
> > > #define PCIE_CAP_CPL_TIMEOUT_DISABLE0x10
> > > Looks like PCI_EXP_DEVCTL2_COMP_TMOUT_DIS
> > >
> > > #define PCIE20_CAP 0x70
> > > This one is obviously device-specific
> > >
> > > #define PCIE20_CAP_LINK_CAPABILITIES(PCIE20_CAP + 0xC)
> > > Looks like (PCIE20_CAP + PCI_EXP_LNKCAP)
> > >
> > > #define PCIE20_CAP_ACTIVE_STATE_LINK_PM_SUPPORT (BIT(10) |
> > > BIT(11))
> > > Looks like PCI_EXP_LNKCAP_ASPMS
> > >
> > > #define PCIE20_CAP_LINK_1 (PCIE20_CAP + 0x14)
> > > #define PCIE_CAP_LINK1_VAL 0x2FD7F
> > > This looks like PCIE20_CAP_LINK_1 should be (PCIE20_CAP +
> > > PCI_EXP_SLTCAP), but "CAP_LINK_1" doesn't sound like the Slot
> > > Capabilities register, and I don't know what PCIE_CAP_LINK1_VAL
> > > means.
> >
> > The define are used by ipq8074 and I really can't test the changes.
> > Anyway it shouldn't make a difference use the define instead of the
> > custom value so a patch should not harm anything... Problem is the
> > last 2 define that we really don't know what they are about... How
> > should I proceed? Change only the value related to
> > PCI_EXP_LNKSTA_CLS_2_5GB or change all the other except the last 2?
>
> I personally would change all the ones I mentioned above (in a
> separate patch from the one that adds "max-link-speed" support).
> Testing isn't a big deal because changing the #defines shouldn't
> change the generated code at all.
>
> PCIE20_CAP_LINK_1 / PCIE_CAP_LINK1_VAL looks like a potential bug or
> at least a very misleading name. I wouldn't touch it unless we can
R: [PATCH v6 1/2] phy: qualcomm: add qcom ipq806x dwc usb phy driver
> On Wed, Jun 03, 2020 at 03:22:34PM +0200, Ansuel Smith wrote: > > This has lost in the original push for the dwc3 qcom driver. > > This is needed for ipq806x SoC as without this the usb ports > > doesn't work at all. > > FWIW I tested this on my RB3011 so feel free to add: > > Tested-by: Jonathan McDowell > > One minor comment; would PHY_QCOM_USB_IPQ806X not be a better > choice > than PHY_QCOM_IPQ806X_USB given the existing naming? > Thanks for the feedback. About naming I'm following the sata ipq806x naming. I really hope someone gets this and reviews it since usb is broken for a long time now. > > Signed-off-by: Andy Gross > > Signed-off-by: Ansuel Smith > > --- > > v6: > > * Use GENMASK instead of hex value > > v4: > > * Add qcom to specific bindings > > v3: > > * Use reg instead of regmap phandle > > v2: > > * Renamed config from PHY_QCOM_DWC3 to PHY_QCOM_IPQ806X_USB > > * Rename inline function to generic name to reduce length > > * Fix check reported by checkpatch --strict > > * Rename compatible to qcom,ipq806x-usb-phy-(hs/ss) > > > > drivers/phy/qualcomm/Kconfig| 12 + > > drivers/phy/qualcomm/Makefile | 1 + > > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c | 593 > > > 3 files changed, 606 insertions(+) > > create mode 100644 drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c > > > > diff --git a/drivers/phy/qualcomm/Kconfig > b/drivers/phy/qualcomm/Kconfig > > index e46824da29f6..9d41c3d12800 100644 > > --- a/drivers/phy/qualcomm/Kconfig > > +++ b/drivers/phy/qualcomm/Kconfig > > @@ -91,3 +91,15 @@ config PHY_QCOM_USB_HSIC > > select GENERIC_PHY > > help > > Support for the USB HSIC ULPI compliant PHY on QCOM chipsets. > > + > > +config PHY_QCOM_IPQ806X_USB > > + tristate "Qualcomm IPQ806x DWC3 USB PHY driver" > > + depends on ARCH_QCOM > > + depends on HAS_IOMEM > > + depends on OF > > + select GENERIC_PHY > > + help > > + This option enables support for the Synopsis PHYs present inside > the > > + Qualcomm USB3.0 DWC3 controller on ipq806x SoC. This driver > supports > > + both HS and SS PHY controllers. > > + > > diff --git a/drivers/phy/qualcomm/Makefile > b/drivers/phy/qualcomm/Makefile > > index 283251d6a5d9..8629299c1495 100644 > > --- a/drivers/phy/qualcomm/Makefile > > +++ b/drivers/phy/qualcomm/Makefile > > @@ -10,3 +10,4 @@ obj-$(CONFIG_PHY_QCOM_UFS_14NM) > += phy-qcom-ufs-qmp-14nm.o > > obj-$(CONFIG_PHY_QCOM_UFS_20NM)+= phy-qcom-ufs- > qmp-20nm.o > > obj-$(CONFIG_PHY_QCOM_USB_HS) += phy-qcom-usb-hs.o > > obj-$(CONFIG_PHY_QCOM_USB_HSIC)+= phy-qcom-usb-hsic.o > > +obj-$(CONFIG_PHY_QCOM_IPQ806X_USB) += phy-qcom- > ipq806x-usb.o > > diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c > b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c > > new file mode 100644 > > index ..f37cd8760118 > > --- /dev/null > > +++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c > > @@ -0,0 +1,593 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* Copyright (c) 2014-2015, Code Aurora Forum. All rights reserved. > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 and > > + * only version 2 as published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +/* USB QSCRATCH Hardware registers */ > > +#define QSCRATCH_GENERAL_CFG (0x08) > > +#define HSUSB_PHY_CTRL_REG (0x10) > > + > > +/* PHY_CTRL_REG */ > > +#define HSUSB_CTRL_DMSEHV_CLAMPBIT(24) > > +#define HSUSB_CTRL_USB2_SUSPENDBIT(23) > > +#define HSUSB_CTRL_UTMI_CLK_EN BIT(21) > > +#define HSUSB_CTRL_UTMI_OTG_VBUS_VALID BIT(20) > > +#define HSUSB_CTRL_USE_CLKCORE BIT(18) > > +#define HSUSB_CTRL_DPSEHV_CLAMPBIT(17) > > +#define HSUSB_CTRL_COMMONONN BIT(11) > > +#define HSUSB_CTRL_ID_HV_CLAMP BIT(9) > > +#define HSUSB_CTRL_OTGSESSVLD_CLAMPBIT(8) > > +#define HSUSB_CTRL_CLAMP_ENBIT(7) > > +#define HSUSB_CTRL_RETENABLEN BIT(1) > > +#define HSUSB_CTRL_POR BIT(0) > > + > > +/* QSCRATCH_GENERAL_CFG */ > > +#define HSUSB_GCFG_XHCI_REVBIT(2) > > + > > +/* USB QSCRATCH Hardware registers */ > > +#define SSUSB_PHY_CTRL_REG (0x00) > > +#define SSUSB_PHY_PARAM_CTRL_1 (0x04) > > +#define SSUSB_PHY_PARAM_CTRL_2 (0x08) > > +#define CR_PROTOCOL_DATA_IN_REG(0x0c) > >
R: [PATCH v5 11/11] PCI: qcom: Add Force GEN1 support
> On Tue, Jun 02, 2020 at 01:53:52PM +0200, Ansuel Smith wrote:
> > From: Sham Muthayyan
> >
> > Add Force GEN1 support needed in some ipq8064 board that needs to
> limit
> > some PCIe line to gen1 for some hardware limitation. This is set by the
> > max-link-speed binding and needed by some soc based on ipq8064. (for
> > example Netgear R7800 router)
> >
> > Signed-off-by: Sham Muthayyan
> > Signed-off-by: Ansuel Smith
> > Reviewed-by: Rob Herring
> > ---
> > drivers/pci/controller/dwc/pcie-qcom.c | 13 +
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 259b627bf890..0ce15d53c46e 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -27,6 +27,7 @@
> > #include
> > #include
> >
> > +#include "../../pci.h"
> > #include "pcie-designware.h"
> >
> > #define PCIE20_PARF_SYS_CTRL 0x00
> > @@ -99,6 +100,8 @@
> > #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE 0x358
> > #define SLV_ADDR_SPACE_SZ 0x1000
> >
> > +#define PCIE20_LNK_CONTROL2_LINK_STATUS2 0xa0
> > +
> > #define DEVICE_TYPE_RC 0x4
> >
> > #define QCOM_PCIE_2_1_0_MAX_SUPPLY 3
> > @@ -195,6 +198,7 @@ struct qcom_pcie {
> > struct phy *phy;
> > struct gpio_desc *reset;
> > const struct qcom_pcie_ops *ops;
> > + int gen;
> > };
> >
> > #define to_qcom_pcie(x)dev_get_drvdata((x)->dev)
> > @@ -395,6 +399,11 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> > /* wait for clock acquisition */
> > usleep_range(1000, 1500);
> >
> > + if (pcie->gen == 1) {
> > + val = readl(pci->dbi_base +
> PCIE20_LNK_CONTROL2_LINK_STATUS2);
> > + val |= 1;
>
> Is this the same bit that's visible in config space as
> PCI_EXP_LNKSTA_CLS_2_5GB? Why not use that #define?
>
> There are a bunch of other #defines in this file that look like
> redefinitions of standard things:
>
> #define PCIE20_COMMAND_STATUS 0x04
> Looks like PCI_COMMAND
>
> #define CMD_BME_VAL 0x4
> Looks like PCI_COMMAND_MASTER
>
> #define PCIE20_DEVICE_CONTROL2_STATUS2 0x98
> Looks like (PCIE20_CAP + PCI_EXP_DEVCTL2)
>
> #define PCIE_CAP_CPL_TIMEOUT_DISABLE0x10
> Looks like PCI_EXP_DEVCTL2_COMP_TMOUT_DIS
>
> #define PCIE20_CAP 0x70
> This one is obviously device-specific
>
> #define PCIE20_CAP_LINK_CAPABILITIES(PCIE20_CAP + 0xC)
> Looks like (PCIE20_CAP + PCI_EXP_LNKCAP)
>
> #define PCIE20_CAP_ACTIVE_STATE_LINK_PM_SUPPORT (BIT(10) |
> BIT(11))
> Looks like PCI_EXP_LNKCAP_ASPMS
>
> #define PCIE20_CAP_LINK_1 (PCIE20_CAP + 0x14)
> #define PCIE_CAP_LINK1_VAL 0x2FD7F
> This looks like PCIE20_CAP_LINK_1 should be (PCIE20_CAP +
> PCI_EXP_SLTCAP), but "CAP_LINK_1" doesn't sound like the Slot
> Capabilities register, and I don't know what PCIE_CAP_LINK1_VAL
> means.
>
The define are used by ipq8074 and I really can't test the changes. Anyway
it shouldn't make a difference use the define instead of the custom value so
a patch should not harm anything... Problem is the last 2 define that we
really
don't know what they are about... How should I proceed? Change only the
value related to PCI_EXP_LNKSTA_CLS_2_5GB or change all the other except the
last 2?
> > + writel(val, pci->dbi_base +
> PCIE20_LNK_CONTROL2_LINK_STATUS2);
> > + }
> >
> > /* Set the Max TLP size to 2K, instead of using default of 4K */
> > writel(CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K,
> > @@ -1397,6 +1406,10 @@ static int qcom_pcie_probe(struct
> platform_device *pdev)
> > goto err_pm_runtime_put;
> > }
> >
> > + pcie->gen = of_pci_get_max_link_speed(pdev->dev.of_node);
> > + if (pcie->gen < 0)
> > + pcie->gen = 2;
> > +
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "parf");
> > pcie->parf = devm_ioremap_resource(dev, res);
> > if (IS_ERR(pcie->parf)) {
> > --
> > 2.25.1
> >
R: [PATCH v4 08/10] PCI: qcom: Add ipq8064 rev2 variant and set tx term offset
> -Messaggio originale-
> Da: Rob Herring
> Inviato: lunedì 1 giugno 2020 23:09
> A: Ansuel Smith
> Cc: Bjorn Andersson ; Sham Muthayyan
> ; Andy Gross ; Bjorn
> Helgaas ; Mark Rutland
> ; Stanimir Varbanov ;
> Lorenzo Pieralisi ; Andrew Murray
> ; Philipp Zabel
> ; linux-arm-...@vger.kernel.org; linux-
> p...@vger.kernel.org; devicet...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Oggetto: Re: [PATCH v4 08/10] PCI: qcom: Add ipq8064 rev2 variant and
> set tx term offset
>
> On Thu, May 14, 2020 at 10:07:09PM +0200, Ansuel Smith wrote:
> > Add tx term offset support to pcie qcom driver need in some revision of
> > the ipq806x SoC. Ipq8064 have tx term offset set to 7. Ipq8064-v2
> revision
> > and ipq8065 have the tx term offset set to 0.
>
> Seems like this should be 2 patches or why isn't 'Ipq8064 have tx term
> offset set to 7' done in the prior patch? One tweak is needed for
> stable, but this isn't?
>
Ok i will split this in 2 patch and set for stable the tx term patch.
> >
> > Signed-off-by: Sham Muthayyan
> > Signed-off-by: Ansuel Smith
> > ---
> > drivers/pci/controller/dwc/pcie-qcom.c | 18 --
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index f5398b0d270c..ab6f1bdd24c3 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -45,6 +45,9 @@
> > #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
> >
> > #define PCIE20_PARF_PHY_CTRL 0x40
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(20,
> 16)
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)((x) << 16)
> > +
> > #define PCIE20_PARF_PHY_REFCLK 0x4C
> > #define PHY_REFCLK_SSP_EN BIT(16)
> > #define PHY_REFCLK_USE_PAD BIT(12)
> > @@ -363,7 +366,8 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> > val &= ~BIT(0);
> > writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> >
> > - if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> > + if (of_device_is_compatible(node, "qcom,pcie-ipq8064") |
> > + of_device_is_compatible(node, "qcom,pcie-ipq8064-v2")) {
> > writel(PCS_DEEMPH_TX_DEEMPH_GEN1(24) |
> >PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(24) |
> >PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(34),
> > @@ -374,9 +378,18 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> > writel(PHY_RX0_EQ(4), pcie->parf +
> PCIE20_PARF_CONFIG_BITS);
> > }
> >
> > + if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> > + /* set TX termination offset */
> > + val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> > + val &= ~PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK;
> > + val |= PHY_CTRL_PHY_TX0_TERM_OFFSET(7);
> > + writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> > + }
> > +
> > /* enable external reference clock */
> > val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> > - val |= BIT(16);
> > + val &= ~PHY_REFCLK_USE_PAD;
> > + val |= PHY_REFCLK_SSP_EN;
> > writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
> >
> > /* wait for clock acquisition */
> > @@ -1452,6 +1465,7 @@ static int qcom_pcie_probe(struct
> platform_device *pdev)
> > static const struct of_device_id qcom_pcie_match[] = {
> > { .compatible = "qcom,pcie-apq8084", .data = _1_0_0 },
> > { .compatible = "qcom,pcie-ipq8064", .data = _2_1_0 },
> > + { .compatible = "qcom,pcie-ipq8064-v2", .data = _2_1_0 },
> > { .compatible = "qcom,pcie-apq8064", .data = _2_1_0 },
> > { .compatible = "qcom,pcie-msm8996", .data = _2_3_2 },
> > { .compatible = "qcom,pcie-ipq8074", .data = _2_3_3 },
> > --
> > 2.25.1
> >
R: [PATCH v4 08/10] PCI: qcom: Add ipq8064 rev2 variant and set tx term offset
> -Messaggio originale-
> Da: Stanimir Varbanov
> Inviato: martedì 2 giugno 2020 09:54
> A: Ansuel Smith ; Bjorn Andersson
>
> Cc: Sham Muthayyan ; Andy Gross
> ; Bjorn Helgaas ; Rob Herring
> ; Mark Rutland ; Lorenzo
> Pieralisi ; Andrew Murray
> ; Philipp Zabel
> ; linux-arm-...@vger.kernel.org; linux-
> p...@vger.kernel.org; devicet...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Oggetto: Re: [PATCH v4 08/10] PCI: qcom: Add ipq8064 rev2 variant and
> set tx term offset
>
> Hi,
>
> On 5/14/20 11:07 PM, Ansuel Smith wrote:
> > Add tx term offset support to pcie qcom driver need in some revision of
> > the ipq806x SoC. Ipq8064 have tx term offset set to 7. Ipq8064-v2
> revision
> > and ipq8065 have the tx term offset set to 0.
> >
> > Signed-off-by: Sham Muthayyan
> > Signed-off-by: Ansuel Smith
> > ---
> > drivers/pci/controller/dwc/pcie-qcom.c | 18 --
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index f5398b0d270c..ab6f1bdd24c3 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -45,6 +45,9 @@
> > #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
> >
> > #define PCIE20_PARF_PHY_CTRL 0x40
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(20,
> 16)
>
> I see you changed the mask, did you found the issue in previous v3 mask
> and shift?
>
I checked the original code and the old GENMASK declaration was wrong as you
suggested.
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)((x) << 16)
> > +
> > #define PCIE20_PARF_PHY_REFCLK 0x4C
> > #define PHY_REFCLK_SSP_EN BIT(16)
> > #define PHY_REFCLK_USE_PAD BIT(12)
> > @@ -363,7 +366,8 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> > val &= ~BIT(0);
> > writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> >
> > - if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> > + if (of_device_is_compatible(node, "qcom,pcie-ipq8064") |
>
> this should be logical OR
>
Will change in v5 since I will have to split this
> > + of_device_is_compatible(node, "qcom,pcie-ipq8064-v2")) {
> > writel(PCS_DEEMPH_TX_DEEMPH_GEN1(24) |
> >PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(24) |
> >PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(34),
> > @@ -374,9 +378,18 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> > writel(PHY_RX0_EQ(4), pcie->parf +
> PCIE20_PARF_CONFIG_BITS);
> > }
> >
> > + if (of_device_is_compatible(node, "qcom,pcie-ipq8064")) {
> > + /* set TX termination offset */
> > + val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> > + val &= ~PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK;
> > + val |= PHY_CTRL_PHY_TX0_TERM_OFFSET(7);
> > + writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> > + }
> > +
> > /* enable external reference clock */
> > val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> > - val |= BIT(16);
> > + val &= ~PHY_REFCLK_USE_PAD;
> > + val |= PHY_REFCLK_SSP_EN;
> > writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
> >
> > /* wait for clock acquisition */
> > @@ -1452,6 +1465,7 @@ static int qcom_pcie_probe(struct
> platform_device *pdev)
> > static const struct of_device_id qcom_pcie_match[] = {
> > { .compatible = "qcom,pcie-apq8084", .data = _1_0_0 },
> > { .compatible = "qcom,pcie-ipq8064", .data = _2_1_0 },
> > + { .compatible = "qcom,pcie-ipq8064-v2", .data = _2_1_0 },
> > { .compatible = "qcom,pcie-apq8064", .data = _2_1_0 },
> > { .compatible = "qcom,pcie-msm8996", .data = _2_3_2 },
> > { .compatible = "qcom,pcie-ipq8074", .data = _2_3_3 },
> >
>
> --
> regards,
> Stan
R: R: [PATCH v3 08/11] devicetree: bindings: pci: document PARF params bindings
> On 5/12/20 6:45 PM, Rob Herring wrote: > > On Thu, May 07, 2020 at 09:34:35PM +0200, ansuels...@gmail.com > wrote: > >>> On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote: > It is now supported the editing of Tx De-Emphasis, Tx Swing and > Rx equalization params on ipq8064. Document this new optional > params. > > Signed-off-by: Ansuel Smith > --- > .../devicetree/bindings/pci/qcom,pcie.txt | 36 > +++ > 1 file changed, 36 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > >>> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > index 6efcef040741..8cc5aea8a1da 100644 > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > @@ -254,6 +254,42 @@ > - "perst-gpios" PCIe endpoint reset signal line > - "wake-gpios" PCIe endpoint wake signal line > > +- qcom,tx-deemph-gen1: > +Usage: optional (available for ipq/apq8064) > +Value type: > +Definition: Gen1 De-emphasis value. > +For ipq806x should be set to 24. > >>> > >>> Unless these need to be tuned per board, then the compatible string > for > >>> ipq806x should imply all these settings. > >>> > >> > >> It was requested by v2 to make this settings tunable. These don't change > are > >> all the same for every ipq806x SoC. The original implementation had this > >> value hardcoded for ipq806x. Should I restore this and drop this patch? > > > > Yes, please. > > I still think that the values for tx deemph and tx swing should be > tunable. But I can live with them in the driver if they not break > support for apq8064. > > The default values in the registers for apq8064 and ipq806x are: > > default your change > TX_DEEMPH_GEN121 24 > TX_DEEMPH_GEN2_3_5DB 21 24 > TX_DEEMPH_GEN2_6DB32 34 > > TX_SWING_FULL 121 120 > TX_SWING_LOW 121 120 > > So until now (without your change) apq8064 worked with default values. > I will limit this to ipq8064(-v2) if this could be a problem. > > > > Rob > > > > -- > regards, > Stan
R: [PATCH v3 09/11] PCI: qcom: add ipq8064 rev2 variant and set tx term offset
> Hi Ansuel,
>
> On 5/1/20 1:06 AM, Ansuel Smith wrote:
> > From: Sham Muthayyan
> >
> > Add tx term offset support to pcie qcom driver need in some revision of
> > the ipq806x SoC.
> > Ipq8064 have tx term offset set to 7.
> > Ipq8064-v2 revision and ipq8065 have the tx term offset set to 0.
> >
> > Signed-off-by: Sham Muthayyan
> > Signed-off-by: Ansuel Smith
> > ---
> > drivers/pci/controller/dwc/pcie-qcom.c | 15 +++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index da8058fd1925..372d2c8508b5 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -45,6 +45,9 @@
> > #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
> >
> > #define PCIE20_PARF_PHY_CTRL 0x40
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(12,
> 16)
>
> The mask definition is not correct. Should be GENMASK(20, 16)
>
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)((x) << 16)
> > +
> > #define PCIE20_PARF_PHY_REFCLK 0x4C
> > #define PHY_REFCLK_SSP_EN BIT(16)
> > #define PHY_REFCLK_USE_PAD BIT(12)
> > @@ -118,6 +121,7 @@ struct qcom_pcie_resources_2_1_0 {
> > u32 tx_swing_full;
> > u32 tx_swing_low;
> > u32 rx0_eq;
> > + u8 phy_tx0_term_offset;
> > };
> >
> > struct qcom_pcie_resources_1_0_0 {
> > @@ -318,6 +322,11 @@ static int
> qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
> > if (IS_ERR(res->ext_reset))
> > return PTR_ERR(res->ext_reset);
> >
> > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-ipq8064"))
> > + res->phy_tx0_term_offset = 7;
>
> Before your change the phy_tx0_term_offser was 0 for apq8064, but here
> you change it to 7, why?
>
apq8064 board should use qcom,pcie-apq8064 right? This should be set to 0
only with pcie-ipq8064 compatible. Tell me if I'm wrong.
> > + else
> > + res->phy_tx0_term_offset = 0;
> > +
> > res->phy_reset = devm_reset_control_get_exclusive(dev, "phy");
> > return PTR_ERR_OR_ZERO(res->phy_reset);
> > }
> > @@ -402,6 +411,11 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> > /* enable PCIe clocks and resets */
> > qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL,
> BIT(0), 0);
> >
> > + /* set TX termination offset */
> > + qcom_clear_and_set_dword(pcie->parf + PCIE20_PARF_PHY_CTRL,
> > + PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK,
>
> As the mask definition is incorrect you actually clear 12 to 16 bit in
> the register where is another PHY parameter. Is that was intentional?
>
Will check this and the wrong genmask.
> > + PHY_CTRL_PHY_TX0_TERM_OFFSET(res-
> >phy_tx0_term_offset));
> > +
> > writel(PCS_DEEMPH_TX_DEEMPH_GEN1(res->tx_deemph_gen1) |
> >PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(res-
> >tx_deemph_gen2_3p5db) |
> >PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(res-
> >tx_deemph_gen2_6db),
> > @@ -1485,6 +1499,7 @@ static int qcom_pcie_probe(struct
> platform_device *pdev)
> > static const struct of_device_id qcom_pcie_match[] = {
> > { .compatible = "qcom,pcie-apq8084", .data = _1_0_0 },
> > { .compatible = "qcom,pcie-ipq8064", .data = _2_1_0 },
> > + { .compatible = "qcom,pcie-ipq8064-v2", .data = _2_1_0 },
> > { .compatible = "qcom,pcie-apq8064", .data = _2_1_0 },
> > { .compatible = "qcom,pcie-msm8996", .data = _2_3_2 },
> > { .compatible = "qcom,pcie-ipq8074", .data = _2_3_3 },
> >
>
> --
> regards,
> Stan
R: [PATCH v3 09/11] PCI: qcom: add ipq8064 rev2 variant and set tx term offset
> On Fri, May 01, 2020 at 12:06:16AM +0200, Ansuel Smith wrote:
> > From: Sham Muthayyan
> >
> > Add tx term offset support to pcie qcom driver need in some revision of
> > the ipq806x SoC.
> > Ipq8064 have tx term offset set to 7.
> > Ipq8064-v2 revision and ipq8065 have the tx term offset set to 0.
> >
> > Signed-off-by: Sham Muthayyan
> > Signed-off-by: Ansuel Smith
> > ---
> > drivers/pci/controller/dwc/pcie-qcom.c | 15 +++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index da8058fd1925..372d2c8508b5 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -45,6 +45,9 @@
> > #define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
> >
> > #define PCIE20_PARF_PHY_CTRL 0x40
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK GENMASK(12,
> 16)
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)((x) << 16)
> > +
> > #define PCIE20_PARF_PHY_REFCLK 0x4C
> > #define PHY_REFCLK_SSP_EN BIT(16)
> > #define PHY_REFCLK_USE_PAD BIT(12)
> > @@ -118,6 +121,7 @@ struct qcom_pcie_resources_2_1_0 {
> > u32 tx_swing_full;
> > u32 tx_swing_low;
> > u32 rx0_eq;
> > + u8 phy_tx0_term_offset;
> > };
> >
> > struct qcom_pcie_resources_1_0_0 {
> > @@ -318,6 +322,11 @@ static int
> qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
> > if (IS_ERR(res->ext_reset))
> > return PTR_ERR(res->ext_reset);
> >
> > + if (of_device_is_compatible(dev->of_node, "qcom,pcie-ipq8064"))
> > + res->phy_tx0_term_offset = 7;
>
> Based on my other comments, you'll want to turn this into match data.
>
I don't understand what you mean here. I really can't think of another way
to set this only for qcom,pci-ipq8064 as ipq8064-v2 and apq8064 use the
same get resource function. Should I create a different get_resources for
the other 2 device?
> > + else
> > + res->phy_tx0_term_offset = 0;
> > +
R: [PATCH v3 08/11] devicetree: bindings: pci: document PARF params bindings
> On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote:
> > It is now supported the editing of Tx De-Emphasis, Tx Swing and
> > Rx equalization params on ipq8064. Document this new optional params.
> >
> > Signed-off-by: Ansuel Smith
> > ---
> > .../devicetree/bindings/pci/qcom,pcie.txt | 36 +++
> > 1 file changed, 36 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > index 6efcef040741..8cc5aea8a1da 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> > @@ -254,6 +254,42 @@
> > - "perst-gpios" PCIe endpoint reset signal line
> > - "wake-gpios" PCIe endpoint wake signal line
> >
> > +- qcom,tx-deemph-gen1:
> > + Usage: optional (available for ipq/apq8064)
> > + Value type:
> > + Definition: Gen1 De-emphasis value.
> > + For ipq806x should be set to 24.
>
> Unless these need to be tuned per board, then the compatible string for
> ipq806x should imply all these settings.
>
It was requested by v2 to make this settings tunable. These don't change are
all the same for every ipq806x SoC. The original implementation had this
value hardcoded for ipq806x. Should I restore this and drop this patch?
Anyway thanks for the review.
> > +
> > +- qcom,tx-deemph-gen2-3p5db:
> > + Usage: optional (available for ipq/apq8064)
> > + Value type:
> > + Definition: Gen2 (3.5db) De-emphasis value.
> > + For ipq806x should be set to 24.
> > +
> > +- qcom,tx-deemph-gen2-6db:
> > + Usage: optional (available for ipq/apq8064)
> > + Value type:
> > + Definition: Gen2 (6db) De-emphasis value.
> > + For ipq806x should be set to 34.
> > +
> > +- qcom,tx-swing-full:
> > + Usage: optional (available for ipq/apq8064)
> > + Value type:
> > + Definition: TX launch amplitude swing_full value.
> > + For ipq806x should be set to 120.
> > +
> > +- qcom,tx-swing-low:
> > + Usage: optional (available for ipq/apq8064)
> > + Value type:
> > + Definition: TX launch amplitude swing_low value.
> > + For ipq806x should be set to 120.
> > +
> > +- qcom,rx0-eq:
> > + Usage: optional (available for ipq/apq8064)
> > + Value type:
> > + Definition: RX0 equalization value.
> > + For ipq806x should be set to 4.
> > +
> > * Example for ipq/apq8064
> > pcie@1b50 {
> > compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064",
> "snps,dw-pcie";
> > --
> > 2.25.1
> >
