* Re: [PATCH v13, 2/2] net: Add dm9051 driver
From: Andrew Lunn @ 2022-01-25 13:59 UTC (permalink / raw)
To: Joseph CHAMG
Cc: David S . Miller, Jakub Kicinski, Rob Herring, joseph_chang,
netdev, devicetree, linux-kernel, andy.shevchenko, leon
In-Reply-To: <20220125085837.10357-3-josright123@gmail.com>
> +static int dm9051_mdiobus_read(struct mii_bus *mdiobus, int phy_id, int reg)
> +{
> + struct board_info *db = mdiobus->priv;
> + unsigned int val = 0;
> + int ret;
> +
> + if (phy_id == DM9051_PHY_ID) {
phy_id is a poor choice of name. It normally means the value you find
in register 2 and 3 of the PHY which identifies the manufacture, make
and possibly revision.
If you look at the read function prototype in struct mii_bus:
https://elixir.bootlin.com/linux/v5.17-rc1/source/include/linux/phy.h#L357
the normal name is addr.
Ideally your driver needs to look similar to other drivers. Ideally
you use the same variable names for the same things. That makes it
easier for somebody else to read your driver and debug it. It makes it
easier to review, etc. It is worth spending time reading a few other
drivers and looking for common patterns, and making use of those
patterns in your driver.
> +static int dm9051_map_phyup(struct board_info *db)
> +{
> + int ret;
> +
> + /* ~BMCR_PDOWN to power-up the internal phy
> + */
> + ret = mdiobus_modify(db->mdiobus, DM9051_PHY_ID, MII_BMCR, BMCR_PDOWN, 0);
> + if (ret < 0)
> + return ret;
You are still touching PHY registers from the MAC driver. Why is your
PHY driver not going this as part of the _config() function?
Andrew
^ permalink raw reply
* [PATCH 7/8] ARM: dts: imx6ul: fix lcdif node compatible
From: Alexander Stein @ 2022-01-25 13:50 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Sascha Hauer, Fabio Estevam
Cc: Alexander Stein, Pengutronix Kernel Team, NXP Linux Team,
devicetree, linux-arm-kernel
In-Reply-To: <20220125135014.3041002-1-alexander.stein@ew.tq-group.com>
In yaml binding "fsl,imx6ul-lcdif" is listed as compatible to imx6sx-lcdif,
but not imx28-lcdif. Change the list accordingly. Fixes the
dt_binding_check warning:
lcdif@21c8000: compatible: 'oneOf' conditional failed, one must be fixed:
['fsl,imx6ul-lcdif', 'fsl,imx28-lcdif'] is too long
Additional items are not allowed ('fsl,imx28-lcdif' was unexpected)
'fsl,imx6ul-lcdif' is not one of ['fsl,imx23-lcdif', 'fsl,imx28-lcdif',
'fsl,imx6sx-lcdif']
'fsl,imx6sx-lcdif' was expected
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
arch/arm/boot/dts/imx6ul.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index e75c2e164551..4d19ba7cb342 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -1007,7 +1007,7 @@ csi: csi@21c4000 {
};
lcdif: lcdif@21c8000 {
- compatible = "fsl,imx6ul-lcdif", "fsl,imx28-lcdif";
+ compatible = "fsl,imx6ul-lcdif", "fsl,imx6sx-lcdif";
reg = <0x021c8000 0x4000>;
interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6UL_CLK_LCDIF_PIX>,
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3 0/3] Add FSD SPI support
From: Mark Brown @ 2022-01-25 13:54 UTC (permalink / raw)
To: Alim Akhtar
Cc: linux-arm-kernel, linux-kernel, devicetree, linus.walleij,
robh+dt, krzysztof.kozlowski, linux-samsung-soc, pankaj.dubey,
andi, linux-spi
In-Reply-To: <20220125031604.76009-1-alim.akhtar@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 539 bytes --]
On Tue, Jan 25, 2022 at 08:46:01AM +0530, Alim Akhtar wrote:
> Note: This series is depended on [1] patches which adds
> support of FSD SoC and on Krzysztof's v6 [2] of spi schema changes
>
> [1] https://lkml.org/lkml/2022/1/24/583
> [2] https://lkml.org/lkml/2022/1/24/120
Please resend this when it can be applied, either wait until the
dependencies are in place or rebase on top of current code. In general
only build time dependencies matter here, the SoC support being merged
shouldn't be an issue one way or another.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH 8/8] ARM: dts: imx6ul: fix qspi node compatible
From: Alexander Stein @ 2022-01-25 13:50 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Sascha Hauer, Fabio Estevam
Cc: Alexander Stein, Pengutronix Kernel Team, NXP Linux Team,
devicetree, linux-arm-kernel
In-Reply-To: <20220125135014.3041002-1-alexander.stein@ew.tq-group.com>
imx6ul is not compatible to imx6sx, both have different erratas.
Fixes the dt_binding_check warning:
spi@21e0000: compatible: 'oneOf' conditional failed, one must be fixed:
['fsl,imx6ul-qspi', 'fsl,imx6sx-qspi'] is too long
Additional items are not allowed ('fsl,imx6sx-qspi' was unexpected)
'fsl,imx6ul-qspi' is not one of ['fsl,ls1043a-qspi']
'fsl,imx6ul-qspi' is not one of ['fsl,imx8mq-qspi']
'fsl,ls1021a-qspi' was expected
'fsl,imx7d-qspi' was expected
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
arch/arm/boot/dts/imx6ul.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 4d19ba7cb342..36be13bf1439 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -1028,7 +1028,7 @@ pxp: pxp@21cc000 {
qspi: spi@21e0000 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "fsl,imx6ul-qspi", "fsl,imx6sx-qspi";
+ compatible = "fsl,imx6ul-qspi";
reg = <0x021e0000 0x4000>, <0x60000000 0x10000000>;
reg-names = "QuadSPI", "QuadSPI-memory";
interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
--
2.25.1
^ permalink raw reply related
* [PATCH 6/8] ARM: dts: imx6ul: fix csi node compatible
From: Alexander Stein @ 2022-01-25 13:50 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Sascha Hauer, Fabio Estevam
Cc: Alexander Stein, Pengutronix Kernel Team, NXP Linux Team,
devicetree, linux-arm-kernel
In-Reply-To: <20220125135014.3041002-1-alexander.stein@ew.tq-group.com>
"fsl,imx6ul-csi" was never listed as compatible to "fsl,imx7-csi", neither
in yaml bindings, nor previous txt binding. Remove the imx7 part. Fixes
the dt schema check warning:
csi@21c4000: compatible: 'oneOf' conditional failed, one must be fixed:
['fsl,imx6ul-csi', 'fsl,imx7-csi'] is too long
Additional items are not allowed ('fsl,imx7-csi' was unexpected)
'fsl,imx8mm-csi' was expected
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
arch/arm/boot/dts/imx6ul.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 8d7d6cfc2c38..e75c2e164551 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -998,7 +998,7 @@ cpu_speed_grade: speed-grade@10 {
};
csi: csi@21c4000 {
- compatible = "fsl,imx6ul-csi", "fsl,imx7-csi";
+ compatible = "fsl,imx6ul-csi";
reg = <0x021c4000 0x4000>;
interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6UL_CLK_CSI>;
--
2.25.1
^ permalink raw reply related
* [PATCH 4/8] ARM: dts: imx6ul: fix adc node compatible
From: Alexander Stein @ 2022-01-25 13:50 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Sascha Hauer, Fabio Estevam
Cc: Alexander Stein, Pengutronix Kernel Team, NXP Linux Team,
devicetree, linux-arm-kernel
In-Reply-To: <20220125135014.3041002-1-alexander.stein@ew.tq-group.com>
fsl,imx6ul-adc is not listed in the binding, so remove it. Fixes the
dt_binding_check warning:
adc@2198000: compatible:0: 'fsl,vf610-adc' was expected
adc@2198000: compatible: ['fsl,imx6ul-adc', 'fsl,vf610-adc'] is too long
adc@2198000: compatible: Additional items are not allowed ('fsl,vf610-adc'
was unexpected)
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
arch/arm/boot/dts/imx6ul.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index df8b4ad62418..d6c2b0ad3eac 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -920,7 +920,7 @@ usdhc2: mmc@2194000 {
};
adc1: adc@2198000 {
- compatible = "fsl,imx6ul-adc", "fsl,vf610-adc";
+ compatible = "fsl,vf610-adc";
reg = <0x02198000 0x4000>;
interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6UL_CLK_ADC1>;
--
2.25.1
^ permalink raw reply related
* [PATCH 3/8] ARM: dts: imx6ul: fix keypad compatible
From: Alexander Stein @ 2022-01-25 13:50 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Sascha Hauer, Fabio Estevam
Cc: Alexander Stein, Pengutronix Kernel Team, NXP Linux Team,
devicetree, linux-arm-kernel
In-Reply-To: <20220125135014.3041002-1-alexander.stein@ew.tq-group.com>
According to binding, the compatible shall only contain imx6ul and imx21
compatibles. Fixes the dt_binding_check warning:
keypad@20b8000: compatible: 'oneOf' conditional failed, one must be fixed:
['fsl,imx6ul-kpp', 'fsl,imx6q-kpp', 'fsl,imx21-kpp'] is too long
Additional items are not allowed ('fsl,imx6q-kpp', 'fsl,imx21-kpp' were
unexpected)
Additional items are not allowed ('fsl,imx21-kpp' was unexpected)
'fsl,imx21-kpp' was expected
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
arch/arm/boot/dts/imx6ul.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 2fcbd9d91521..df8b4ad62418 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -544,7 +544,7 @@ fec2: ethernet@20b4000 {
};
kpp: keypad@20b8000 {
- compatible = "fsl,imx6ul-kpp", "fsl,imx6q-kpp", "fsl,imx21-kpp";
+ compatible = "fsl,imx6ul-kpp", "fsl,imx21-kpp";
reg = <0x020b8000 0x4000>;
interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6UL_CLK_KPP>;
--
2.25.1
^ permalink raw reply related
* [PATCH 5/8] ARM: dts: imx6ul: remove unsupported adc property
From: Alexander Stein @ 2022-01-25 13:50 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Sascha Hauer, Fabio Estevam
Cc: Alexander Stein, Pengutronix Kernel Team, NXP Linux Team,
devicetree, linux-arm-kernel
In-Reply-To: <20220125135014.3041002-1-alexander.stein@ew.tq-group.com>
'num-channels' is not supported by binding, nor driver, remove it. Fixes
the dt_binding_check warning:
adc@2198000: 'num-channels' does not match any of the regexes:
'pinctrl-[0-9]+'
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
arch/arm/boot/dts/imx6ul.dtsi | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index d6c2b0ad3eac..8d7d6cfc2c38 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -924,7 +924,6 @@ adc1: adc@2198000 {
reg = <0x02198000 0x4000>;
interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6UL_CLK_ADC1>;
- num-channels = <2>;
clock-names = "adc";
fsl,adck-max-frequency = <30000000>, <40000000>,
<20000000>;
--
2.25.1
^ permalink raw reply related
* [PATCH 2/8] ARM: dts: imx6ul: change operating-points to uint32-matrix
From: Alexander Stein @ 2022-01-25 13:50 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Sascha Hauer, Fabio Estevam
Cc: Alexander Stein, Pengutronix Kernel Team, NXP Linux Team,
devicetree, linux-arm-kernel
In-Reply-To: <20220125135014.3041002-1-alexander.stein@ew.tq-group.com>
operating-points is a uint32-matrix as per opp-v1.yaml. Change it
accordingly. While at it, change fsl,soc-operating-points as well,
although there is no bindings file (yet). But they should have the same
format. Fixes the dt_binding_check warning:
cpu@0: operating-points:0: [696000, 1275000, 528000, 1175000, 396000,
1025000, 198000, 950000] is too long
cpu@0: operating-points:0: Additional items are not allowed (528000,
1175000, 396000, 1025000, 198000, 950000 were unexpected)
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
arch/arm/boot/dts/imx6ul.dtsi | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 1d435a46fc5c..2fcbd9d91521 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -64,20 +64,18 @@ cpu0: cpu@0 {
clock-frequency = <696000000>;
clock-latency = <61036>; /* two CLK32 periods */
#cooling-cells = <2>;
- operating-points = <
+ operating-points =
/* kHz uV */
- 696000 1275000
- 528000 1175000
- 396000 1025000
- 198000 950000
- >;
- fsl,soc-operating-points = <
+ <696000 1275000>,
+ <528000 1175000>,
+ <396000 1025000>,
+ <198000 950000>;
+ fsl,soc-operating-points =
/* KHz uV */
- 696000 1275000
- 528000 1175000
- 396000 1175000
- 198000 1175000
- >;
+ <696000 1275000>,
+ <528000 1175000>,
+ <396000 1175000>,
+ <198000 1175000>;
clocks = <&clks IMX6UL_CLK_ARM>,
<&clks IMX6UL_CLK_PLL2_BUS>,
<&clks IMX6UL_CLK_PLL2_PFD2>,
--
2.25.1
^ permalink raw reply related
* [PATCH 1/8] ARM: dts: imx6ul: add missing properties for sram
From: Alexander Stein @ 2022-01-25 13:50 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Sascha Hauer, Fabio Estevam
Cc: Alexander Stein, Pengutronix Kernel Team, NXP Linux Team,
devicetree, linux-arm-kernel
In-Reply-To: <20220125135014.3041002-1-alexander.stein@ew.tq-group.com>
All 3 properties are required by sram.yaml. Fixes the dt_binding_check
warning:
sram@900000: '#address-cells' is a required property
sram@900000: '#size-cells' is a required property
sram@900000: 'ranges' is a required property
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
arch/arm/boot/dts/imx6ul.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index afeec01f6522..1d435a46fc5c 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -149,6 +149,9 @@ soc {
ocram: sram@900000 {
compatible = "mmio-sram";
reg = <0x00900000 0x20000>;
+ ranges = <0 0x00900000 0x20000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
};
intc: interrupt-controller@a01000 {
--
2.25.1
^ permalink raw reply related
* [PATCH 0/8] various imx6ul DT fixes
From: Alexander Stein @ 2022-01-25 13:50 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Sascha Hauer, Fabio Estevam
Cc: Alexander Stein, Pengutronix Kernel Team, NXP Linux Team,
devicetree, linux-arm-kernel
This patch set fixes several, but not all, dt_binding_check warnings. Some are
missing properties or their format, others address issues regarding to
compatible names. I only touched those where compatibles are stated in the yaml
bindings.
There are several imx6ul compatible names used by imx6ul.dtsi, but which are
not listed in the bindings. Namely:
* "fsl,imx6ul-gpt", "fsl,imx6sx-gpt"
* "fsl,imx6ul-tempmon", "fsl,imx6sx-tempmon"
* "fsl,imx6ul-gpc", "fsl,imx6q-gpc"
* "fsl,imx6ul-usdhc", "fsl,imx6sx-usdhc"
Despite GPC, the others are apperently compatible to imx6sx. I'm not sure how
to fix the DTB check warning. Add the imx6ul compatible to bindings and drivers?
Or change the .dtsi to use only the imx6sx compatible.
The GPC on imx6ul apparently does only support power gating for ARM CPU, so
this would require actually a new compatible which supports only 1 domain.
Alexander Stein (8):
ARM: dts: imx6ul: add missing properties for sram
ARM: dts: imx6ul: change operating-points to uint32-matrix
ARM: dts: imx6ul: fix keypad compatible
ARM: dts: imx6ul: fix adc node compatible
ARM: dts: imx6ul: remove unsupported adc property
ARM: dts: imx6ul: fix csi node compatible
ARM: dts: imx6ul: fix lcdif node compatible
ARM: dts: imx6ul: fix qspi node compatible
arch/arm/boot/dts/imx6ul.dtsi | 36 +++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH V6 2/7] soc: qcom: dcc:Add driver support for Data Capture and Compare unit(DCC)
From: Souradeep Chowdhury @ 2022-01-25 13:44 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Souradeep Chowdhury, Andy Gross, Rob Herring, linux-arm-kernel,
linux-kernel, linux-arm-msm, devicetree, Sai Prakash Ranjan,
Sibi Sankar, Rajendra Nayak, vkoul
In-Reply-To: <c0f792f4-522e-f1fd-5b58-579389c2c48d@quicinc.com>
On 1/17/2022 11:19 AM, Souradeep Chowdhury wrote:
>
> On 1/7/2022 9:27 PM, Bjorn Andersson wrote:
>> On Fri 07 Jan 07:27 PST 2022, Souradeep Chowdhury wrote:
>>
>>> On 1/7/2022 5:31 AM, Bjorn Andersson wrote:
>>>> On Wed 05 Jan 23:57 PST 2022, Souradeep Chowdhury wrote:
>>>>
>>>>> On 12/18/2021 1:41 AM, Bjorn Andersson wrote:
>>>>>> On Tue 10 Aug 12:54 CDT 2021, Souradeep Chowdhury wrote:
>>>>>>
>>>>>>> The DCC is a DMA Engine designed to capture and store data
>>>>>>> during system crash or software triggers.The DCC operates
>>>>>> Please include a space after '.'
>>>>> Ack
>>>>>>> based on user inputs via the sysfs interface.The user gives
>>>>>>> addresses as inputs and these addresses are stored in the
>>>>>>> form of linkedlists.In case of a system crash or a manual
>>>>>> I think the user configures the DCC hardware with "a sequence of
>>>>>> operations to be performed as DCC is triggered".
>>>>>>
>>>>>> Afaict the sequence is stored just as a sequence of operations in
>>>>>> SRAM,
>>>>>> there's no linked list involved - except in your intermediate
>>>>>> implementation.
>>>>> The user just enters the addresses as input whereas the sequence of
>>>>> operations takes
>>>>>
>>>>> place as per configuration code inside the driver. The end result
>>>>> is storage
>>>>> of these
>>>>>
>>>>> addresses inside the DCC SRAM. The DCC hardware will capture the
>>>>> value at
>>>>> these
>>>>>
>>>>> addresses on a crash or manual trigger by the user.
>>>>>
>>>>>>> software trigger by the user through the sysfs interface,
>>>>>>> the dcc captures and stores the values at these addresses.
>>>>>>> This patch contains the driver which has all the methods
>>>>>>> pertaining to the sysfs interface, auxiliary functions to
>>>>>>> support all the four fundamental operations of dcc namely
>>>>>>> read, write, first read then write and loop.The probe method
>>>>>> "first read then write" is called "read/modify/write"
>>>>> Ack
>>>>>>> here instantiates all the resources necessary for dcc to
>>>>>>> operate mainly the dedicated dcc sram where it stores the
>>>>>>> values.The DCC driver can be used for debugging purposes
>>>>>>> without going for a reboot since it can perform manual
>>>>>>> triggers.
>>>>>>>
>>>>>>> Also added the documentation for sysfs entries
>>>>>>> and explained the functionalities of each sysfs file that
>>>>>>> has been created for dcc.
>>>>>>>
>>>>>>> The following is the justification of using sysfs interface
>>>>>>> over the other alternatives like ioctls
>>>>>>>
>>>>>>> i) As can be seen from the sysfs attribute descriptions,
>>>>>>> most of it does basic hardware manipulations like dcc_enable,
>>>>>>> dcc_disable, config reset etc. As a result sysfs is preferred
>>>>>>> over ioctl as we just need to enter a 0 or 1.
>>>>>>>
>>>>>> As I mentioned in our chat, using sysfs allows us to operate the
>>>>>> interface using the shell without additional tools.
>>>>>>
>>>>>> But I don't think that it's easy to implement
>>>>>> enable/disable/reset using
>>>>>> sysfs is a strong argument. The difficult part of this ABI is the
>>>>>> operations to manipulate the sequence of operations, so that's
>>>>>> what you
>>>>>> need to have a solid plan for.
>>>>> The sysfs interface is being used to get the addresses values
>>>>> entered by the
>>>>> user
>>>>>
>>>>> and to also go for manual triggers. The sequence of operations are
>>>>> kept as a
>>>>> part of
>>>>>
>>>>> fixed driver code which is called when the user enters the data.
>>>>>
>>>> But does the hardware really just operate on "addresses values entered
>>>> by the user". Given the various types of operations: read, write,
>>>> read-modify-write and loop I get the feeling that the hardware
>>>> "executes" a series of actions.
>>>>
>>>> I'm don't think the proposed sysfs interface best exposes this to the
>>>> user and I don't think that "it's easy to implement enable/disable
>>>> attributes in sysfs" is reason enough to go with that approach.
>>> So the sysfs interface here has been introduced keeping in mind how the
>>> DCC_SRAM needs to be
>>>
>>> programmed for the dcc hardware to work. We are maintaining a list here
>>> based on the address
>>>
>>> entry. The 4 cases for the type of addresses are as follows-:
>>>
>>> i) READ ADDRESSES
>>>
>>> user enters something like "echo <addr> <len> > config"
>>>
>>> DCC driver stores the <addr> along with the length information in the
>>> DCC_SRAM.
>>>
>>> ii) WRITE ADDRESSES
>>>
>>> User enters something like "echo <addr> <write_val> 1 > config_write"
>>>
>>> DCC stores the <addr> first in sram followed by <write_val>.
>>>
>>> For the above 2 type of addresses there won't be much difference if
>>> we use
>>> IOCTL.
>>>
>>> However, for the next 2 type of addresses
>>>
>>> iii) LOOP ADDRESSES
>>>
>>> user has to enter something like below
>>>
>>> echo 9 > loop
>>> echo 0x01741010 1 > config
>>> echo 0x01741014 1 > config
>>> echo 1 > loop
>>>
>>> The DCC SRAM will be programmed precisely like the above entries
>>> where the
>>> loop count will be stored first
>>>
>>> followed by loop addresses and then again a "echo 1 > loop " marks
>>> the end
>>> of loop addresses.
>>>
>>> in DCC_SRAM.
>>>
>>> iv) READ_WRITE ADDRESSES
>>>
>>> User has to enter something like below
>>>
>>> echo <addr> > /sys/bus/platform/devices/../config
>>>
>>> echo <mask> <val> > /sys/bus/platform/devices/../rd_mod_wr
>>>
>>> Here first the <addr> is stored in DCC_SRAM followed by <mask> and
>>> then the
>>> <val>.
>>>
>>> The above representation to the user space is consistent with the dcc
>>> hardware in terms of
>>>
>>> the way the sequence of values are programmed in the DCC SRAM .
>>> Moving to
>>> IOCTL will
>>>
>>> only change the way the READ_WRITE address is represented although
>>> user will
>>> have to enter
>>>
>>> multiple parameters at once, let me know if we still need to go for the
>>> same.
>>>
>> So if I understand correctly, my concern is that if I would like to
>> perform something like (in pseudo code):
>>
>> readl(X)
>> write(1, Y)
>> readl(Z) 5 times
>>
>> then I will do this as:
>>
>> echo X > config
>> echo Y 1 > config_write
>> echo 5 > loop
>> echo Z > config
>> echo 1 > loop
>>
>> And the DCC driver will then write this to SRAM as something like:
>>
>> read X
>> write Y, 1
>> loop 5
>> read Z
>> loop
>>
>>
>> In other words, my mind and the DCC has the same representation of this
>> sequence of operations, but I have to shuffle the information into 4
>> different sysfs attributes to get there.
>>
>> The design guideline for sysfs is that each attribute should hold one
>> value per attribute, but in your model the attributes are tangled and
>> writing things to them depends on what has been written in that or other
>> attributes previously.
>>
>> I simply don't think that's a good ABI.
>
> Ack.
>
> Should I change this to have separate sysfs files dealing with
> separate type of instructions?
>
> For example I can have separate files like config_read,
> config_write(already exists), config_loop
>
> and config_read_write to handle 4 different type of instructions with
> inputs like
>
> echo <loop_offset> <loop_address_numbers> <loop_address_1>
> <loop_address_2>.. > config_loop
>
> echo <address> <mask> <val> > config_read_write
>
> and so on
Hi Bjorn,
Further to this, since in case of sysfs , we cannot have multiple values
for a single sysfs file,
handling loop instructions become difficult.
So can I go for debugfs for the above implementation?
I can have separate debugfs files to handle config_loop , config_read,
config_write, config_read_write
with inputs as follows
-> echo <read_address> > config_read
-> echo <write_address> <write_val> > config_write
-> echo <loop_counter> <loop_address_number> <loop_address1>
<loop_address2>.. > config_loop
-> echo <read_write_address> <write_mask> <write_val> > config_read_write
Let me know your thoughts regarding this.
Thanks,
Souradeep
>
>>
>> [..]
>>>>>>> + The address argument should
>>>>>>> + be given of the form <mask> <value>.For debugging
>>>>>>> + purposes sometimes we need to first read from a register
>>>>>>> + and then set some values to the register.
>>>>>>> + Example:
>>>>>>> + echo 0x80000000 > /sys/bus/platform/devices/.../config
>>>>>>> + (Set the address in config file)
>>>>>>> + echo 0xF 0xA > /sys/bus/platform/devices/.../rd_mod_wr
>>>>>>> + (Provide the mask and the value to write)
>>>>>>> +
>>>>>>> +What: /sys/bus/platform/devices/.../ready
>>>>>>> +Date: March 2021
>>>>>>> +Contact: Souradeep Chowdhury<schowdhu@codeaurora.org>
>>>>>>> +Description:
>>>>>>> + This file is used to check the status of the dcc
>>>>>>> + hardware if it's ready to take the inputs.
>>>>>> When will this read "false"?
>>>>> This will give false if the DCC hardware is not in an operational
>>>>> state.
>>>>>
>>>>> Will update accordingly.
>>>>>
>>>>>>> + Example:
>>>>>>> + cat /sys/bus/platform/devices/.../ready
>>>>>>> +
>>>>>>> +What: /sys/bus/platform/devices/.../curr_list
>>>>>>> +Date: February 2021
>>>>>>> +Contact: Souradeep Chowdhury<schowdhu@codeaurora.org>
>>>>>>> +Description:
>>>>>>> + This attribute is used to enter the linklist to be
>>>>>> I think it would be more appropriate to use the verb "select"
>>>>>> here and
>>>>>> afaict it's a "list" as the "linked" part only relates to your
>>>>>> implementation).
>>>>>>
>>>>>> But that said, I don't like this ABI. I think it would be cleaner
>>>>>> if you
>>>>>> had specific attributes for each of the lists. That way it would be
>>>>>> clear that you have N lists and they can be configured and enabled
>>>>>> independently, and there's no possible race conditions.
>>>>> So we do have attributes for independent lists in this case. The
>>>>> user is
>>>>> given the option
>>>>>
>>>>> to configure multiple lists at one go. For example I can do
>>>>>
>>>>> echo 1 > curr_list
>>>>>
>>>>> echo 0x18000010 1 > config
>>>>> echo 0x18000024 1 > config
>>>>>
>>>>> Then followed by
>>>>>
>>>>> echo 2 > curr_list
>>>>>
>>>>> echo 0x18010038 6 > config
>>>>> echo 0x18020010 1 > config
>>>>>
>>>>> We will get the output in terms of two separate list of registers
>>>>> values.
>>>>>
>>>> I understand that this will define two lists of operations and that we
>>>> will get 2 and 7 registers dumped, respectively. Perhaps unlikely, but
>>>> what happens if you try to do these two operations concurrently?
>>>>
>>>>
>>>> What I'm suggesting here is that if you have N contexts, you should
>>>> have
>>>> N interfaces to modify each one independently - simply because that's
>>>> generally a very good thing.
>>> Not sure if there will ever be a concurrency issue in this case.
>>> This is just about programming the DCC SRAM from the user entries
>>> sequentially.
>> So you've decided that two such sequences must not happen at the same
>> time. (I know it's unlikely, but there's nothing preventing me from
>> running the two snippets of echos concurrently and the outcome will be
>> unexpected)
>
> So as per the dcc hardware configuration document, parallel
> programming of lists are not
>
> supported for dcc. We program the lists sequentially one after the other.
>
>>
>>> The curr_list number is nothing but some register writes
>>> done in the dcc so that the dcc_hardware knows the beginning and end
>>> of a particular list and can dump the captured data according. Even if
>>> an user chooses multiple curr_list entries, it will be programmed
>>> sequentially in DCC_SRAM.
>>>
>> So there's no separation between the lists in the hardware?
>>
>> Looking at the driver I get a sense that we have N lists that can be
>> configured independently and will be run "independently" upon a trigger.
>>
>> If this isn't the case, what's the purpose of the multiple lists?
>
> Lists are programmed sequentially from the driver perspective.
>
> We have multiple lists in case of dcc because multiple software
> components may be using
>
> dcc hardware at once in which case there are individual lists allotted
> for each individual components.
>
> For example kernel might have a few lists to access whereas the rest
> maybe used by SDI which is a
>
> separate component. Each list is of arbitrary length as entered by the
> user and one list can be updated
>
> with it's base address after the previous list has been programmed in
> DCC_SRAM like below :-
>
> ret = __dcc_ll_cfg(drvdata, list);
> if (ret) {
> dcc_writel(drvdata, 0, DCC_LL_LOCK(list));
> goto err;
> }
>
> /* 3. program DCC_RAM_CFG reg */
> dcc_writel(drvdata, ram_cfg_base +
> drvdata->ram_offset/4, DCC_LL_BASE(list));
>
>
>>
>>>>>>> + used while appending addresses.The range of values
>>>>>>> + for this can be from 0 to 3.This feature is given in
>>>>>>> + order to use certain linkedlist for certain debugging
>>>>>>> + purposes.
>>>>>>> + Example:
>>>>>>> + echo 0 > /sys/bus/platform/devices/10a2000.dcc/curr_list
>>>>>>> +
>>>> [..]
>>>>>>> diff --git a/drivers/soc/qcom/dcc.c b/drivers/soc/qcom/dcc.c
>>>> [..]
>>>>>>> +static int dcc_valid_list(struct dcc_drvdata *drvdata, int
>>>>>>> curr_list)
>>>>>>> +{
>>>>>>> + u32 lock_reg;
>>>>>>> +
>>>>>>> + if (list_empty(&drvdata->cfg_head[curr_list]))
>>>>>>> + return -EINVAL;
>>>>>>> +
>>>>>>> + if (drvdata->enable[curr_list]) {
>>>>>>> + dev_err(drvdata->dev, "List %d is already enabled\n",
>>>>>>> + curr_list);
>>>>>>> + return -EINVAL;
>>>>>>> + }
>>>>>>> +
>>>>>>> + lock_reg = dcc_readl(drvdata, DCC_LL_LOCK(curr_list));
>>>>>> Under what circumstances would this differ from
>>>>>> drvdata->enable[curr_list}?
>>>>> So locking the list is done on the register as soon as the user
>>>>> enters the
>>>>> curr_list entry whereas
>>>>>
>>>>> the list is marked as enabled only on successfully programming the
>>>>> SRAM
>>>>> contents. So a list can
>>>>>
>>>>> be locked and not marked enabled in certain cases. The first is
>>>>> used so that
>>>>> the user doesn't
>>>>>
>>>>> mistakenly enter the same curr_list twice whereas the later is
>>>>> used to mark
>>>>> that the list has been
>>>>>
>>>>> successfully configured.
>>>>>
>>>> So this will mark the list as "actively in use, but disabled"? Why is
>>>> this kept in the hardware? When is this not the same as the list of
>>>> operations for that list being non-empty?
>>> So this is in accordance with the dcc hardware configuration
>>> requirement. We have to lock the list first and after that proceed
>>> with the subsequent writes.
>> But what does this mean? What happens when I lock a list?
>>
>> Afacit we have a "lock" bit and an "enable" bit. So in what circumstance
>> does the hardware care about a list being locked? Wouldn't it be
>> sufficient to just have the enable bit?
>
> As explained above multiple software components might be using DCC
> hardware for which lock bit is
>
> necessary. From only kernel perspective enable bit alone suffices for
> the operation.
>
>>
>>> As per the driver code below
>>>
>>> /* 1. Take ownership of the list */
>>> dcc_writel(drvdata, BIT(0), DCC_LL_LOCK(list));
>>>
>>> /* 2. Program linked-list in the SRAM */
>>> ram_cfg_base = drvdata->ram_cfg;
>>> ret = __dcc_ll_cfg(drvdata, list);
>>> if (ret) {
>>> dcc_writel(drvdata, 0, DCC_LL_LOCK(list));
>>> goto err;
>>> }
>>>
>>> /* 3. program DCC_RAM_CFG reg */
>>> dcc_writel(drvdata, ram_cfg_base +
>>> drvdata->ram_offset/4, DCC_LL_BASE(list));
>>> dcc_writel(drvdata, drvdata->ram_start +
>>> drvdata->ram_offset/4, DCC_FD_BASE(list));
>>> dcc_writel(drvdata, 0xFFF, DCC_LL_TIMEOUT(list));
>>>
>>> /* 4. Clears interrupt status register */
>>> dcc_writel(drvdata, 0, DCC_LL_INT_ENABLE(list));
>>> dcc_writel(drvdata, (BIT(0) | BIT(1) | BIT(2)),
>>> DCC_LL_INT_STATUS(list));
>>>
>>> In case of any errors we again unlock the list before exiting.
>>>
>> So it needs to be locked while SRAM contains a valid sequence of
>> operations?
>>
>> Or does it need to be locked while we write to SRAM? If so, why is that?
>
> So it needs to be locked while SRAM contains a valid sequence of
> operations.
>
> The reason has been explained as above.
>
>> Regards,
>> Bjorn
^ permalink raw reply
* Re: [PATCH v6 05/13] peci: Add peci-aspeed controller driver
From: kernel test robot @ 2022-01-25 13:31 UTC (permalink / raw)
To: Iwona Winiarska, linux-kernel, openbmc, Greg Kroah-Hartman
Cc: kbuild-all, devicetree, linux-aspeed, linux-arm-kernel,
linux-hwmon, linux-doc, Rob Herring, Joel Stanley
In-Reply-To: <20220125011104.2480133-6-iwona.winiarska@intel.com>
Hi Iwona,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on linux/master linus/master v5.17-rc1 next-20220125]
[cannot apply to joel-aspeed/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Iwona-Winiarska/Introduce-PECI-subsystem/20220125-115946
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20220125/202201252130.U4qxBhmg-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/35075a61a26913806122a9b500915dc66ad678bd
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Iwona-Winiarska/Introduce-PECI-subsystem/20220125-115946
git checkout 35075a61a26913806122a9b500915dc66ad678bd
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sh SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/clk/clk.c:856:6: error: redefinition of 'clk_unprepare'
856 | void clk_unprepare(struct clk *clk)
| ^~~~~~~~~~~~~
In file included from drivers/clk/clk.c:9:
include/linux/clk.h:303:20: note: previous definition of 'clk_unprepare' with type 'void(struct clk *)'
303 | static inline void clk_unprepare(struct clk *clk)
| ^~~~~~~~~~~~~
>> drivers/clk/clk.c:937:5: error: redefinition of 'clk_prepare'
937 | int clk_prepare(struct clk *clk)
| ^~~~~~~~~~~
In file included from drivers/clk/clk.c:9:
include/linux/clk.h:271:19: note: previous definition of 'clk_prepare' with type 'int(struct clk *)'
271 | static inline int clk_prepare(struct clk *clk)
| ^~~~~~~~~~~
>> drivers/clk/clk.c:1183:6: error: redefinition of 'clk_is_enabled_when_prepared'
1183 | bool clk_is_enabled_when_prepared(struct clk *clk)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/clk/clk.c:9:
include/linux/clk.h:284:20: note: previous definition of 'clk_is_enabled_when_prepared' with type 'bool(struct clk *)' {aka '_Bool(struct clk *)'}
284 | static inline bool clk_is_enabled_when_prepared(struct clk *clk)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for COMMON_CLK
Depends on !HAVE_LEGACY_CLK
Selected by
- PECI_ASPEED && PECI && (ARCH_ASPEED || COMPILE_TEST && OF && HAS_IOMEM
vim +/clk_unprepare +856 drivers/clk/clk.c
a6adc30ba7bef8 Dong Aisheng 2016-06-30 844
4dff95dc9477a3 Stephen Boyd 2015-04-30 845 /**
4dff95dc9477a3 Stephen Boyd 2015-04-30 846 * clk_unprepare - undo preparation of a clock source
4dff95dc9477a3 Stephen Boyd 2015-04-30 847 * @clk: the clk being unprepared
4dff95dc9477a3 Stephen Boyd 2015-04-30 848 *
4dff95dc9477a3 Stephen Boyd 2015-04-30 849 * clk_unprepare may sleep, which differentiates it from clk_disable. In a
4dff95dc9477a3 Stephen Boyd 2015-04-30 850 * simple case, clk_unprepare can be used instead of clk_disable to gate a clk
4dff95dc9477a3 Stephen Boyd 2015-04-30 851 * if the operation may sleep. One example is a clk which is accessed over
4dff95dc9477a3 Stephen Boyd 2015-04-30 852 * I2c. In the complex case a clk gate operation may require a fast and a slow
4dff95dc9477a3 Stephen Boyd 2015-04-30 853 * part. It is this reason that clk_unprepare and clk_disable are not mutually
4dff95dc9477a3 Stephen Boyd 2015-04-30 854 * exclusive. In fact clk_disable must be called before clk_unprepare.
4dff95dc9477a3 Stephen Boyd 2015-04-30 855 */
4dff95dc9477a3 Stephen Boyd 2015-04-30 @856 void clk_unprepare(struct clk *clk)
b2476490ef1113 Mike Turquette 2012-03-15 857 {
4dff95dc9477a3 Stephen Boyd 2015-04-30 858 if (IS_ERR_OR_NULL(clk))
4dff95dc9477a3 Stephen Boyd 2015-04-30 859 return;
b2476490ef1113 Mike Turquette 2012-03-15 860
a6adc30ba7bef8 Dong Aisheng 2016-06-30 861 clk_core_unprepare_lock(clk->core);
1e435256d625c2 Olof Johansson 2013-04-27 862 }
4dff95dc9477a3 Stephen Boyd 2015-04-30 863 EXPORT_SYMBOL_GPL(clk_unprepare);
1e435256d625c2 Olof Johansson 2013-04-27 864
4dff95dc9477a3 Stephen Boyd 2015-04-30 865 static int clk_core_prepare(struct clk_core *core)
4dff95dc9477a3 Stephen Boyd 2015-04-30 866 {
4dff95dc9477a3 Stephen Boyd 2015-04-30 867 int ret = 0;
b2476490ef1113 Mike Turquette 2012-03-15 868
a63347251907d7 Stephen Boyd 2015-05-06 869 lockdep_assert_held(&prepare_lock);
a63347251907d7 Stephen Boyd 2015-05-06 870
4dff95dc9477a3 Stephen Boyd 2015-04-30 871 if (!core)
4dff95dc9477a3 Stephen Boyd 2015-04-30 872 return 0;
b2476490ef1113 Mike Turquette 2012-03-15 873
4dff95dc9477a3 Stephen Boyd 2015-04-30 874 if (core->prepare_count == 0) {
9a34b45397e5a3 Marek Szyprowski 2017-08-21 875 ret = clk_pm_runtime_get(core);
4dff95dc9477a3 Stephen Boyd 2015-04-30 876 if (ret)
4dff95dc9477a3 Stephen Boyd 2015-04-30 877 return ret;
b2476490ef1113 Mike Turquette 2012-03-15 878
9a34b45397e5a3 Marek Szyprowski 2017-08-21 879 ret = clk_core_prepare(core->parent);
9a34b45397e5a3 Marek Szyprowski 2017-08-21 880 if (ret)
9a34b45397e5a3 Marek Szyprowski 2017-08-21 881 goto runtime_put;
9a34b45397e5a3 Marek Szyprowski 2017-08-21 882
4dff95dc9477a3 Stephen Boyd 2015-04-30 883 trace_clk_prepare(core);
1c155b3dfe0835 Ulf Hansson 2013-03-12 884
4dff95dc9477a3 Stephen Boyd 2015-04-30 885 if (core->ops->prepare)
4dff95dc9477a3 Stephen Boyd 2015-04-30 886 ret = core->ops->prepare(core->hw);
1c155b3dfe0835 Ulf Hansson 2013-03-12 887
4dff95dc9477a3 Stephen Boyd 2015-04-30 888 trace_clk_prepare_complete(core);
b2476490ef1113 Mike Turquette 2012-03-15 889
9a34b45397e5a3 Marek Szyprowski 2017-08-21 890 if (ret)
9a34b45397e5a3 Marek Szyprowski 2017-08-21 891 goto unprepare;
b2476490ef1113 Mike Turquette 2012-03-15 892 }
b2476490ef1113 Mike Turquette 2012-03-15 893
4dff95dc9477a3 Stephen Boyd 2015-04-30 894 core->prepare_count++;
b2476490ef1113 Mike Turquette 2012-03-15 895
9461f7b33d11cb Jerome Brunet 2018-06-19 896 /*
9461f7b33d11cb Jerome Brunet 2018-06-19 897 * CLK_SET_RATE_GATE is a special case of clock protection
9461f7b33d11cb Jerome Brunet 2018-06-19 898 * Instead of a consumer claiming exclusive rate control, it is
9461f7b33d11cb Jerome Brunet 2018-06-19 899 * actually the provider which prevents any consumer from making any
9461f7b33d11cb Jerome Brunet 2018-06-19 900 * operation which could result in a rate change or rate glitch while
9461f7b33d11cb Jerome Brunet 2018-06-19 901 * the clock is prepared.
9461f7b33d11cb Jerome Brunet 2018-06-19 902 */
9461f7b33d11cb Jerome Brunet 2018-06-19 903 if (core->flags & CLK_SET_RATE_GATE)
9461f7b33d11cb Jerome Brunet 2018-06-19 904 clk_core_rate_protect(core);
9461f7b33d11cb Jerome Brunet 2018-06-19 905
4dff95dc9477a3 Stephen Boyd 2015-04-30 906 return 0;
9a34b45397e5a3 Marek Szyprowski 2017-08-21 907 unprepare:
9a34b45397e5a3 Marek Szyprowski 2017-08-21 908 clk_core_unprepare(core->parent);
9a34b45397e5a3 Marek Szyprowski 2017-08-21 909 runtime_put:
9a34b45397e5a3 Marek Szyprowski 2017-08-21 910 clk_pm_runtime_put(core);
9a34b45397e5a3 Marek Szyprowski 2017-08-21 911 return ret;
b2476490ef1113 Mike Turquette 2012-03-15 912 }
b2476490ef1113 Mike Turquette 2012-03-15 913
a6adc30ba7bef8 Dong Aisheng 2016-06-30 914 static int clk_core_prepare_lock(struct clk_core *core)
a6adc30ba7bef8 Dong Aisheng 2016-06-30 915 {
a6adc30ba7bef8 Dong Aisheng 2016-06-30 916 int ret;
a6adc30ba7bef8 Dong Aisheng 2016-06-30 917
a6adc30ba7bef8 Dong Aisheng 2016-06-30 918 clk_prepare_lock();
a6adc30ba7bef8 Dong Aisheng 2016-06-30 919 ret = clk_core_prepare(core);
a6adc30ba7bef8 Dong Aisheng 2016-06-30 920 clk_prepare_unlock();
a6adc30ba7bef8 Dong Aisheng 2016-06-30 921
a6adc30ba7bef8 Dong Aisheng 2016-06-30 922 return ret;
a6adc30ba7bef8 Dong Aisheng 2016-06-30 923 }
a6adc30ba7bef8 Dong Aisheng 2016-06-30 924
4dff95dc9477a3 Stephen Boyd 2015-04-30 925 /**
4dff95dc9477a3 Stephen Boyd 2015-04-30 926 * clk_prepare - prepare a clock source
4dff95dc9477a3 Stephen Boyd 2015-04-30 927 * @clk: the clk being prepared
4dff95dc9477a3 Stephen Boyd 2015-04-30 928 *
4dff95dc9477a3 Stephen Boyd 2015-04-30 929 * clk_prepare may sleep, which differentiates it from clk_enable. In a simple
4dff95dc9477a3 Stephen Boyd 2015-04-30 930 * case, clk_prepare can be used instead of clk_enable to ungate a clk if the
4dff95dc9477a3 Stephen Boyd 2015-04-30 931 * operation may sleep. One example is a clk which is accessed over I2c. In
4dff95dc9477a3 Stephen Boyd 2015-04-30 932 * the complex case a clk ungate operation may require a fast and a slow part.
4dff95dc9477a3 Stephen Boyd 2015-04-30 933 * It is this reason that clk_prepare and clk_enable are not mutually
4dff95dc9477a3 Stephen Boyd 2015-04-30 934 * exclusive. In fact clk_prepare must be called before clk_enable.
4dff95dc9477a3 Stephen Boyd 2015-04-30 935 * Returns 0 on success, -EERROR otherwise.
4dff95dc9477a3 Stephen Boyd 2015-04-30 936 */
4dff95dc9477a3 Stephen Boyd 2015-04-30 @937 int clk_prepare(struct clk *clk)
b2476490ef1113 Mike Turquette 2012-03-15 938 {
035a61c314eb3d Tomeu Vizoso 2015-01-23 939 if (!clk)
4dff95dc9477a3 Stephen Boyd 2015-04-30 940 return 0;
035a61c314eb3d Tomeu Vizoso 2015-01-23 941
a6adc30ba7bef8 Dong Aisheng 2016-06-30 942 return clk_core_prepare_lock(clk->core);
7ef3dcc8145263 James Hogan 2013-07-29 943 }
4dff95dc9477a3 Stephen Boyd 2015-04-30 944 EXPORT_SYMBOL_GPL(clk_prepare);
035a61c314eb3d Tomeu Vizoso 2015-01-23 945
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH v9 2/2] clk: microchip: Add driver for Microchip PolarFire SoC
From: conor.dooley @ 2022-01-25 13:40 UTC (permalink / raw)
To: sboyd
Cc: conor.dooley, cyril.jean, daire.mcnamara, david.abdurachmanov,
devicetree, geert, krzysztof.kozlowski, linux-clk, mturquette,
padmarao.begari, palmer, robh+dt
In-Reply-To: <20220125004818.83E27C340E4@smtp.kernel.org>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Quoting conor.dooley@microchip.com (2021-12-16 06:00:22)
> > diff --git a/drivers/clk/microchip/Makefile b/drivers/clk/microchip/Makefile
> > index f34b247e870f..0dce0b12eac4 100644
> > --- a/drivers/clk/microchip/Makefile
> > +++ b/drivers/clk/microchip/Makefile
Snipping the rest, will/have addressed them.
> > +static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hws,
> > + unsigned int num_clks, struct mpfs_clock_data *data,
> > + struct clk *clk_parent)
> > +{
> > + struct clk_hw *hw;
> > + void __iomem *sys_base = data->base;
> > + unsigned int i, id;
> > +
> > + for (i = 0; i < num_clks; i++) {
> > + struct mpfs_cfg_hw_clock *cfg_hw = &cfg_hws[i];
> > +
> > + cfg_hw->cfg.parent = __clk_get_hw(clk_parent);
> > + cfg_hw->hw.init = CLK_HW_INIT_HW(cfg_hw->cfg.name, cfg_hw->cfg.parent,
> > + &mpfs_clk_cfg_ops, cfg_hw->cfg.flags);
> > + hw = mpfs_clk_register_cfg(dev, cfg_hw, sys_base);
> > + if (IS_ERR(hw)) {
> > + dev_err(dev, "failed to register clock %s\n", cfg_hw->cfg.name);
> > + goto err_clk;
> > + }
> > +
> > + id = cfg_hws[i].cfg.id;
> > + data->hw_data.hws[id] = hw;
> > + }
> > +
> > + return 0;
> > +
> > +err_clk:
> > + while (i--)
> > + devm_clk_hw_unregister(dev, data->hw_data.hws[cfg_hws[i].cfg.id]);
>
> > + clk_parent = devm_clk_get(dev, NULL);
>
> Use clk_parent_data instead please.
>
> > + if (IS_ERR(clk_parent))
> > + return PTR_ERR(clk_parent);
Please correct me if I am misinterpreting:
I had the devm_clk_get() in there to pickup the refclk from the device
tree as a result of previous feedback. I have replaced this with the
following, which I have found in several other drivers - does it achieve
the same thing?
If it does, all of the args to CLK_HW_INIT_PARENTS_DATA are now set at
compile time & I will take CLK_HW_INIT_PARENTS_DATA back out of this
function.
static struct clk_parent_data mpfs_cfg_parent[] = {
{ .index = 0 },
};
static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hws,
unsigned int num_clks, struct mpfs_clock_data *data)
{
void __iomem *sys_base = data->base;
unsigned int i, id;
int ret;
for (i = 0; i < num_clks; i++) {
struct mpfs_cfg_hw_clock *cfg_hw = &cfg_hws[i];
cfg_hw->hw.init = CLK_HW_INIT_PARENTS_DATA(cfg_hw->cfg.name, mpfs_cfg_parent,
&mpfs_clk_cfg_ops, cfg_hw->cfg.flags);
ret = mpfs_clk_register_cfg(dev, cfg_hw, sys_base);
if (ret) {
dev_err_probe(dev, ret, "failed to register clock %s\n",
cfg_hw->cfg.name);
return ret;
}
id = cfg_hws[i].cfg.id;
data->hw_data.hws[id] = &cfg_hw->hw;
}
return 0;
}
^ permalink raw reply
* Re: [PATCH v10 1/3] pwm: driver for qualcomm ipq6018 pwm block
From: Baruch Siach @ 2022-01-25 13:03 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Thierry Reding, Andy Gross, Bjorn Andersson, Balaji Prakash J,
Rob Herring, Robert Marko, Kathiravan T, linux-pwm, devicetree,
linux-arm-msm, linux-arm-kernel
In-Reply-To: <20220119172439.be4xpaqgwzdy26oh@pengutronix.de>
Hi Uwe,
Thanks for your detailed review and comments. Please find my comments
below.
On Wed, Jan 19 2022, Uwe Kleine-König wrote:
> On Tue, Dec 14, 2021 at 06:27:17PM +0200, Baruch Siach wrote:
>> From: Baruch Siach <baruch.siach@siklu.com>
>>
>> Driver for the PWM block in Qualcomm IPQ6018 line of SoCs. Based on
>> driver from downstream Codeaurora kernel tree. Removed support for older
>> (V1) variants because I have no access to that hardware.
>>
>> Tested on IPQ6010 based hardware.
>>
>> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
>> ---
>> v10:
>>
>> Restore round up in pwm_div calculation; otherwise diff is always <=
>> 0, so only bingo match works
>>
>> Don't overwrite min_diff on every loop iteration
>>
>> v9:
>>
>> Address comment from Uwe Kleine-König:
>>
>> Use period_ns*rate in dividers calculation for better accuracy
>>
>> Round down pre_div and pwm_div
>>
>> Add a comment explaining why pwm_div can't underflow
>>
>> Add a comment explaining why pre_div > pwm_div end the search loop
>>
>> Drop 'CFG_' from register macros
>>
>> Rename to_ipq_pwm_chip() to ipq_pwm_from_chip()
>>
>> Change bare 'unsigned' to 'unsigned int'
>>
>> Clarify the comment on separate REG1 write for enable/disable
>>
>> Round up the period value in .get_state
>>
>> Use direct readl/writel so no need to check for regmap errors
>>
>> v7:
>>
>> Change 'offset' to 'reg' for the tcsr offset (Rob)
>>
>> Drop clock name; there is only one clock (Bjorn)
>>
>> Simplify probe failure code path (Bjorn)
>>
>> v6:
>>
>> Address Uwe Kleine-König review comments:
>>
>> Drop IPQ_PWM_MAX_DEVICES
>>
>> Rely on assigned-clock-rates; drop IPQ_PWM_CLK_SRC_FREQ
>>
>> Simplify register offset calculation
>>
>> Calculate duty cycle more precisely
>>
>> Refuse to set inverted polarity
>>
>> Drop redundant IPQ_PWM_REG1_ENABLE bit clear
>>
>> Remove x1000 factor in pwm_div calculation, use rate directly, and round up
>>
>> Choose initial pre_div such that pwm_div < IPQ_PWM_MAX_DIV
>>
>> Ensure pre_div <= pwm_div
>>
>> Rename close_ to best_
>>
>> Explain in comment why effective_div doesn't overflow
>>
>> Limit pwm_div to IPQ_PWM_MAX_DIV - 1 to allow 100% duty cycle
>>
>> Disable clock only after pwmchip_remove()
>>
>> const pwm_ops
>>
>> Other changes:
>>
>> Add missing linux/bitfield.h header include (kernel test robot)
>>
>> Adjust code for PWM device node under TCSR (Rob Herring)
>>
>> v5:
>>
>> Use &tcsr_q6 syscon to access registers (Bjorn Andersson)
>>
>> Address Uwe Kleine-König review comments:
>>
>> Implement .get_state()
>>
>> Add IPQ_PWM_ prefix to local macros
>>
>> Use GENMASK/BIT/FIELD_PREP for register fields access
>>
>> Make type of config_div_and_duty() parameters consistent
>>
>> Derive IPQ_PWM_MIN_PERIOD_NS from IPQ_PWM_CLK_SRC_FREQ
>>
>> Integrate enable/disable into config_div_and_duty() to save register read,
>> and reduce frequency glitch on update
>>
>> Use min() instead of min_t()
>>
>> Fix comment format
>>
>> Use dev_err_probe() to indicate probe step failure
>>
>> Add missing clk_disable_unprepare() in .remove
>>
>> Don't set .owner
>>
>> v4:
>>
>> Use div64_u64() to fix link for 32-bit targets ((kernel test robot
>> <lkp@intel.com>, Uwe Kleine-König)
>>
>> v3:
>>
>> s/qcom,pwm-ipq6018/qcom,ipq6018-pwm/ (Rob Herring)
>>
>> Fix integer overflow on 32-bit targets (kernel test robot <lkp@intel.com>)
>>
>> v2:
>>
>> Address Uwe Kleine-König review comments:
>>
>> Fix period calculation when out of range
>>
>> Don't set period larger than requested
>>
>> Remove PWM disable on configuration change
>>
>> Implement .apply instead of non-atomic .config/.enable/.disable
>>
>> Don't modify PWM on .request/.free
>>
>> Check pwm_div underflow
>>
>> Fix various code and comment formatting issues
>>
>> Other changes:
>>
>> Use u64 divisor safe division
>>
>> Remove now empty .request/.free
>> ---
>> drivers/pwm/Kconfig | 12 ++
>> drivers/pwm/Makefile | 1 +
>> drivers/pwm/pwm-ipq.c | 275 ++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 288 insertions(+)
>> create mode 100644 drivers/pwm/pwm-ipq.c
>>
>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>> index 21e3b05a5153..e39718137ecd 100644
>> --- a/drivers/pwm/Kconfig
>> +++ b/drivers/pwm/Kconfig
>> @@ -260,6 +260,18 @@ config PWM_INTEL_LGM
>> To compile this driver as a module, choose M here: the module
>> will be called pwm-intel-lgm.
>>
>> +config PWM_IPQ
>> + tristate "IPQ PWM support"
>> + depends on ARCH_QCOM || COMPILE_TEST
>> + depends on HAVE_CLK && HAS_IOMEM
>> + help
>> + Generic PWM framework driver for IPQ PWM block which supports
>> + 4 pwm channels. Each of the these channels can be configured
>> + independent of each other.
>> +
>> + To compile this driver as a module, choose M here: the module
>> + will be called pwm-ipq.
>> +
>> config PWM_IQS620A
>> tristate "Azoteq IQS620A PWM support"
>> depends on MFD_IQS62X || COMPILE_TEST
>> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
>> index 708840b7fba8..7402feae4b36 100644
>> --- a/drivers/pwm/Makefile
>> +++ b/drivers/pwm/Makefile
>> @@ -22,6 +22,7 @@ obj-$(CONFIG_PWM_IMX1) += pwm-imx1.o
>> obj-$(CONFIG_PWM_IMX27) += pwm-imx27.o
>> obj-$(CONFIG_PWM_IMX_TPM) += pwm-imx-tpm.o
>> obj-$(CONFIG_PWM_INTEL_LGM) += pwm-intel-lgm.o
>> +obj-$(CONFIG_PWM_IPQ) += pwm-ipq.o
>> obj-$(CONFIG_PWM_IQS620A) += pwm-iqs620a.o
>> obj-$(CONFIG_PWM_JZ4740) += pwm-jz4740.o
>> obj-$(CONFIG_PWM_KEEMBAY) += pwm-keembay.o
>> diff --git a/drivers/pwm/pwm-ipq.c b/drivers/pwm/pwm-ipq.c
>> new file mode 100644
>> index 000000000000..3764010808f0
>> --- /dev/null
>> +++ b/drivers/pwm/pwm-ipq.c
>> @@ -0,0 +1,275 @@
>> +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
>> +/*
>> + * Copyright (c) 2016-2017, 2020 The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pwm.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/math64.h>
>> +#include <linux/of_device.h>
>> +#include <linux/bitfield.h>
>> +
>> +/* The frequency range supported is 1 Hz to clock rate */
>> +#define IPQ_PWM_MAX_PERIOD_NS ((u64)NSEC_PER_SEC)
>> +
>> +/*
>> + * The max value specified for each field is based on the number of bits
>> + * in the pwm control register for that field
>> + */
>> +#define IPQ_PWM_MAX_DIV 0xFFFF
>> +
>> +/*
>> + * Two 32-bit registers for each PWM: REG0, and REG1.
>> + * Base offset for PWM #i is at 8 * #i.
>> + */
>> +#define IPQ_PWM_REG0 0 /*PWM_DIV PWM_HI*/
>> +#define IPQ_PWM_REG0_PWM_DIV GENMASK(15, 0)
>> +#define IPQ_PWM_REG0_HI_DURATION GENMASK(31, 16)
>
> PWM_HI in the comment of IPQ_PWM_REG0 vs. HI_DURATION? Should this
> match? I'd say the comment is redundant.
>
>> +#define IPQ_PWM_REG1 4 /*ENABLE UPDATE PWM_PRE_DIV*/
>> +#define IPQ_PWM_REG1_PRE_DIV GENMASK(15, 0)
>> +/*
>> + * Enable bit is set to enable output toggling in pwm device.
>> + * Update bit is set to reflect the changed divider and high duration
>> + * values in register.
>> + */
>> +#define IPQ_PWM_REG1_UPDATE BIT(30)
>> +#define IPQ_PWM_REG1_ENABLE BIT(31)
>> +
>> +
>> +struct ipq_pwm_chip {
>> + struct pwm_chip chip;
>> + struct clk *clk;
>> + void __iomem *mem;
>> +};
>> +
>> +static struct ipq_pwm_chip *ipq_pwm_from_chip(struct pwm_chip *chip)
>> +{
>> + return container_of(chip, struct ipq_pwm_chip, chip);
>> +}
>> +
>> +static unsigned int ipq_pwm_reg_read(struct pwm_device *pwm, unsigned int reg)
>> +{
>> + struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(pwm->chip);
>> + unsigned int off = 8 * pwm->hwpwm + reg;
>> +
>> + return readl(ipq_chip->mem + off);
>> +}
>> +
>> +static void ipq_pwm_reg_write(struct pwm_device *pwm, unsigned int reg,
>> + unsigned int val)
>> +{
>> + struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(pwm->chip);
>> + unsigned int off = 8 * pwm->hwpwm + reg;
>> +
>> + writel(val, ipq_chip->mem + off);
>> +}
>> +
>> +static void config_div_and_duty(struct pwm_device *pwm, unsigned int pre_div,
>> + unsigned int pwm_div, unsigned long rate, u64 duty_ns,
>> + bool enable)
>> +{
>> + unsigned long hi_dur;
>> + unsigned long val = 0;
>> +
>> + /*
>> + * high duration = pwm duty * (pwm div + 1)
>> + * pwm duty = duty_ns / period_ns
>> + */
>> + hi_dur = div64_u64(duty_ns * rate, (pre_div + 1) * NSEC_PER_SEC);
>> +
>> + val = FIELD_PREP(IPQ_PWM_REG0_HI_DURATION, hi_dur) |
>> + FIELD_PREP(IPQ_PWM_REG0_PWM_DIV, pwm_div);
>> + ipq_pwm_reg_write(pwm, IPQ_PWM_REG0, val);
>> +
>> + val = FIELD_PREP(IPQ_PWM_REG1_PRE_DIV, pre_div);
>> + ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
>> +
>> + /* PWM enable toggle needs a separate write to REG1 */
>> + val |= IPQ_PWM_REG1_UPDATE;
>> + if (enable)
>> + val |= IPQ_PWM_REG1_ENABLE;
>> + ipq_pwm_reg_write(pwm, IPQ_PWM_REG1, val);
>> +}
>> +
>> +static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>> + const struct pwm_state *state)
>> +{
>> + struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(chip);
>> + unsigned int pre_div, pwm_div, best_pre_div, best_pwm_div;
>> + unsigned long rate = clk_get_rate(ipq_chip->clk);
>> + u64 period_ns, duty_ns, period_rate;
>> + u64 min_diff;
>> +
>> + if (state->polarity != PWM_POLARITY_NORMAL)
>> + return -EINVAL;
>> +
>> + if (state->period < div64_u64(NSEC_PER_SEC, rate))
>> + return -ERANGE;
>
> NSEC_PER_SEC / rate is the smallest period you can achieve, right?
> Consider rate = 33333 (Hz), then the minimal period is
> 30000.30000300003 ns. So you should refuse a request to configure
> state->period = 30000, but as div64_u64(1000000000, 33333) is 30000 you
> don't.
>
>> + period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
>> + duty_ns = min(state->duty_cycle, period_ns);
>> +
>> + /*
>> + * period_ns is 1G or less. As long as rate is less than 16 GHz this
>> + * does not overflow.
>
> Well, rate cannot be bigger than 4294967295 because an unsigned long
> cannot hold a bigger value.
On 64-bit systems __SIZEOF_LONG__ is 8, which can hold more than 2^32.
>> + */
>> + period_rate = period_ns * rate;
>> + best_pre_div = IPQ_PWM_MAX_DIV;
>> + best_pwm_div = IPQ_PWM_MAX_DIV;
>> + /* Initial pre_div value such that pwm_div < IPQ_PWM_MAX_DIV */
Note this comment.
>> + pre_div = div64_u64(period_rate,
>> + (u64)NSEC_PER_SEC * (IPQ_PWM_MAX_DIV + 1));
>
> Hmmm, we want
>
> (pre_div + 1) * (pwm_div + 1) * NSEC_PER_SEC
> -------------------------------------------- <= period_ns
> rate
>
> , right? Resolving that for pre_div this gives:
>
> period_ns * rate
> pre_div <= ----------------------------
> NSEC_PER_SEC * (pwm_div + 1)
>
> The term on the right hand side is maximal for pwm_div == 0 so the
> possible values for pre_div are
>
> 0 ... min(div64_u64(period_rate / NSEC_PER_SEC), IPQ_PWM_MAX_DIV)
>
> isn't it?
I don't think so. pre_div == 0 will produce pwm_div larger than
IPQ_PWM_MAX_DIV for a large period_rate value. The initial pre_div here is the
smallest value that produces pwm_div within it limit.
> If so, your algorithm is wrong as you're iterating over
>
> div64_u64(period_rate, NSEC_PER_SEC * (IPQ_PWM_MAX_DIV + 1)) ... IPQ_PWM_MAX_DIV
The loop stops when pre_div > pwm_div. That should be before pre_div ==
IPQ_PWM_MAX_DIV because pwm_div <= IPQ_PWM_MAX_DIV. Should I put the pre_div >
pwm_div condition directly in the for statement?
>> + min_diff = period_rate;
>> +
>> + for (; pre_div <= IPQ_PWM_MAX_DIV; pre_div++) {
>> + long long diff;
>> +
>> + pwm_div = DIV64_U64_ROUND_UP(period_rate,
>> + (u64)NSEC_PER_SEC * (pre_div + 1));
>> + /* pwm_div is unsigned; the check below catches underflow */
>> + pwm_div--;
>
> What underflow? DIV64_U64_ROUND_UP returns > 0 assuming period_rate > 0.
> So pwm_div - 1 doesn't underflow?!
I'll update the comment.
> The task here is to calculate the biggest pwm_div for a given pre_div
> such that
>
>
> (pre_div + 1) * (pwm_div + 1) * NSEC_PER_SEC
> -------------------------------------------- <= period_ns
> rate
>
> right?
>
> This is equivalent to:
>
> period_ns * rate
> pre_div <= ---------------------------- - 1
> (pre_div + 1) * NSEC_PER_SEC
>
> As pre_div is integer, rounding down should be fine?!
I can't follow. With round down (as in v8) the result is always:
NSEC_PER_SEC * (pre_div + 1) * (pwm_div + 1) <= period_rate
As a result, 'diff' calculation below will always produce diff <= 0. When
there is no diff == 0 result (bingo) we get IPQ_PWM_MAX_DIV in both best_
values at the end of the loop.
Do we actually need diff > 0 in the condition below?
>> + /*
>> + * pre_div and pwm_div values swap produces the same
>> + * result. This loop goes over all pre_div <= pwm_div
>> + * combinations. The rest are equivalent.
>> + */
>
> I'd write:
>
> /*
> * Swapping values for pre_div and pwm_div produces the same
> * period length. So we can skip all settings with pre_div <
> * pwm_div which results in bigger constraints for selecting the
> * duty_cycle than with the two values swapped.
> */
I'll take your wording with inverted inequality sign.
Thanks,
baruch
>> + if (pre_div > pwm_div)
>> + break;
>> +
>> + /*
>> + * Make sure we can do 100% duty cycle where
>> + * hi_dur == pwm_div + 1
>> + */
>> + if (pwm_div > IPQ_PWM_MAX_DIV - 1)
>> + continue;
>> +
>> + diff = ((uint64_t)NSEC_PER_SEC * (pre_div + 1) * (pwm_div + 1))
>> + - period_rate;
>> +
>> + if (diff < 0) /* period larger than requested */
>> + continue;
>
> This shouldn't happen if the above calculation is correct.
>
>> + if (diff == 0) { /* bingo */
>> + best_pre_div = pre_div;
>> + best_pwm_div = pwm_div;
>> + break;
>> + }
>> + if (diff < min_diff) {
>> + min_diff = diff;
>> + best_pre_div = pre_div;
>> + best_pwm_div = pwm_div;
>> + }
>
> This can be simplified as:
>
> if (diff < min_diff) {
> best_pre_div = pre_div;
> best_pwm_div = pwm_div;
> min_diff = diff;
>
> if (min_diff == 0)
> /* bingo! */
> break;
> }
>
>> + }
>> +
>> + /* config divider values for the closest possible frequency */
>> + config_div_and_duty(pwm, best_pre_div, best_pwm_div,
>> + rate, duty_ns, state->enabled);
>> +
>> + return 0;
>> +}
>> +
>> +static void ipq_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
>> + struct pwm_state *state)
>> +{
>> + struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(chip);
>> + unsigned long rate = clk_get_rate(ipq_chip->clk);
>> + unsigned int pre_div, pwm_div, hi_dur;
>> + u64 effective_div, hi_div;
>> + u32 reg0, reg1;
>> +
>> + reg0 = ipq_pwm_reg_read(pwm, IPQ_PWM_REG0);
>> + reg1 = ipq_pwm_reg_read(pwm, IPQ_PWM_REG1);
>> +
>> + state->polarity = PWM_POLARITY_NORMAL;
>> + state->enabled = reg1 & IPQ_PWM_REG1_ENABLE;
>> +
>> + pwm_div = FIELD_GET(IPQ_PWM_REG0_PWM_DIV, reg0);
>> + hi_dur = FIELD_GET(IPQ_PWM_REG0_HI_DURATION, reg0);
>> + pre_div = FIELD_GET(IPQ_PWM_REG1_PRE_DIV, reg1);
>> +
>> + /* No overflow here, both pre_div and pwm_div <= 0xffff */
>> + effective_div = (u64)(pre_div + 1) * (pwm_div + 1);
>> + state->period = DIV64_U64_ROUND_UP(effective_div * NSEC_PER_SEC, rate);
>> +
>> + hi_div = hi_dur * (pre_div + 1);
>> + state->duty_cycle = div64_u64(hi_div * NSEC_PER_SEC, rate);
>
> This must be round up for the same reasons as for period.
>
>> +}
>
> Best regards
> Uwe
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: mediatek: mt8186: Add binding for MM iommu
From: Krzysztof Kozlowski @ 2022-01-25 13:19 UTC (permalink / raw)
To: Yong Wu, Joerg Roedel, Rob Herring, Matthias Brugger, Will Deacon
Cc: Robin Murphy, Tomasz Figa, linux-mediatek, srv_heupstream,
devicetree, linux-kernel, linux-arm-kernel, iommu, Hsin-Yi Wang,
youlin.pei, anan.sun, xueqi.zhang, yen-chang.chen,
AngeloGioacchino Del Regno, mingyuan.ma, yf.wang, libo.kang,
chengci.xu
In-Reply-To: <20220125093244.18230-2-yong.wu@mediatek.com>
On 25/01/2022 10:32, Yong Wu wrote:
> Add mt8186 iommu binding. "-mm" means the iommu is for Multimedia.
>
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
> .../bindings/iommu/mediatek,iommu.yaml | 4 +
> .../dt-bindings/memory/mt8186-memory-port.h | 217 ++++++++++++++++++
> 2 files changed, 221 insertions(+)
> create mode 100644 include/dt-bindings/memory/mt8186-memory-port.h
>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH 2/2] pinctrl: ocelot: Add support for ServalT SoC
From: Horatiu Vultur @ 2022-01-25 13:18 UTC (permalink / raw)
To: linux-gpio, devicetree, linux-kernel
Cc: linus.walleij, robh+dt, Horatiu Vultur
In-Reply-To: <20220125131858.309237-1-horatiu.vultur@microchip.com>
This patch adds support for ServalT pinctrl, using the ocelot driver as
basis.
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
drivers/pinctrl/pinctrl-ocelot.c | 102 +++++++++++++++++++++++++++++++
1 file changed, 102 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c
index fc969208d904..685c79e08d40 100644
--- a/drivers/pinctrl/pinctrl-ocelot.c
+++ b/drivers/pinctrl/pinctrl-ocelot.c
@@ -695,6 +695,98 @@ static const struct pinctrl_pin_desc jaguar2_pins[] = {
JAGUAR2_PIN(63),
};
+#define SERVALT_P(p, f0, f1, f2) \
+static struct ocelot_pin_caps servalt_pin_##p = { \
+ .pin = p, \
+ .functions = { \
+ FUNC_GPIO, FUNC_##f0, FUNC_##f1, FUNC_##f2 \
+ }, \
+}
+
+SERVALT_P(0, SG0, NONE, NONE);
+SERVALT_P(1, SG0, NONE, NONE);
+SERVALT_P(2, SG0, NONE, NONE);
+SERVALT_P(3, SG0, NONE, NONE);
+SERVALT_P(4, IRQ0_IN, IRQ0_OUT, TWI_SCL_M);
+SERVALT_P(5, IRQ1_IN, IRQ1_OUT, TWI_SCL_M);
+SERVALT_P(6, UART, NONE, NONE);
+SERVALT_P(7, UART, NONE, NONE);
+SERVALT_P(8, SI, SFP, TWI_SCL_M);
+SERVALT_P(9, PCI_WAKE, SFP, SI);
+SERVALT_P(10, PTP0, SFP, TWI_SCL_M);
+SERVALT_P(11, PTP1, SFP, TWI_SCL_M);
+SERVALT_P(12, REF_CLK, SFP, TWI_SCL_M);
+SERVALT_P(13, REF_CLK, SFP, TWI_SCL_M);
+SERVALT_P(14, REF_CLK, IRQ0_OUT, SI);
+SERVALT_P(15, REF_CLK, IRQ1_OUT, SI);
+SERVALT_P(16, TACHO, SFP, SI);
+SERVALT_P(17, PWM, NONE, TWI_SCL_M);
+SERVALT_P(18, PTP2, SFP, SI);
+SERVALT_P(19, PTP3, SFP, SI);
+SERVALT_P(20, UART2, SFP, SI);
+SERVALT_P(21, UART2, NONE, NONE);
+SERVALT_P(22, MIIM, SFP, TWI2);
+SERVALT_P(23, MIIM, SFP, TWI2);
+SERVALT_P(24, TWI, NONE, NONE);
+SERVALT_P(25, TWI, SFP, TWI_SCL_M);
+SERVALT_P(26, TWI_SCL_M, SFP, SI);
+SERVALT_P(27, TWI_SCL_M, SFP, SI);
+SERVALT_P(28, TWI_SCL_M, SFP, SI);
+SERVALT_P(29, TWI_SCL_M, NONE, NONE);
+SERVALT_P(30, TWI_SCL_M, NONE, NONE);
+SERVALT_P(31, TWI_SCL_M, NONE, NONE);
+SERVALT_P(32, TWI_SCL_M, NONE, NONE);
+SERVALT_P(33, RCVRD_CLK, NONE, NONE);
+SERVALT_P(34, RCVRD_CLK, NONE, NONE);
+SERVALT_P(35, RCVRD_CLK, NONE, NONE);
+SERVALT_P(36, RCVRD_CLK, NONE, NONE);
+
+#define SERVALT_PIN(n) { \
+ .number = n, \
+ .name = "GPIO_"#n, \
+ .drv_data = &servalt_pin_##n \
+}
+
+static const struct pinctrl_pin_desc servalt_pins[] = {
+ SERVALT_PIN(0),
+ SERVALT_PIN(1),
+ SERVALT_PIN(2),
+ SERVALT_PIN(3),
+ SERVALT_PIN(4),
+ SERVALT_PIN(5),
+ SERVALT_PIN(6),
+ SERVALT_PIN(7),
+ SERVALT_PIN(8),
+ SERVALT_PIN(9),
+ SERVALT_PIN(10),
+ SERVALT_PIN(11),
+ SERVALT_PIN(12),
+ SERVALT_PIN(13),
+ SERVALT_PIN(14),
+ SERVALT_PIN(15),
+ SERVALT_PIN(16),
+ SERVALT_PIN(17),
+ SERVALT_PIN(18),
+ SERVALT_PIN(19),
+ SERVALT_PIN(20),
+ SERVALT_PIN(21),
+ SERVALT_PIN(22),
+ SERVALT_PIN(23),
+ SERVALT_PIN(24),
+ SERVALT_PIN(25),
+ SERVALT_PIN(26),
+ SERVALT_PIN(27),
+ SERVALT_PIN(28),
+ SERVALT_PIN(29),
+ SERVALT_PIN(30),
+ SERVALT_PIN(31),
+ SERVALT_PIN(32),
+ SERVALT_PIN(33),
+ SERVALT_PIN(34),
+ SERVALT_PIN(35),
+ SERVALT_PIN(36),
+};
+
#define SPARX5_P(p, f0, f1, f2) \
static struct ocelot_pin_caps sparx5_pin_##p = { \
.pin = p, \
@@ -1497,6 +1589,15 @@ static struct pinctrl_desc jaguar2_desc = {
.owner = THIS_MODULE,
};
+static struct pinctrl_desc servalt_desc = {
+ .name = "servalt-pinctrl",
+ .pins = servalt_pins,
+ .npins = ARRAY_SIZE(servalt_pins),
+ .pctlops = &ocelot_pctl_ops,
+ .pmxops = &ocelot_pmx_ops,
+ .owner = THIS_MODULE,
+};
+
static struct pinctrl_desc sparx5_desc = {
.name = "sparx5-pinctrl",
.pins = sparx5_pins,
@@ -1774,6 +1875,7 @@ static const struct of_device_id ocelot_pinctrl_of_match[] = {
{ .compatible = "mscc,serval-pinctrl", .data = &serval_desc },
{ .compatible = "mscc,ocelot-pinctrl", .data = &ocelot_desc },
{ .compatible = "mscc,jaguar2-pinctrl", .data = &jaguar2_desc },
+ { .compatible = "mscc,servalt-pinctrl", .data = &servalt_desc },
{ .compatible = "microchip,sparx5-pinctrl", .data = &sparx5_desc },
{ .compatible = "microchip,lan966x-pinctrl", .data = &lan966x_desc },
{},
--
2.33.0
^ permalink raw reply related
* [PATCH 0/2] pinctrl: ocelot Add support for ServalT
From: Horatiu Vultur @ 2022-01-25 13:18 UTC (permalink / raw)
To: linux-gpio, devicetree, linux-kernel
Cc: linus.walleij, robh+dt, Horatiu Vultur
This patch series adds support for ServalT pinctrl.
Horatiu Vultur (2):
dt-bindings: pinctrl: ocelot: Add ServalT SoC support
pinctrl: ocelot: Add support for ServalT SoC
.../bindings/pinctrl/mscc,ocelot-pinctrl.txt | 4 +-
drivers/pinctrl/pinctrl-ocelot.c | 102 ++++++++++++++++++
2 files changed, 104 insertions(+), 2 deletions(-)
--
2.33.0
^ permalink raw reply
* [PATCH 1/2] dt-bindings: pinctrl: ocelot: Add ServalT SoC support
From: Horatiu Vultur @ 2022-01-25 13:18 UTC (permalink / raw)
To: linux-gpio, devicetree, linux-kernel
Cc: linus.walleij, robh+dt, Horatiu Vultur
In-Reply-To: <20220125131858.309237-1-horatiu.vultur@microchip.com>
Add the documentation for the Microsemi ServalT pinmuxing and gpio
controller.
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
.../devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.txt
index 3bb76487669f..5d84fd299ccf 100644
--- a/Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.txt
@@ -4,8 +4,8 @@ Microsemi Ocelot pin controller Device Tree Bindings
Required properties:
- compatible : Should be "mscc,ocelot-pinctrl",
"mscc,jaguar2-pinctrl", "microchip,sparx5-pinctrl",
- "mscc,luton-pinctrl", "mscc,serval-pinctrl" or
- "microchip,lan966x-pinctrl"
+ "mscc,luton-pinctrl", "mscc,serval-pinctrl",
+ "microchip,lan966x-pinctrl" or "mscc,servalt-pinctrl"
- reg : Address and length of the register set for the device
- gpio-controller : Indicates this device is a GPIO controller
- #gpio-cells : Must be 2.
--
2.33.0
^ permalink raw reply related
* [PATCH 2/2] arm64: dts: ti: k3-j721s2-common-proc-board: Enable PCIe
From: Aswath Govindraju @ 2022-01-25 13:12 UTC (permalink / raw)
Cc: linux-kernel, devicetree, Rob Herring, Tero Kristo,
Vignesh Raghavendra, Nishanth Menon, Kishon Vijay Abraham I,
Aswath Govindraju
In-Reply-To: <20220125131225.871-1-a-govindraju@ti.com>
x1 lane PCIe slot in the common processor board is enabled and connected to
J721S2 SOM. Add PCIe DT node in common processor board to reflect the
same.
Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
---
.../boot/dts/ti/k3-j721s2-common-proc-board.dts | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
index cb99a97af426..793ee77838f4 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
@@ -428,6 +428,20 @@
};
};
+&pcie1_rc {
+ reset-gpios = <&exp1 2 GPIO_ACTIVE_HIGH>;
+ phys = <&serdes0_pcie_link>;
+ phy-names = "pcie-phy";
+ num-lanes = <1>;
+};
+
+&pcie1_ep {
+ phys = <&serdes0_pcie_link>;
+ phy-names = "pcie-phy";
+ num-lanes = <1>;
+ status = "disabled";
+};
+
&mcu_mcan0 {
pinctrl-names = "default";
pinctrl-0 = <&mcu_mcan0_pins_default>;
--
2.17.1
^ permalink raw reply related
* [PATCH 1/2] arm64: dts: ti: k3-j721s2-main: Add PCIe device tree node
From: Aswath Govindraju @ 2022-01-25 13:12 UTC (permalink / raw)
Cc: linux-kernel, devicetree, Rob Herring, Tero Kristo,
Vignesh Raghavendra, Nishanth Menon, Kishon Vijay Abraham I,
Aswath Govindraju
In-Reply-To: <20220125131225.871-1-a-govindraju@ti.com>
Add PCIe device tree node (both RC and EP) for the single PCIe
instance present in j721s2.
Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
---
arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi | 48 ++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
index ebd55032e59c..dc365a1880d0 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
@@ -795,6 +795,54 @@
};
};
+ pcie1_rc: pcie@2910000 {
+ compatible = "ti,j7200-pcie-host", "ti,j721e-pcie-host";
+ reg = <0x00 0x02910000 0x00 0x1000>,
+ <0x00 0x02917000 0x00 0x400>,
+ <0x00 0x0d800000 0x00 0x00800000>,
+ <0x00 0x18000000 0x00 0x00001000>;
+ reg-names = "intd_cfg", "user_cfg", "reg", "cfg";
+ interrupt-names = "link_state";
+ interrupts = <GIC_SPI 330 IRQ_TYPE_EDGE_RISING>;
+ device_type = "pci";
+ ti,syscon-pcie-ctrl = <&scm_conf 0x074>;
+ max-link-speed = <3>;
+ num-lanes = <4>;
+ power-domains = <&k3_pds 276 TI_SCI_PD_EXCLUSIVE>;
+ clocks = <&k3_clks 276 41>;
+ clock-names = "fck";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ bus-range = <0x0 0xff>;
+ vendor-id = <0x104c>;
+ device-id = <0xb013>;
+ msi-map = <0x0 &gic_its 0x0 0x10000>;
+ dma-coherent;
+ ranges = <0x01000000 0x0 0x18001000 0x00 0x18001000 0x0 0x0010000>,
+ <0x02000000 0x0 0x18011000 0x00 0x18011000 0x0 0x7fef000>;
+ dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x10000 0x0>;
+ };
+
+ pcie1_ep: pcie-ep@2910000 {
+ compatible = "ti,j7200-pcie-ep", "ti,j721e-pcie-ep";
+ reg = <0x00 0x02910000 0x00 0x1000>,
+ <0x00 0x02917000 0x00 0x400>,
+ <0x00 0x0d800000 0x00 0x00800000>,
+ <0x00 0x18000000 0x00 0x08000000>;
+ reg-names = "intd_cfg", "user_cfg", "reg", "mem";
+ interrupt-names = "link_state";
+ interrupts = <GIC_SPI 330 IRQ_TYPE_EDGE_RISING>;
+ ti,syscon-pcie-ctrl = <&scm_conf 0x074>;
+ max-link-speed = <3>;
+ num-lanes = <4>;
+ power-domains = <&k3_pds 276 TI_SCI_PD_EXCLUSIVE>;
+ clocks = <&k3_clks 276 41>;
+ clock-names = "fck";
+ max-functions = /bits/ 8 <6>;
+ max-virtual-functions = /bits/ 8 <4 4 4 4 0 0>;
+ dma-coherent;
+ };
+
main_mcan0: can@2701000 {
compatible = "bosch,m_can";
reg = <0x00 0x02701000 0x00 0x200>,
--
2.17.1
^ permalink raw reply related
* [PATCH 0/2] J721S2: Add support for PCIE
From: Aswath Govindraju @ 2022-01-25 13:12 UTC (permalink / raw)
Cc: linux-kernel, devicetree, Rob Herring, Tero Kristo,
Vignesh Raghavendra, Nishanth Menon, Kishon Vijay Abraham I,
Aswath Govindraju
The following series of patches add support for single
instance of PCIe brought out on J721S2 common processor
board.
Notes:
- Applying this patch series **breaks the boot** of J721S2.
This is because of the following commit,
"19e863828acf PCI: j721e: Drop redundant struct device *"
Dicussions are currently ongoing regarding the required
fix.
- This needs to be merged after the following patch
to avoid dtbs_check errors
https://lkml.org/lkml/2021/11/29/1752
Aswath Govindraju (2):
arm64: dts: ti: k3-j721s2-main: Add PCIe device tree node
arm64: dts: ti: k3-j721s2-common-proc-board: Enable PCIe
.../dts/ti/k3-j721s2-common-proc-board.dts | 14 ++++++
arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi | 48 +++++++++++++++++++
2 files changed, 62 insertions(+)
--
2.17.1
^ permalink raw reply
* [PATCH 6/6] arm64: dts: k3-j721s2: Add support for OSPI Flashes
From: Aswath Govindraju @ 2022-01-25 13:11 UTC (permalink / raw)
Cc: linux-kernel, devicetree, Rob Herring, Tero Kristo,
Vignesh Raghavendra, Nishanth Menon, Aswath Govindraju
In-Reply-To: <20220125131113.727-1-a-govindraju@ti.com>
J721S2 has an OSPI NOR flash on its SOM connected the OSPI0 instance and a
QSPI NOR flash on the common processor board connected to the OSPI1
instance. Add support for the same
Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
---
.../dts/ti/k3-j721s2-common-proc-board.dts | 34 +++++++++++++++
arch/arm64/boot/dts/ti/k3-j721s2-som-p0.dtsi | 42 +++++++++++++++++++
2 files changed, 76 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
index aa75dc541842..cb99a97af426 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
@@ -206,6 +206,20 @@
J721S2_WKUP_IOPAD(0x0c8, PIN_INPUT, 7) /* (C28) WKUP_GPIO0_2 */
>;
};
+
+ mcu_fss0_ospi1_pins_default: mcu-fss0-ospi1-pins-default {
+ pinctrl-single,pins = <
+ J721S2_WKUP_IOPAD(0x040, PIN_OUTPUT, 0) /* (A19) MCU_OSPI1_CLK */
+ J721S2_WKUP_IOPAD(0x05c, PIN_OUTPUT, 0) /* (D20) MCU_OSPI1_CSn0 */
+ J721S2_WKUP_IOPAD(0x060, PIN_OUTPUT, 0) /* (C21) MCU_OSPI1_CSn1 */
+ J721S2_WKUP_IOPAD(0x04c, PIN_INPUT, 0) /* (D21) MCU_OSPI1_D0 */
+ J721S2_WKUP_IOPAD(0x050, PIN_INPUT, 0) /* (G20) MCU_OSPI1_D1 */
+ J721S2_WKUP_IOPAD(0x054, PIN_INPUT, 0) /* (C20) MCU_OSPI1_D2 */
+ J721S2_WKUP_IOPAD(0x058, PIN_INPUT, 0) /* (A20) MCU_OSPI1_D3 */
+ J721S2_WKUP_IOPAD(0x048, PIN_INPUT, 0) /* (B19) MCU_OSPI1_DQS */
+ J721S2_WKUP_IOPAD(0x044, PIN_INPUT, 0) /* (B20) MCU_OSPI1_LBCLKO */
+ >;
+ };
};
&main_gpio2 {
@@ -394,6 +408,26 @@
maximum-speed = "high-speed";
};
+&ospi1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mcu_fss0_ospi1_pins_default>;
+
+ flash@0{
+ compatible = "jedec,spi-nor";
+ reg = <0x0>;
+ spi-tx-bus-width = <1>;
+ spi-rx-bus-width = <4>;
+ spi-max-frequency = <40000000>;
+ cdns,tshsl-ns = <60>;
+ cdns,tsd2d-ns = <60>;
+ cdns,tchsh-ns = <60>;
+ cdns,tslch-ns = <60>;
+ cdns,read-delay = <2>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ };
+};
+
&mcu_mcan0 {
pinctrl-names = "default";
pinctrl-0 = <&mcu_mcan0_pins_default>;
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-som-p0.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-som-p0.dtsi
index 76f0ceacb6d4..a05c17dd69b6 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-som-p0.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-som-p0.dtsi
@@ -39,6 +39,28 @@
};
};
+&wkup_pmx0 {
+ mcu_fss0_ospi0_pins_default: mcu-fss0-ospi0-pins-default {
+ pinctrl-single,pins = <
+ J721S2_WKUP_IOPAD(0x000, PIN_OUTPUT, 0) /* (D19) MCU_OSPI0_CLK */
+ J721S2_WKUP_IOPAD(0x02c, PIN_OUTPUT, 0) /* (F15) MCU_OSPI0_CSn0 */
+ J721S2_WKUP_IOPAD(0x030, PIN_OUTPUT, 0) /* (G17) MCU_OSPI0_CSn1 */
+ J721S2_WKUP_IOPAD(0x038, PIN_OUTPUT, 0) /* (F14) MCU_OSPI0_CSn2 */
+ J721S2_WKUP_IOPAD(0x03c, PIN_OUTPUT, 0) /* (F17) MCU_OSPI0_CSn3 */
+ J721S2_WKUP_IOPAD(0x00c, PIN_INPUT, 0) /* (C19) MCU_OSPI0_D0 */
+ J721S2_WKUP_IOPAD(0x010, PIN_INPUT, 0) /* (F16) MCU_OSPI0_D1 */
+ J721S2_WKUP_IOPAD(0x014, PIN_INPUT, 0) /* (G15) MCU_OSPI0_D2 */
+ J721S2_WKUP_IOPAD(0x018, PIN_INPUT, 0) /* (F18) MCU_OSPI0_D3 */
+ J721S2_WKUP_IOPAD(0x01c, PIN_INPUT, 0) /* (E19) MCU_OSPI0_D4 */
+ J721S2_WKUP_IOPAD(0x020, PIN_INPUT, 0) /* (G19) MCU_OSPI0_D5 */
+ J721S2_WKUP_IOPAD(0x024, PIN_INPUT, 0) /* (F19) MCU_OSPI0_D6 */
+ J721S2_WKUP_IOPAD(0x028, PIN_INPUT, 0) /* (F20) MCU_OSPI0_D7 */
+ J721S2_WKUP_IOPAD(0x008, PIN_INPUT, 0) /* (E18) MCU_OSPI0_DQS */
+ J721S2_WKUP_IOPAD(0x004, PIN_INPUT, 0) /* (E20) MCU_OSPI0_LBCLKO */
+ >;
+ };
+};
+
&main_pmx0 {
main_i2c0_pins_default: main-i2c0-pins-default {
pinctrl-single,pins = <
@@ -78,6 +100,26 @@
phys = <&transceiver0>;
};
+&ospi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mcu_fss0_ospi0_pins_default>;
+
+ flash@0 {
+ compatible = "jedec,spi-nor";
+ reg = <0x0>;
+ spi-tx-bus-width = <8>;
+ spi-rx-bus-width = <8>;
+ spi-max-frequency = <25000000>;
+ cdns,tshsl-ns = <60>;
+ cdns,tsd2d-ns = <60>;
+ cdns,tchsh-ns = <60>;
+ cdns,tslch-ns = <60>;
+ cdns,read-delay = <4>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ };
+};
+
&mailbox0_cluster0 {
status = "disabled";
};
--
2.17.1
^ permalink raw reply related
* [PATCH 4/6] arm64: dts: ti: k3-j721s2-common-proc-board: Enable SERDES0
From: Aswath Govindraju @ 2022-01-25 13:11 UTC (permalink / raw)
Cc: linux-kernel, devicetree, Rob Herring, Tero Kristo,
Vignesh Raghavendra, Nishanth Menon, Aswath Govindraju
In-Reply-To: <20220125131113.727-1-a-govindraju@ti.com>
Configure first lane to PCIe, the second lane to USB and the last two lanes
to eDP. Also, add sub-nodes to SERDES0 DT node to represent SERDES0 is
connected to PCIe.
Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
---
.../dts/ti/k3-j721s2-common-proc-board.dts | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
index b210cc07c539..791f235bd95f 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
@@ -9,6 +9,9 @@
#include "k3-j721s2-som-p0.dtsi"
#include <dt-bindings/net/ti-dp83867.h>
+#include <dt-bindings/phy/phy-cadence.h>
+#include <dt-bindings/phy/phy.h>
+#include <dt-bindings/mux/ti-serdes.h>
/ {
compatible = "ti,j721s2-evm", "ti,j721s2";
@@ -350,6 +353,25 @@
phy-handle = <&phy0>;
};
+&serdes_ln_ctrl {
+ idle-states = <J721S2_SERDES0_LANE0_PCIE1_LANE0>, <J721S2_SERDES0_LANE1_USB>,
+ <J721S2_SERDES0_LANE2_EDP_LANE2>, <J721S2_SERDES0_LANE3_EDP_LANE3>;
+};
+
+&serdes_refclk {
+ clock-frequency = <100000000>;
+};
+
+&serdes0 {
+ serdes0_pcie_link: phy@0 {
+ reg = <0>;
+ cdns,num-lanes = <1>;
+ #phy-cells = <0>;
+ cdns,phy-type = <PHY_TYPE_PCIE>;
+ resets = <&serdes_wiz0 1>;
+ };
+};
+
&mcu_mcan0 {
pinctrl-names = "default";
pinctrl-0 = <&mcu_mcan0_pins_default>;
--
2.17.1
^ permalink raw reply related
* [PATCH 5/6] arm64: dts: ti: k3-j721s2-common-proc-board: Add USB support
From: Aswath Govindraju @ 2022-01-25 13:11 UTC (permalink / raw)
Cc: linux-kernel, devicetree, Rob Herring, Tero Kristo,
Vignesh Raghavendra, Nishanth Menon, Aswath Govindraju
In-Reply-To: <20220125131113.727-1-a-govindraju@ti.com>
The board uses lane 1 of SERDES for USB. Set the mux
accordingly.
The USB controller and EVM supports super-speed for USB0
on the Type-C port. However, the SERDES has a limitation
that upto 2 protocols can be used at a time. The SERDES is
wired for PCIe, eDP and USB super-speed. It has been
chosen to use PCIe and eDP as default. So restrict
USB0 to high-speed mode.
Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
---
.../dts/ti/k3-j721s2-common-proc-board.dts | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
index 791f235bd95f..aa75dc541842 100644
--- a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
@@ -147,6 +147,12 @@
J721S2_IOPAD(0x020, PIN_INPUT, 7) /* (AA23) MCAN15_RX.GPIO0_8 */
>;
};
+
+ main_usbss0_pins_default: main-usbss0-pins-default {
+ pinctrl-single,pins = <
+ J721S2_IOPAD(0x0ec, PIN_OUTPUT, 6) /* (AG25) TIMER_IO1.USB0_DRVVBUS */
+ >;
+ };
};
&wkup_pmx0 {
@@ -372,6 +378,22 @@
};
};
+&usb_serdes_mux {
+ idle-states = <1>; /* USB0 to SERDES lane 1 */
+};
+
+&usbss0 {
+ pinctrl-0 = <&main_usbss0_pins_default>;
+ pinctrl-names = "default";
+ ti,vbus-divider;
+ ti,usb2-only;
+};
+
+&usb0 {
+ dr_mode = "otg";
+ maximum-speed = "high-speed";
+};
+
&mcu_mcan0 {
pinctrl-names = "default";
pinctrl-0 = <&mcu_mcan0_pins_default>;
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox