* [PATCH 2/6] powerpc/time: Make mftb() common to PPC32 and PPC64
From: Christophe Leroy @ 2020-10-01 12:42 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <94dc68d3d9ef9eb549796d4b938b6ba0305a049b.1601556145.git.christophe.leroy@csgroup.eu>
No need to have two versions that are identical.
CONFIG_PPC_CELL is only selected by PPC64 targets.
CONFIG_E500 is the only PPC64 target selecting CONFIG_FSL_BOOK3E.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/reg.h | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index c66dcdb47c44..f877a576b338 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1419,8 +1419,7 @@ static inline void msr_check_and_clear(unsigned long bits)
__msr_check_and_clear(bits);
}
-#ifdef __powerpc64__
-#if defined(CONFIG_PPC_CELL) || defined(CONFIG_PPC_FSL_BOOK3E)
+#if defined(CONFIG_PPC_CELL) || defined(CONFIG_E500)
#define mftb() ({unsigned long rval; \
asm volatile( \
"90: mfspr %0, %2;\n" \
@@ -1430,28 +1429,23 @@ static inline void msr_check_and_clear(unsigned long bits)
: "=r" (rval) \
: "i" (CPU_FTR_CELL_TB_BUG), "i" (SPRN_TBRL) : "cr0"); \
rval;})
+#elif defined(CONFIG_PPC_8xx)
+#define mftb() ({unsigned long rval; \
+ asm volatile("mftbl %0" : "=r" (rval)); rval;})
#else
#define mftb() ({unsigned long rval; \
asm volatile("mfspr %0, %1" : \
"=r" (rval) : "i" (SPRN_TBRL)); rval;})
#endif /* !CONFIG_PPC_CELL */
-#else /* __powerpc64__ */
-
#if defined(CONFIG_PPC_8xx)
-#define mftb() ({unsigned long rval; \
- asm volatile("mftbl %0" : "=r" (rval)); rval;})
#define mftbu() ({unsigned long rval; \
asm volatile("mftbu %0" : "=r" (rval)); rval;})
#else
-#define mftb() ({unsigned long rval; \
- asm volatile("mfspr %0, %1" : "=r" (rval) : \
- "i" (SPRN_TBRL)); rval;})
#define mftbu() ({unsigned long rval; \
asm volatile("mfspr %0, %1" : "=r" (rval) : \
"i" (SPRN_TBRU)); rval;})
#endif
-#endif /* !__powerpc64__ */
#define mttbl(v) asm volatile("mttbl %0":: "r"(v))
#define mttbu(v) asm volatile("mttbu %0":: "r"(v))
--
2.25.0
^ permalink raw reply related
* [PATCH] powerpc/time: Remove ifdef in get_dec() and set_dec()
From: Christophe Leroy @ 2020-10-01 10:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Move SPRN_PIT definition in reg.h.
This allows to remove ifdef in get_dec() and set_dec() and
makes them more readable.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/reg.h | 2 ++
arch/powerpc/include/asm/reg_booke.h | 1 -
arch/powerpc/include/asm/time.h | 23 ++++++++++-------------
3 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index d25c357a873c..788058af1d44 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -521,6 +521,8 @@
#define SPRN_TSCR 0x399 /* Thread Switch Control Register */
#define SPRN_DEC 0x016 /* Decrement Register */
+#define SPRN_PIT 0x3DB /* Programmable Interval Timer (40x/BOOKE) */
+
#define SPRN_DER 0x095 /* Debug Enable Register */
#define DER_RSTE 0x40000000 /* Reset Interrupt */
#define DER_CHSTPE 0x20000000 /* Check Stop */
diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index ff30f1076162..29a948e0c0f2 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -174,7 +174,6 @@
#define SPRN_L1CSR1 0x3F3 /* L1 Cache Control and Status Register 1 */
#define SPRN_MMUCSR0 0x3F4 /* MMU Control and Status Register 0 */
#define SPRN_MMUCFG 0x3F7 /* MMU Configuration Register */
-#define SPRN_PIT 0x3DB /* Programmable Interval Timer */
#define SPRN_BUCSR 0x3F5 /* Branch Unit Control and Status */
#define SPRN_L2CSR0 0x3F9 /* L2 Data Cache Control and Status Register 0 */
#define SPRN_L2CSR1 0x3FA /* L2 Data Cache Control and Status Register 1 */
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 6c663a382a75..a80abf64c8a5 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -126,11 +126,10 @@ static inline void set_tb(unsigned int upper, unsigned int lower)
*/
static inline u64 get_dec(void)
{
-#if defined(CONFIG_40x)
- return (mfspr(SPRN_PIT));
-#else
- return (mfspr(SPRN_DEC));
-#endif
+ if (IS_ENABLED(CONFIG_40x))
+ return mfspr(SPRN_PIT);
+
+ return mfspr(SPRN_DEC);
}
/*
@@ -140,14 +139,12 @@ static inline u64 get_dec(void)
*/
static inline void set_dec(u64 val)
{
-#if defined(CONFIG_40x)
- mtspr(SPRN_PIT, (u32) val);
-#else
-#ifndef CONFIG_BOOKE
- --val;
-#endif
- mtspr(SPRN_DEC, val);
-#endif /* not 40x */
+ if (IS_ENABLED(CONFIG_40x))
+ mtspr(SPRN_PIT, (u32)val);
+ else if (IS_ENABLED(CONFIG_BOOKE))
+ mtspr(SPRN_DEC, val);
+ else
+ mtspr(SPRN_DEC, val - 1);
}
static inline unsigned long tb_ticks_since(unsigned long tstamp)
--
2.25.0
^ permalink raw reply related
* [PATCH] powerpc/time: Remove ifdef in get_vtb()
From: Christophe Leroy @ 2020-10-01 10:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
SPRN_VTB and CPU_FTR_ARCH_207S are always defined,
no need of an ifdef.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/time.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index cb326720a8a1..6c663a382a75 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -80,10 +80,9 @@ static inline u64 get_rtc(void)
static inline u64 get_vtb(void)
{
-#ifdef CONFIG_PPC_BOOK3S_64
if (cpu_has_feature(CPU_FTR_ARCH_207S))
return mfspr(SPRN_VTB);
-#endif
+
return 0;
}
--
2.25.0
^ permalink raw reply related
* Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")
From: Miguel Ojeda @ 2020-10-01 10:15 UTC (permalink / raw)
To: Joe Perches
Cc: Kees Cook, Paul E . McKenney, Nick Desaulniers, Lai Jiangshan,
Josh Triplett, Steven Rostedt, LKML, rcu, Clang-Built-Linux ML,
Mathieu Desnoyers, Sedat Dilek, Paul Mackerras, linuxppc-dev
In-Reply-To: <46040e2776a4848add06126ce1cb8f846709294f.camel@perches.com>
Hi Joe,
On Thu, Oct 1, 2020 at 12:56 AM Joe Perches <joe@perches.com> wrote:
>
> So I installed the powerpc cross compiler, and
> nope, that doesn't work, it makes a mess.
Thanks a lot for reviving the script and sending the treewide cleanup!
> So it looks like the best option is to exclude these
> 2 files from conversion.
Agreed. Nevertheless, is there any reason arch/powerpc/* should not be
compiling cleanly with compiler.h? (CC'ing the rest of the PowerPC
reviewers and ML).
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH v2 14/14] powerpc/pseries/iommu: Rename "direct window" to "dma window"
From: Alexey Kardashevskiy @ 2020-09-30 7:29 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev
In-Reply-To: <1b813ab38869e2e6770ed09487a3fba7befaca86.camel@gmail.com>
On 30/09/2020 06:54, Leonardo Bras wrote:
> On Tue, 2020-09-29 at 13:55 +1000, Alexey Kardashevskiy wrote:
>>
>> On 12/09/2020 03:07, Leonardo Bras wrote:
>>> Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
>>>
>>> A previous change introduced the usage of DDW as a bigger indirect DMA
>>> mapping when the DDW available size does not map the whole partition.
>>>
>>> As most of the code that manipulates direct mappings was reused for
>>> indirect mappings, it's necessary to rename all names and debug/info
>>> messages to reflect that it can be used for both kinds of mapping.
>>>
>>> Also, defines DEFAULT_DMA_WIN as "ibm,dma-window" to document that
>>> it's the name of the default DMA window.
>>
>> "ibm,dma-window" is so old so it does not need a macro (which btw would
>> be DMA_WIN_PROPNAME to match the other names) :)
>
> Thanks for bringing that to my attention!
> In fact, DMA_WIN_PROPNAME makes more sense, but it's still generic and
> doesn't look to point to a generic one.
>
> Would that be ok to call it DEFAULT_WIN_PROPNAME ?
I would not touch it at all, the property name is painfully known and
not going to change ever. Does anyone else define it as a macro? I do
not see any:
[fstn1-p1 kernel-dma-bypass]$ git grep "ibm,dma-window" | wc -l
8
[fstn1-p1 kernel-dma-bypass]$ git grep "define.*ibm,dma-window" | wc -l
0
>
>
>>
>>
>>> Those changes are not supposed to change how the code works in any
>>> way, just adjust naming.
>>
>> I simply have this in my .vimrc for the cases like this one:
>>
>> ===
>> This should cause no behavioural change.
>> ===
>
> Great tip! I will make sure to have this saved here :)
>
> Thank you!
>
--
Alexey
^ permalink raw reply
* Re: [PATCH] ibmvfc: Protect vhost->task_set increment by the host lock
From: Martin K. Petersen @ 2020-09-30 3:34 UTC (permalink / raw)
To: linux-scsi, Brian King; +Cc: tyreld, linuxppc-dev, Martin K . Petersen
In-Reply-To: <1600286999-22059-1-git-send-email-brking@linux.vnet.ibm.com>
On Wed, 16 Sep 2020 15:09:59 -0500, Brian King wrote:
> In the discovery thread, ibmvfc does a vhost->task_set++ without
> any lock held. This could result in two targets getting the same
> cancel key, which could have strange effects in error recovery.
> The actual probability of this occurring should be extremely
> small, since this should all be done in a single threaded loop
> from the discovery thread, but let's fix it up anyway to be safe.
Applied to 5.10/scsi-queue, thanks!
[1/1] scsi: ibmvfc: Protect vhost->task_set increment by the host lock
https://git.kernel.org/mkp/scsi/c/2584e5aef87a
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH v2 4/7] powerpc: Remove PowerPC 601
From: Segher Boessenkool @ 2020-09-29 22:01 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <00a6948d659e017f8ca63437d1384222c3aede57.1601359702.git.christophe.leroy@csgroup.eu>
On Tue, Sep 29, 2020 at 06:09:21AM +0000, Christophe Leroy wrote:
> Powerpc 601 is 25 years old.
So is 603, but that one is still used! :-)
> It is not selected by any defconfig.
>
> It requires a lot of special handling as it deviates from the
> standard 6xx.
>
> Retire it.
That is fine with me of course. If I get a vote at all for this!
Thanks,
Segher
^ permalink raw reply
* Re: [PATCH v2 devicetree 2/2] powerpc: dts: t1040rdb: add ports for Seville Ethernet switch
From: Vladimir Oltean @ 2020-09-29 20:33 UTC (permalink / raw)
To: Andrew Lunn
Cc: devicetree@vger.kernel.org, Madalin Bucur (OSS),
Radu-andrei Bulie, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, fido_max@inbox.ru,
robh+dt@kernel.org, paulus@samba.org, Vladimir Oltean,
shawnguo@kernel.org, netdev@vger.kernel.org
In-Reply-To: <20200929201048.GG3996795@lunn.ch>
On Tue, Sep 29, 2020 at 10:10:48PM +0200, Andrew Lunn wrote:
> On Tue, Sep 29, 2020 at 07:39:54PM +0000, Vladimir Oltean wrote:
> > On Tue, Sep 29, 2020 at 09:11:53PM +0200, Andrew Lunn wrote:
> > > > +&seville_port0 {
> > > > + managed = "in-band-status";
> > > > + phy-handle = <&phy_qsgmii_0>;
> > > > + phy-mode = "qsgmii";
> > > > + /* ETH4 written on chassis */
> > > > + label = "swp4";
> > >
> > > If ETH4 is on the chassis why not use ETH4?
> >
> > You mean all-caps, just like that?
>
> Yes.
>
> DSA is often used in WiFI access point, etc. The user is not a
> computer professional. If the WebGUI says ETH4, and the label on the
> front says ETH4, they probably think the two are the same, and are
> happy.
>
> I have one box which does not have an labels on the front panels, but
> the industrial sockets for Ethernet are colour coded. So the interface
> names are red, blue, green, to match the socket colour, and the cable
> set is also colour coded the same.
>
> So long as it is unique, the kernel does not care. So make it easy for
> the user.
It would look like this:
[root@T1040 ~] # ip link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
link/ether de:91:41:1a:92:b8 brd ff:ff:ff:ff:ff:ff
3: fm1-gb3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:68 brd ff:ff:ff:ff:ff:ff
4: fm1-gb4: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc mq state DOWN mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:88 brd ff:ff:ff:ff:ff:ff
5: fm1-gb0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1504 qdisc mq state UP mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:08 brd ff:ff:ff:ff:ff:ff
6: fm1-gb1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:28 brd ff:ff:ff:ff:ff:ff
7: fm1-gb2: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc mq state DOWN mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:48 brd ff:ff:ff:ff:ff:ff
8: tunl0@NONE: <NOARP> mtu 1480 qdisc noop state DOWN mode DEFAULT group default qlen 1000
link/ipip 0.0.0.0 brd 0.0.0.0
9: sit0@NONE: <NOARP> mtu 1480 qdisc noop state DOWN mode DEFAULT group default qlen 1000
link/sit 0.0.0.0 brd 0.0.0.0
10: ETH4@fm1-gb0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:08 brd ff:ff:ff:ff:ff:ff
11: ETH5@fm1-gb0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:08 brd ff:ff:ff:ff:ff:ff
12: ETH6@fm1-gb0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:08 brd ff:ff:ff:ff:ff:ff
13: ETH7@fm1-gb0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:08 brd ff:ff:ff:ff:ff:ff
14: ETH8@fm1-gb0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:08 brd ff:ff:ff:ff:ff:ff
15: ETH9@fm1-gb0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:08 brd ff:ff:ff:ff:ff:ff
16: ETH10@fm1-gb0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:08 brd ff:ff:ff:ff:ff:ff
17: ETH11@fm1-gb0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
link/ether 00:1f:7b:6a:02:08 brd ff:ff:ff:ff:ff:ff
[root@T1040 ~] # ip link set ETH4 down
[ 94.942190] mscc_seville ffe800000.ethernet-switch ETH4: Link is Down
[root@T1040 ~] # ip link set ETH4 up
[ 100.262533] mscc_seville ffe800000.ethernet-switch ETH4: configuring for inband/qsgmii link mode
[ 100.272122] 8021q: adding VLAN 0 to HW filter on device ETH4
[ 103.333369] mscc_seville ffe800000.ethernet-switch ETH4: Link is Up - 1Gbps/Full - flow control rx/tx
[ 103.342697] IPv6: ADDRCONF(NETDEV_CHANGE): ETH4: link becomes ready
I'm not in love, but I guess at least there won't be any doubt if they
are named like this. I'm sending another revision with these names soon.
Thanks,
-Vladimir
^ permalink raw reply
* Re: [PATCH v2 devicetree 2/2] powerpc: dts: t1040rdb: add ports for Seville Ethernet switch
From: Vladimir Oltean @ 2020-09-29 19:39 UTC (permalink / raw)
To: Andrew Lunn
Cc: devicetree@vger.kernel.org, Madalin Bucur (OSS),
Radu-andrei Bulie, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, fido_max@inbox.ru,
robh+dt@kernel.org, paulus@samba.org, Vladimir Oltean,
shawnguo@kernel.org, netdev@vger.kernel.org
In-Reply-To: <20200929191153.GF3996795@lunn.ch>
On Tue, Sep 29, 2020 at 09:11:53PM +0200, Andrew Lunn wrote:
> > +&seville_port0 {
> > + managed = "in-band-status";
> > + phy-handle = <&phy_qsgmii_0>;
> > + phy-mode = "qsgmii";
> > + /* ETH4 written on chassis */
> > + label = "swp4";
>
> If ETH4 is on the chassis why not use ETH4?
You mean all-caps, just like that?
I don't know, I never saw an interface named in all-caps, it looks
strange to me. I understand that board designers are typically
case-insensitive, and that's kind of what my problem is, "eth4" is
clashing with the default naming scheme of the kernel and I also want to
avoid that. All in all, this is a reference design board, I don't care
too much. I've seen the "swp" convention being quite frequent, and I
thought that would be more intuitive. I've been using the same scheme
(the switch ports starting from swp2, corresponding to ETH2 on the
chassis) for the LS1021A-TSN board (arch/arm/boot/dts/ls1021a-tsn.dts)
and my users haven't complained about it.
Plus, it's not like the dpaa-eth (standalone) ports are named after the
chassis labels. Freescale/NXP typically ships an udev rule file that
names the interface after the associated FMan hardware port (for
example, the DSA master for the switch on this SoC is called "fm1-gb0",
and it's an internal port having nothing to do with ETH0, which is
"fm1-gb3").
I think it's a bit strange that the Rest Of World doesn't allow
interface naming via device tree, on this board the switch ports are not
where the big interface naming problem is. Although I'm not even sure
what to do to not increase it even more. With users being used to have
ETH0 going to fm1-gb3, maybe naming ETH4 as swp4 isn't the brightest
idea, true...
^ permalink raw reply
* Re: [PATCH v2 devicetree 0/2] Add Seville Ethernet switch to T1040RDB
From: Maxim Kochetkov @ 2020-09-29 13:11 UTC (permalink / raw)
To: Vladimir Oltean, robh+dt, shawnguo, mpe, devicetree
Cc: madalin.bucur, linux-kernel, radu-andrei.bulie, paulus, netdev,
linuxppc-dev
In-Reply-To: <20200929113209.3767787-1-vladimir.oltean@nxp.com>
Reviewed-by: Maxim Kochetkov <fido_max@inbox.ru>
29.09.2020 14:32, Vladimir Oltean пишет:
> Seville is a DSA switch that is embedded inside the T1040 SoC, and
> supported by the mscc_seville DSA driver inside drivers/net/dsa/ocelot.
>
> This series adds this switch to the SoC's dtsi files and to the T1040RDB
> board file.
>
> Vladimir Oltean (2):
> powerpc: dts: t1040: add bindings for Seville Ethernet switch
> powerpc: dts: t1040rdb: add ports for Seville Ethernet switch
>
> arch/powerpc/boot/dts/fsl/t1040rdb.dts | 115 ++++++++++++++++++++
> arch/powerpc/boot/dts/fsl/t1040si-post.dtsi | 76 +++++++++++++
> 2 files changed, 191 insertions(+)
>
^ permalink raw reply
* Re: [PATCH v2 devicetree 2/2] powerpc: dts: t1040rdb: add ports for Seville Ethernet switch
From: Maxim Kochetkov @ 2020-09-29 13:12 UTC (permalink / raw)
To: Vladimir Oltean, robh+dt, shawnguo, mpe, devicetree
Cc: madalin.bucur, linux-kernel, radu-andrei.bulie, paulus, netdev,
Vladimir Oltean, linuxppc-dev
In-Reply-To: <20200929113209.3767787-3-vladimir.oltean@nxp.com>
Reviewed-by: Maxim Kochetkov <fido_max@inbox.ru>
29.09.2020 14:32, Vladimir Oltean пишет:
> From: Vladimir Oltean <olteanv@gmail.com>
>
> Define the network interface names for the switch ports and hook them up
> to the 2 QSGMII PHYs that are onboard.
>
> A conscious decision was taken to go along with the numbers that are
> written on the front panel of the board and not with the hardware
> numbers of the switch chip ports. The 2 numbering schemes are
> shifted by 8.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> Changes in v2:
> Use the existing way of accessing the mdio bus and not labels.
>
> arch/powerpc/boot/dts/fsl/t1040rdb.dts | 115 +++++++++++++++++++++++++
> 1 file changed, 115 insertions(+)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t1040rdb.dts b/arch/powerpc/boot/dts/fsl/t1040rdb.dts
> index 65ff34c49025..3fd08a2b6dcb 100644
> --- a/arch/powerpc/boot/dts/fsl/t1040rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t1040rdb.dts
> @@ -64,6 +64,40 @@ mdio@fc000 {
> phy_sgmii_2: ethernet-phy@3 {
> reg = <0x03>;
> };
> +
> + /* VSC8514 QSGMII PHY */
> + phy_qsgmii_0: ethernet-phy@4 {
> + reg = <0x4>;
> + };
> +
> + phy_qsgmii_1: ethernet-phy@5 {
> + reg = <0x5>;
> + };
> +
> + phy_qsgmii_2: ethernet-phy@6 {
> + reg = <0x6>;
> + };
> +
> + phy_qsgmii_3: ethernet-phy@7 {
> + reg = <0x7>;
> + };
> +
> + /* VSC8514 QSGMII PHY */
> + phy_qsgmii_4: ethernet-phy@8 {
> + reg = <0x8>;
> + };
> +
> + phy_qsgmii_5: ethernet-phy@9 {
> + reg = <0x9>;
> + };
> +
> + phy_qsgmii_6: ethernet-phy@a {
> + reg = <0xa>;
> + };
> +
> + phy_qsgmii_7: ethernet-phy@b {
> + reg = <0xb>;
> + };
> };
> };
> };
> @@ -76,3 +110,84 @@ cpld@3,0 {
> };
>
> #include "t1040si-post.dtsi"
> +
> +&seville_switch {
> + status = "okay";
> +};
> +
> +&seville_port0 {
> + managed = "in-band-status";
> + phy-handle = <&phy_qsgmii_0>;
> + phy-mode = "qsgmii";
> + /* ETH4 written on chassis */
> + label = "swp4";
> + status = "okay";
> +};
> +
> +&seville_port1 {
> + managed = "in-band-status";
> + phy-handle = <&phy_qsgmii_1>;
> + phy-mode = "qsgmii";
> + /* ETH5 written on chassis */
> + label = "swp5";
> + status = "okay";
> +};
> +
> +&seville_port2 {
> + managed = "in-band-status";
> + phy-handle = <&phy_qsgmii_2>;
> + phy-mode = "qsgmii";
> + /* ETH6 written on chassis */
> + label = "swp6";
> + status = "okay";
> +};
> +
> +&seville_port3 {
> + managed = "in-band-status";
> + phy-handle = <&phy_qsgmii_3>;
> + phy-mode = "qsgmii";
> + /* ETH7 written on chassis */
> + label = "swp7";
> + status = "okay";
> +};
> +
> +&seville_port4 {
> + managed = "in-band-status";
> + phy-handle = <&phy_qsgmii_4>;
> + phy-mode = "qsgmii";
> + /* ETH8 written on chassis */
> + label = "swp8";
> + status = "okay";
> +};
> +
> +&seville_port5 {
> + managed = "in-band-status";
> + phy-handle = <&phy_qsgmii_5>;
> + phy-mode = "qsgmii";
> + /* ETH9 written on chassis */
> + label = "swp9";
> + status = "okay";
> +};
> +
> +&seville_port6 {
> + managed = "in-band-status";
> + phy-handle = <&phy_qsgmii_6>;
> + phy-mode = "qsgmii";
> + /* ETH10 written on chassis */
> + label = "swp10";
> + status = "okay";
> +};
> +
> +&seville_port7 {
> + managed = "in-band-status";
> + phy-handle = <&phy_qsgmii_7>;
> + phy-mode = "qsgmii";
> + /* ETH11 written on chassis */
> + label = "swp11";
> + status = "okay";
> +};
> +
> +&seville_port8 {
> + ethernet = <&enet0>;
> + status = "okay";
> +};
>
^ permalink raw reply
* Re: [PATCH v2 devicetree 1/2] powerpc: dts: t1040: add bindings for Seville Ethernet switch
From: Maxim Kochetkov @ 2020-09-29 13:12 UTC (permalink / raw)
To: Vladimir Oltean, robh+dt, shawnguo, mpe, devicetree
Cc: madalin.bucur, linux-kernel, radu-andrei.bulie, paulus, netdev,
linuxppc-dev
In-Reply-To: <20200929113209.3767787-2-vladimir.oltean@nxp.com>
Reviewed-by: Maxim Kochetkov <fido_max@inbox.ru>
29.09.2020 14:32, Vladimir Oltean пишет:
> Add the description of the embedded L2 switch inside the SoC dtsi file
> for NXP T1040.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> Changes in v2:
> Make switch node disabled by default.
>
> arch/powerpc/boot/dts/fsl/t1040si-post.dtsi | 76 +++++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi b/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
> index 315d0557eefc..5cb90c66cd3f 100644
> --- a/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
> @@ -628,6 +628,82 @@ mdio@fd000 {
> status = "disabled";
> };
> };
> +
> + seville_switch: ethernet-switch@800000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "mscc,vsc9953-switch";
> + reg = <0x800000 0x290000>;
> + little-endian;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + seville_port0: port@0 {
> + reg = <0>;
> + status = "disabled";
> + };
> +
> + seville_port1: port@1 {
> + reg = <1>;
> + status = "disabled";
> + };
> +
> + seville_port2: port@2 {
> + reg = <2>;
> + status = "disabled";
> + };
> +
> + seville_port3: port@3 {
> + reg = <3>;
> + status = "disabled";
> + };
> +
> + seville_port4: port@4 {
> + reg = <4>;
> + status = "disabled";
> + };
> +
> + seville_port5: port@5 {
> + reg = <5>;
> + status = "disabled";
> + };
> +
> + seville_port6: port@6 {
> + reg = <6>;
> + status = "disabled";
> + };
> +
> + seville_port7: port@7 {
> + reg = <7>;
> + status = "disabled";
> + };
> +
> + seville_port8: port@8 {
> + reg = <8>;
> + phy-mode = "internal";
> + status = "disabled";
> +
> + fixed-link {
> + speed = <2500>;
> + full-duplex;
> + };
> + };
> +
> + seville_port9: port@9 {
> + reg = <9>;
> + phy-mode = "internal";
> + status = "disabled";
> +
> + fixed-link {
> + speed = <2500>;
> + full-duplex;
> + };
> + };
> + };
> + };
> };
>
> &qe {
>
^ permalink raw reply
* [PATCH v2 devicetree 2/2] powerpc: dts: t1040rdb: add ports for Seville Ethernet switch
From: Vladimir Oltean @ 2020-09-29 11:32 UTC (permalink / raw)
To: robh+dt, shawnguo, mpe, devicetree
Cc: madalin.bucur, linux-kernel, radu-andrei.bulie, fido_max, paulus,
netdev, Vladimir Oltean, linuxppc-dev
In-Reply-To: <20200929113209.3767787-1-vladimir.oltean@nxp.com>
From: Vladimir Oltean <olteanv@gmail.com>
Define the network interface names for the switch ports and hook them up
to the 2 QSGMII PHYs that are onboard.
A conscious decision was taken to go along with the numbers that are
written on the front panel of the board and not with the hardware
numbers of the switch chip ports. The 2 numbering schemes are
shifted by 8.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
Use the existing way of accessing the mdio bus and not labels.
arch/powerpc/boot/dts/fsl/t1040rdb.dts | 115 +++++++++++++++++++++++++
1 file changed, 115 insertions(+)
diff --git a/arch/powerpc/boot/dts/fsl/t1040rdb.dts b/arch/powerpc/boot/dts/fsl/t1040rdb.dts
index 65ff34c49025..3fd08a2b6dcb 100644
--- a/arch/powerpc/boot/dts/fsl/t1040rdb.dts
+++ b/arch/powerpc/boot/dts/fsl/t1040rdb.dts
@@ -64,6 +64,40 @@ mdio@fc000 {
phy_sgmii_2: ethernet-phy@3 {
reg = <0x03>;
};
+
+ /* VSC8514 QSGMII PHY */
+ phy_qsgmii_0: ethernet-phy@4 {
+ reg = <0x4>;
+ };
+
+ phy_qsgmii_1: ethernet-phy@5 {
+ reg = <0x5>;
+ };
+
+ phy_qsgmii_2: ethernet-phy@6 {
+ reg = <0x6>;
+ };
+
+ phy_qsgmii_3: ethernet-phy@7 {
+ reg = <0x7>;
+ };
+
+ /* VSC8514 QSGMII PHY */
+ phy_qsgmii_4: ethernet-phy@8 {
+ reg = <0x8>;
+ };
+
+ phy_qsgmii_5: ethernet-phy@9 {
+ reg = <0x9>;
+ };
+
+ phy_qsgmii_6: ethernet-phy@a {
+ reg = <0xa>;
+ };
+
+ phy_qsgmii_7: ethernet-phy@b {
+ reg = <0xb>;
+ };
};
};
};
@@ -76,3 +110,84 @@ cpld@3,0 {
};
#include "t1040si-post.dtsi"
+
+&seville_switch {
+ status = "okay";
+};
+
+&seville_port0 {
+ managed = "in-band-status";
+ phy-handle = <&phy_qsgmii_0>;
+ phy-mode = "qsgmii";
+ /* ETH4 written on chassis */
+ label = "swp4";
+ status = "okay";
+};
+
+&seville_port1 {
+ managed = "in-band-status";
+ phy-handle = <&phy_qsgmii_1>;
+ phy-mode = "qsgmii";
+ /* ETH5 written on chassis */
+ label = "swp5";
+ status = "okay";
+};
+
+&seville_port2 {
+ managed = "in-band-status";
+ phy-handle = <&phy_qsgmii_2>;
+ phy-mode = "qsgmii";
+ /* ETH6 written on chassis */
+ label = "swp6";
+ status = "okay";
+};
+
+&seville_port3 {
+ managed = "in-band-status";
+ phy-handle = <&phy_qsgmii_3>;
+ phy-mode = "qsgmii";
+ /* ETH7 written on chassis */
+ label = "swp7";
+ status = "okay";
+};
+
+&seville_port4 {
+ managed = "in-band-status";
+ phy-handle = <&phy_qsgmii_4>;
+ phy-mode = "qsgmii";
+ /* ETH8 written on chassis */
+ label = "swp8";
+ status = "okay";
+};
+
+&seville_port5 {
+ managed = "in-band-status";
+ phy-handle = <&phy_qsgmii_5>;
+ phy-mode = "qsgmii";
+ /* ETH9 written on chassis */
+ label = "swp9";
+ status = "okay";
+};
+
+&seville_port6 {
+ managed = "in-band-status";
+ phy-handle = <&phy_qsgmii_6>;
+ phy-mode = "qsgmii";
+ /* ETH10 written on chassis */
+ label = "swp10";
+ status = "okay";
+};
+
+&seville_port7 {
+ managed = "in-band-status";
+ phy-handle = <&phy_qsgmii_7>;
+ phy-mode = "qsgmii";
+ /* ETH11 written on chassis */
+ label = "swp11";
+ status = "okay";
+};
+
+&seville_port8 {
+ ethernet = <&enet0>;
+ status = "okay";
+};
--
2.25.1
^ permalink raw reply related
* [PATCH v2 devicetree 1/2] powerpc: dts: t1040: add bindings for Seville Ethernet switch
From: Vladimir Oltean @ 2020-09-29 11:32 UTC (permalink / raw)
To: robh+dt, shawnguo, mpe, devicetree
Cc: madalin.bucur, linux-kernel, radu-andrei.bulie, fido_max, paulus,
netdev, linuxppc-dev
In-Reply-To: <20200929113209.3767787-1-vladimir.oltean@nxp.com>
Add the description of the embedded L2 switch inside the SoC dtsi file
for NXP T1040.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
Make switch node disabled by default.
arch/powerpc/boot/dts/fsl/t1040si-post.dtsi | 76 +++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi b/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
index 315d0557eefc..5cb90c66cd3f 100644
--- a/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
@@ -628,6 +628,82 @@ mdio@fd000 {
status = "disabled";
};
};
+
+ seville_switch: ethernet-switch@800000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "mscc,vsc9953-switch";
+ reg = <0x800000 0x290000>;
+ little-endian;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ seville_port0: port@0 {
+ reg = <0>;
+ status = "disabled";
+ };
+
+ seville_port1: port@1 {
+ reg = <1>;
+ status = "disabled";
+ };
+
+ seville_port2: port@2 {
+ reg = <2>;
+ status = "disabled";
+ };
+
+ seville_port3: port@3 {
+ reg = <3>;
+ status = "disabled";
+ };
+
+ seville_port4: port@4 {
+ reg = <4>;
+ status = "disabled";
+ };
+
+ seville_port5: port@5 {
+ reg = <5>;
+ status = "disabled";
+ };
+
+ seville_port6: port@6 {
+ reg = <6>;
+ status = "disabled";
+ };
+
+ seville_port7: port@7 {
+ reg = <7>;
+ status = "disabled";
+ };
+
+ seville_port8: port@8 {
+ reg = <8>;
+ phy-mode = "internal";
+ status = "disabled";
+
+ fixed-link {
+ speed = <2500>;
+ full-duplex;
+ };
+ };
+
+ seville_port9: port@9 {
+ reg = <9>;
+ phy-mode = "internal";
+ status = "disabled";
+
+ fixed-link {
+ speed = <2500>;
+ full-duplex;
+ };
+ };
+ };
+ };
};
&qe {
--
2.25.1
^ permalink raw reply related
* [PATCH v2 devicetree 0/2] Add Seville Ethernet switch to T1040RDB
From: Vladimir Oltean @ 2020-09-29 11:32 UTC (permalink / raw)
To: robh+dt, shawnguo, mpe, devicetree
Cc: madalin.bucur, linux-kernel, radu-andrei.bulie, fido_max, paulus,
netdev, linuxppc-dev
Seville is a DSA switch that is embedded inside the T1040 SoC, and
supported by the mscc_seville DSA driver inside drivers/net/dsa/ocelot.
This series adds this switch to the SoC's dtsi files and to the T1040RDB
board file.
Vladimir Oltean (2):
powerpc: dts: t1040: add bindings for Seville Ethernet switch
powerpc: dts: t1040rdb: add ports for Seville Ethernet switch
arch/powerpc/boot/dts/fsl/t1040rdb.dts | 115 ++++++++++++++++++++
arch/powerpc/boot/dts/fsl/t1040si-post.dtsi | 76 +++++++++++++
2 files changed, 191 insertions(+)
--
2.25.1
^ permalink raw reply
* Re: [PATCH v2 14/14] powerpc/pseries/iommu: Rename "direct window" to "dma window"
From: Leonardo Bras @ 2020-09-29 20:54 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev
In-Reply-To: <2ba89065-4e9c-57c2-3825-aed9a7d8451a@ozlabs.ru>
On Tue, 2020-09-29 at 13:55 +1000, Alexey Kardashevskiy wrote:
>
> On 12/09/2020 03:07, Leonardo Bras wrote:
> > Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
> >
> > A previous change introduced the usage of DDW as a bigger indirect DMA
> > mapping when the DDW available size does not map the whole partition.
> >
> > As most of the code that manipulates direct mappings was reused for
> > indirect mappings, it's necessary to rename all names and debug/info
> > messages to reflect that it can be used for both kinds of mapping.
> >
> > Also, defines DEFAULT_DMA_WIN as "ibm,dma-window" to document that
> > it's the name of the default DMA window.
>
> "ibm,dma-window" is so old so it does not need a macro (which btw would
> be DMA_WIN_PROPNAME to match the other names) :)
Thanks for bringing that to my attention!
In fact, DMA_WIN_PROPNAME makes more sense, but it's still generic and
doesn't look to point to a generic one.
Would that be ok to call it DEFAULT_WIN_PROPNAME ?
>
>
> > Those changes are not supposed to change how the code works in any
> > way, just adjust naming.
>
> I simply have this in my .vimrc for the cases like this one:
>
> ===
> This should cause no behavioural change.
> ===
Great tip! I will make sure to have this saved here :)
Thank you!
^ permalink raw reply
* Re: [PATCH v2 devicetree 2/2] powerpc: dts: t1040rdb: add ports for Seville Ethernet switch
From: Andrew Lunn @ 2020-09-29 20:10 UTC (permalink / raw)
To: Vladimir Oltean
Cc: devicetree@vger.kernel.org, Madalin Bucur (OSS),
Radu-andrei Bulie, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, fido_max@inbox.ru,
robh+dt@kernel.org, paulus@samba.org, Vladimir Oltean,
shawnguo@kernel.org, netdev@vger.kernel.org
In-Reply-To: <20200929193953.rgibttgt6lh5huef@skbuf>
On Tue, Sep 29, 2020 at 07:39:54PM +0000, Vladimir Oltean wrote:
> On Tue, Sep 29, 2020 at 09:11:53PM +0200, Andrew Lunn wrote:
> > > +&seville_port0 {
> > > + managed = "in-band-status";
> > > + phy-handle = <&phy_qsgmii_0>;
> > > + phy-mode = "qsgmii";
> > > + /* ETH4 written on chassis */
> > > + label = "swp4";
> >
> > If ETH4 is on the chassis why not use ETH4?
>
> You mean all-caps, just like that?
Yes.
DSA is often used in WiFI access point, etc. The user is not a
computer professional. If the WebGUI says ETH4, and the label on the
front says ETH4, they probably think the two are the same, and are
happy.
I have one box which does not have an labels on the front panels, but
the industrial sockets for Ethernet are colour coded. So the interface
names are red, blue, green, to match the socket colour, and the cable
set is also colour coded the same.
So long as it is unique, the kernel does not care. So make it easy for
the user.
Andrew
^ permalink raw reply
* Re: [PATCH v2 devicetree 2/2] powerpc: dts: t1040rdb: add ports for Seville Ethernet switch
From: Andrew Lunn @ 2020-09-29 19:11 UTC (permalink / raw)
To: Vladimir Oltean
Cc: devicetree, madalin.bucur, radu-andrei.bulie, linuxppc-dev,
linux-kernel, fido_max, robh+dt, paulus, Vladimir Oltean,
shawnguo, netdev
In-Reply-To: <20200929113209.3767787-3-vladimir.oltean@nxp.com>
> +&seville_port0 {
> + managed = "in-band-status";
> + phy-handle = <&phy_qsgmii_0>;
> + phy-mode = "qsgmii";
> + /* ETH4 written on chassis */
> + label = "swp4";
If ETH4 is on the chassis why not use ETH4?
Andrew
^ permalink raw reply
* Re: [PATCH v2 01/14] powerpc/pseries/iommu: Replace hard-coded page shift
From: Leonardo Bras @ 2020-09-29 18:25 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev
In-Reply-To: <5c23f1f9-4cb5-be24-3064-258c0f68c77c@ozlabs.ru>
On Tue, 2020-09-29 at 13:56 +1000, Alexey Kardashevskiy wrote:
>
> On 12/09/2020 03:07, Leonardo Bras wrote:
> > Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
>
> These normally go right before "signed-off-by".
>
Yeah, it looks like something went wrong between git format-patch and
git send-email. I will look for in in the next series.
>
> > Some functions assume IOMMU page size can only be 4K (pageshift == 12).
> > Update them to accept any page size passed, so we can use 64K pages.
> >
> > In the process, some defines like TCE_SHIFT were made obsolete, and then
> > removed.
> >
> > IODA3 Revision 3.0_prd1 (OpenPowerFoundation), Figures 3.4 and 3.5 show
> > a RPN of 52-bit, and considers a 12-bit pageshift, so there should be
> > no need of using TCE_RPN_MASK, which masks out any bit after 40 in rpn.
> > It's usage removed from tce_build_pSeries(), tce_build_pSeriesLP(), and
> > tce_buildmulti_pSeriesLP().
> >
> > Most places had a tbl struct, so using tbl->it_page_shift was simple.
> > tce_free_pSeriesLP() was a special case, since callers not always have a
> > tbl struct, so adding a tceshift parameter seems the right thing to do.
> >
> > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Thanks for reviewing!
>
>
> > ---
> > arch/powerpc/include/asm/tce.h | 8 ------
> > arch/powerpc/platforms/pseries/iommu.c | 39 +++++++++++++++-----------
> > 2 files changed, 23 insertions(+), 24 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/tce.h b/arch/powerpc/include/asm/tce.h
> > index db5fc2f2262d..0c34d2756d92 100644
> > --- a/arch/powerpc/include/asm/tce.h
> > +++ b/arch/powerpc/include/asm/tce.h
> > @@ -19,15 +19,7 @@
> > #define TCE_VB 0
> > #define TCE_PCI 1
> >
> > -/* TCE page size is 4096 bytes (1 << 12) */
> > -
> > -#define TCE_SHIFT 12
> > -#define TCE_PAGE_SIZE (1 << TCE_SHIFT)
> > -
> > #define TCE_ENTRY_SIZE 8 /* each TCE is 64 bits */
> > -
> > -#define TCE_RPN_MASK 0xfffffffffful /* 40-bit RPN (4K pages) */
> > -#define TCE_RPN_SHIFT 12
> > #define TCE_VALID 0x800 /* TCE valid */
> > #define TCE_ALLIO 0x400 /* TCE valid for all lpars */
> > #define TCE_PCI_WRITE 0x2 /* write from PCI allowed */
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > index e4198700ed1a..9db3927607a4 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -107,6 +107,8 @@ static int tce_build_pSeries(struct iommu_table *tbl, long index,
> > u64 proto_tce;
> > __be64 *tcep;
> > u64 rpn;
> > + const unsigned long tceshift = tbl->it_page_shift;
> > + const unsigned long pagesize = IOMMU_PAGE_SIZE(tbl);
> >
> > proto_tce = TCE_PCI_READ; // Read allowed
> >
> > @@ -117,10 +119,10 @@ static int tce_build_pSeries(struct iommu_table *tbl, long index,
> >
> > while (npages--) {
> > /* can't move this out since we might cross MEMBLOCK boundary */
> > - rpn = __pa(uaddr) >> TCE_SHIFT;
> > - *tcep = cpu_to_be64(proto_tce | (rpn & TCE_RPN_MASK) << TCE_RPN_SHIFT);
> > + rpn = __pa(uaddr) >> tceshift;
> > + *tcep = cpu_to_be64(proto_tce | rpn << tceshift);
> >
> > - uaddr += TCE_PAGE_SIZE;
> > + uaddr += pagesize;
> > tcep++;
> > }
> > return 0;
> > @@ -146,7 +148,7 @@ static unsigned long tce_get_pseries(struct iommu_table *tbl, long index)
> > return be64_to_cpu(*tcep);
> > }
> >
> > -static void tce_free_pSeriesLP(unsigned long liobn, long, long);
> > +static void tce_free_pSeriesLP(unsigned long liobn, long, long, long);
> > static void tce_freemulti_pSeriesLP(struct iommu_table*, long, long);
> >
> > static int tce_build_pSeriesLP(unsigned long liobn, long tcenum, long tceshift,
> > @@ -166,12 +168,12 @@ static int tce_build_pSeriesLP(unsigned long liobn, long tcenum, long tceshift,
> > proto_tce |= TCE_PCI_WRITE;
> >
> > while (npages--) {
> > - tce = proto_tce | (rpn & TCE_RPN_MASK) << tceshift;
> > + tce = proto_tce | rpn << tceshift;
> > rc = plpar_tce_put((u64)liobn, (u64)tcenum << tceshift, tce);
> >
> > if (unlikely(rc == H_NOT_ENOUGH_RESOURCES)) {
> > ret = (int)rc;
> > - tce_free_pSeriesLP(liobn, tcenum_start,
> > + tce_free_pSeriesLP(liobn, tcenum_start, tceshift,
> > (npages_start - (npages + 1)));
> > break;
> > }
> > @@ -205,10 +207,11 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum,
> > long tcenum_start = tcenum, npages_start = npages;
> > int ret = 0;
> > unsigned long flags;
> > + const unsigned long tceshift = tbl->it_page_shift;
> >
> > if ((npages == 1) || !firmware_has_feature(FW_FEATURE_PUT_TCE_IND)) {
> > return tce_build_pSeriesLP(tbl->it_index, tcenum,
> > - tbl->it_page_shift, npages, uaddr,
> > + tceshift, npages, uaddr,
> > direction, attrs);
> > }
> >
> > @@ -225,13 +228,13 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum,
> > if (!tcep) {
> > local_irq_restore(flags);
> > return tce_build_pSeriesLP(tbl->it_index, tcenum,
> > - tbl->it_page_shift,
> > + tceshift,
> > npages, uaddr, direction, attrs);
> > }
> > __this_cpu_write(tce_page, tcep);
> > }
> >
> > - rpn = __pa(uaddr) >> TCE_SHIFT;
> > + rpn = __pa(uaddr) >> tceshift;
> > proto_tce = TCE_PCI_READ;
> > if (direction != DMA_TO_DEVICE)
> > proto_tce |= TCE_PCI_WRITE;
> > @@ -245,12 +248,12 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum,
> > limit = min_t(long, npages, 4096/TCE_ENTRY_SIZE);
> >
> > for (l = 0; l < limit; l++) {
> > - tcep[l] = cpu_to_be64(proto_tce | (rpn & TCE_RPN_MASK) << TCE_RPN_SHIFT);
> > + tcep[l] = cpu_to_be64(proto_tce | rpn << tceshift);
> > rpn++;
> > }
> >
> > rc = plpar_tce_put_indirect((u64)tbl->it_index,
> > - (u64)tcenum << 12,
> > + (u64)tcenum << tceshift,
> > (u64)__pa(tcep),
> > limit);
> >
> > @@ -277,12 +280,13 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum,
> > return ret;
> > }
> >
> > -static void tce_free_pSeriesLP(unsigned long liobn, long tcenum, long npages)
> > +static void tce_free_pSeriesLP(unsigned long liobn, long tcenum, long tceshift,
> > + long npages)
> > {
> > u64 rc;
> >
> > while (npages--) {
> > - rc = plpar_tce_put((u64)liobn, (u64)tcenum << 12, 0);
> > + rc = plpar_tce_put((u64)liobn, (u64)tcenum << tceshift, 0);
> >
> > if (rc && printk_ratelimit()) {
> > printk("tce_free_pSeriesLP: plpar_tce_put failed. rc=%lld\n", rc);
> > @@ -301,9 +305,11 @@ static void tce_freemulti_pSeriesLP(struct iommu_table *tbl, long tcenum, long n
> > u64 rc;
> >
> > if (!firmware_has_feature(FW_FEATURE_STUFF_TCE))
> > - return tce_free_pSeriesLP(tbl->it_index, tcenum, npages);
> > + return tce_free_pSeriesLP(tbl->it_index, tcenum,
> > + tbl->it_page_shift, npages);
> >
> > - rc = plpar_tce_stuff((u64)tbl->it_index, (u64)tcenum << 12, 0, npages);
> > + rc = plpar_tce_stuff((u64)tbl->it_index,
> > + (u64)tcenum << tbl->it_page_shift, 0, npages);
> >
> > if (rc && printk_ratelimit()) {
> > printk("tce_freemulti_pSeriesLP: plpar_tce_stuff failed\n");
> > @@ -319,7 +325,8 @@ static unsigned long tce_get_pSeriesLP(struct iommu_table *tbl, long tcenum)
> > u64 rc;
> > unsigned long tce_ret;
> >
> > - rc = plpar_tce_get((u64)tbl->it_index, (u64)tcenum << 12, &tce_ret);
> > + rc = plpar_tce_get((u64)tbl->it_index,
> > + (u64)tcenum << tbl->it_page_shift, &tce_ret);
> >
> > if (rc && printk_ratelimit()) {
> > printk("tce_get_pSeriesLP: plpar_tce_get failed. rc=%lld\n", rc);
> >
^ permalink raw reply
* Re: [PATCH] powerpc: net: bpf_jit_comp: Fix misuse of fallthrough
From: Daniel Borkmann @ 2020-09-29 14:41 UTC (permalink / raw)
To: zhe.he, gustavo, netdev, bpf, linuxppc-dev, linux-kernel
In-Reply-To: <20200928090023.38117-1-zhe.he@windriver.com>
On 9/28/20 11:00 AM, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
>
> The user defined label following "fallthrough" is not considered by GCC
> and causes build failure.
>
> kernel-source/include/linux/compiler_attributes.h:208:41: error: attribute
> 'fallthrough' not preceding a case label or default label [-Werror]
> 208 define fallthrough _attribute((fallthrough_))
> ^~~~~~~~~~~~~
>
> Signed-off-by: He Zhe <zhe.he@windriver.com>
Applied, thanks! I've also added Fixes tag with df561f6688fe ("treewide: Use fallthrough pseudo-keyword")
which added the bug.
^ permalink raw reply
* Re: [PATCH v2 2/3] lkdtm/powerpc: Add SLB multihit test
From: Michal Suchánek @ 2020-09-29 15:01 UTC (permalink / raw)
To: Kees Cook; +Cc: linuxppc-dev, Ganesh Goudar, mahesh, npiggin
In-Reply-To: <202009251244.A4396AFF@keescook>
Hello,
On Fri, Sep 25, 2020 at 12:57:33PM -0700, Kees Cook wrote:
> On Fri, Sep 25, 2020 at 04:01:22PM +0530, Ganesh Goudar wrote:
> > Add support to inject slb multihit errors, to test machine
> > check handling.
>
> Thank you for more tests in here!
Thanks for working on integrating this.
>
> >
> > Based on work by Mahesh Salgaonkar and Michal Suchánek.
> >
> > Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> > Cc: Michal Suchánek <msuchanek@suse.de>
>
> Should these be Co-developed-by: with S-o-b?
I don't think I wrote any of this code. I packaged it for SUSE and maybe
changed some constants based on test result discussion.
I compared this code to my saved snapshots of past versions of the test
module and this covers all the test cases I have. The only difference is that
the development modules have verbose prints showing what's going on.
It is true that without the verbose prints some explanatory comments
could be helpful.
Reviewed-by: Michal Suchánek <msuchanek@suse.de>
>
> > Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
> > ---
> > drivers/misc/lkdtm/Makefile | 4 ++
> > drivers/misc/lkdtm/core.c | 3 +
> > drivers/misc/lkdtm/lkdtm.h | 3 +
> > drivers/misc/lkdtm/powerpc.c | 132 +++++++++++++++++++++++++++++++++++
> > 4 files changed, 142 insertions(+)
> > create mode 100644 drivers/misc/lkdtm/powerpc.c
> >
> > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
> > index c70b3822013f..6a82f407fbcd 100644
> > --- a/drivers/misc/lkdtm/Makefile
> > +++ b/drivers/misc/lkdtm/Makefile
> > @@ -11,6 +11,10 @@ lkdtm-$(CONFIG_LKDTM) += usercopy.o
> > lkdtm-$(CONFIG_LKDTM) += stackleak.o
> > lkdtm-$(CONFIG_LKDTM) += cfi.o
> >
> > +ifeq ($(CONFIG_PPC64),y)
> > +lkdtm-$(CONFIG_LKDTM) += powerpc.o
> > +endif
>
> This can just be:
>
> lkdtm-$(CONFIG_PPC64) += powerpc.o
>
> > +
> > KASAN_SANITIZE_stackleak.o := n
> > KCOV_INSTRUMENT_rodata.o := n
> >
> > diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
> > index a5e344df9166..8d5db42baa90 100644
> > --- a/drivers/misc/lkdtm/core.c
> > +++ b/drivers/misc/lkdtm/core.c
> > @@ -178,6 +178,9 @@ static const struct crashtype crashtypes[] = {
> > #ifdef CONFIG_X86_32
> > CRASHTYPE(DOUBLE_FAULT),
> > #endif
> > +#ifdef CONFIG_PPC64
> > + CRASHTYPE(PPC_SLB_MULTIHIT),
> > +#endif
> > };
> >
> >
> > diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
> > index 8878538b2c13..b305bd511ee5 100644
> > --- a/drivers/misc/lkdtm/lkdtm.h
> > +++ b/drivers/misc/lkdtm/lkdtm.h
> > @@ -104,4 +104,7 @@ void lkdtm_STACKLEAK_ERASING(void);
> > /* cfi.c */
> > void lkdtm_CFI_FORWARD_PROTO(void);
> >
> > +/* powerpc.c */
> > +void lkdtm_PPC_SLB_MULTIHIT(void);
> > +
> > #endif
> > diff --git a/drivers/misc/lkdtm/powerpc.c b/drivers/misc/lkdtm/powerpc.c
> > new file mode 100644
> > index 000000000000..d6db18444757
> > --- /dev/null
> > +++ b/drivers/misc/lkdtm/powerpc.c
> > @@ -0,0 +1,132 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
>
> Please #include "lkdtm.h" here to get the correct pr_fmt heading (and
> any future header adjustments).
>
> > +#include <linux/slab.h>
> > +#include <linux/vmalloc.h>
> > +
> > +static inline unsigned long get_slb_index(void)
> > +{
> > + unsigned long index;
> > +
> > + index = get_paca()->stab_rr;
> > +
> > + /*
> > + * simple round-robin replacement of slb starting at SLB_NUM_BOLTED.
> > + */
> > + if (index < (mmu_slb_size - 1))
> > + index++;
> > + else
> > + index = SLB_NUM_BOLTED;
> > + get_paca()->stab_rr = index;
> > + return index;
> > +}
> > +
> > +#define slb_esid_mask(ssize) \
> > + (((ssize) == MMU_SEGSIZE_256M) ? ESID_MASK : ESID_MASK_1T)
> > +
> > +static inline unsigned long mk_esid_data(unsigned long ea, int ssize,
> > + unsigned long slot)
> > +{
> > + return (ea & slb_esid_mask(ssize)) | SLB_ESID_V | slot;
> > +}
> > +
> > +#define slb_vsid_shift(ssize) \
> > + ((ssize) == MMU_SEGSIZE_256M ? SLB_VSID_SHIFT : SLB_VSID_SHIFT_1T)
> > +
> > +static inline unsigned long mk_vsid_data(unsigned long ea, int ssize,
> > + unsigned long flags)
> > +{
> > + return (get_kernel_vsid(ea, ssize) << slb_vsid_shift(ssize)) | flags |
> > + ((unsigned long)ssize << SLB_VSID_SSIZE_SHIFT);
> > +}
> > +
> > +static void insert_slb_entry(char *p, int ssize)
> > +{
> > + unsigned long flags, entry;
> > +
> > + flags = SLB_VSID_KERNEL | mmu_psize_defs[MMU_PAGE_64K].sllp;
> > + preempt_disable();
> > +
> > + entry = get_slb_index();
> > + asm volatile("slbmte %0,%1" :
> > + : "r" (mk_vsid_data((unsigned long)p, ssize, flags)),
> > + "r" (mk_esid_data((unsigned long)p, ssize, entry))
> > + : "memory");
> > +
> > + entry = get_slb_index();
> > + asm volatile("slbmte %0,%1" :
> > + : "r" (mk_vsid_data((unsigned long)p, ssize, flags)),
> > + "r" (mk_esid_data((unsigned long)p, ssize, entry))
> > + : "memory");
> > + preempt_enable();
> > + p[0] = '!';
> > +}
>
> Can you add some comments to these helpers? It'll help people (me) with
> understanding what is actually being done here (and more importantly,
> what is _expected_ to happen).
>
> > +
> > +static void inject_vmalloc_slb_multihit(void)
> > +{
> > + char *p;
> > +
> > + p = vmalloc(2048);
> > + if (!p)
> > + return;
> > +
> > + insert_slb_entry(p, MMU_SEGSIZE_1T);
> > + vfree(p);
> > +}
> > +
> > +static void inject_kmalloc_slb_multihit(void)
> > +{
> > + char *p;
> > +
> > + p = kmalloc(2048, GFP_KERNEL);
> > + if (!p)
> > + return;
> > +
> > + insert_slb_entry(p, MMU_SEGSIZE_1T);
> > + kfree(p);
> > +}
>
> It looks like the expected failure injection is actually the "p[0] = '!'" line in the
> "insert" helper? I would expect pr_info/pr_err wrappers, etc, as in
> other lkdtm tests.
>
> If this is the negative test, what does the positive test look like?
> e.g. in other lkdtm tests, first a positive test is done, then a
> negative: first run a legit function, then run a function from a bad
> location.
>
> > +
> > +static void insert_dup_slb_entry_0(void)
> > +{
> > + unsigned long test_address = 0xC000000000000000;
> > + volatile unsigned long *test_ptr;
> > + unsigned long entry, i = 0;
> > + unsigned long esid, vsid;
> > +
> > + test_ptr = (unsigned long *)test_address;
> > + preempt_disable();
> > +
> > + asm volatile("slbmfee %0,%1" : "=r" (esid) : "r" (i));
> > + asm volatile("slbmfev %0,%1" : "=r" (vsid) : "r" (i));
> > + entry = get_slb_index();
> > +
> > + /* for i !=0 we would need to mask out the old entry number */
> > + asm volatile("slbmte %0,%1" :
> > + : "r" (vsid),
> > + "r" (esid | entry)
> > + : "memory");
> > +
> > + asm volatile("slbmfee %0,%1" : "=r" (esid) : "r" (i));
> > + asm volatile("slbmfev %0,%1" : "=r" (vsid) : "r" (i));
> > + entry = get_slb_index();
> > +
> > + /* for i !=0 we would need to mask out the old entry number */
> > + asm volatile("slbmte %0,%1" :
> > + : "r" (vsid),
> > + "r" (esid | entry)
> > + : "memory");
> > +
> > + pr_info("lkdtm: %s accessing test address 0x%lx: 0x%lx\n",
> > + __func__, test_address, *test_ptr);
> > +
> > + preempt_enable();
> > +}
>
> What does this do?
>
> > +
> > +void lkdtm_PPC_SLB_MULTIHIT(void)
> > +{
> > + if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
> > + inject_vmalloc_slb_multihit();
> > + inject_kmalloc_slb_multihit();
> > + insert_dup_slb_entry_0();
> > + }
> > + pr_info("Recovered from SLB multihit. (Ignore this message on non HPTE machines)");
>
> Is this bad? If so, I'd expect pr_err("FAIL: ...") Can HPTE machines be
> detected so that an XFAIL can be emitted instead?
>
> Since there are three (two?) distinct regions being tested, should these
> be separate tests? Right now there is no way to separate a vmalloc
> failure from a kmalloc failure, and no way to fail the first without
> hiding the result from the latter (or maybe the machine cannot survive
> this test? ... which should also be a comment.)
>
> And finally, assuming a successful test (or testing from a separate
> thread later), so there any state that needs to be restored (or cleaned
> up before doing the "insert" calls)?
>
> Thanks!
>
> --
> Kees Cook
^ permalink raw reply
* Re: [PATCH] linux: configure CONFIG_I2C_OPAL as in-built.
From: Nayna @ 2020-09-29 13:17 UTC (permalink / raw)
To: Joel Stanley, linuxppc-dev
Cc: openpower-firmware, Nayna Jain, klaus, Mimi Zohar
In-Reply-To: <CACPK8XdeAzbXcm2w6kJuAQzckdyFQ2P9h+fC36ZArpkubqC1mg@mail.gmail.com>
On 9/29/20 2:14 AM, Joel Stanley wrote:
> On Fri, 25 Sep 2020 at 18:19, Mimi Zohar <zohar@linux.ibm.com> wrote:
>> Hi Nayna,
>>
>> On Wed, 2020-09-23 at 14:25 -0400, Nayna Jain wrote:
>>> Currently, skiroot_defconfig CONFIG_I2C_OPAL is built as a loadable
>>> module rather than builtin, even if CONFIG_I2C=y is defined. This
>>> results in a delay in the TPM initialization, causing IMA to go into
>>> TPM bypass mode. As a result, the IMA measurements are added to the
>>> measurement list, but do not extend the TPM. Because of this, it is
>>> impossible to verify or attest to the system's integrity, either from
>>> skiroot or the target Host OS.
>> The patch description is good, but perhaps we could provide a bit more
>> context before.
>>
>> The concept of trusted boot requires the measurement to be added to the
>> measurement list and extend the TPM, prior to allowing access to the
>> file. By allowing access to a file before its measurement is included
>> in the measurement list and extended into the TPM PCR, a malicious file
>> could potentially prevent its own measurement from being added. As the
>> PCRs are tamper proof, measuring and extending the TPM prior to giving
>> access to the file, guarantees that all file measurements are included
>> in the measurement list, including the malicious file.
>>
>> IMA needs to be enabled before any files are accessed in order to
>> verify a file's integrity and extend the TPM with the file
>> measurement. Queueing file measurements breaks the measure and extend,
>> before usage, trusted boot paradigm.
>>
>> The ima-evm-utils package includes a test for walking the IMA
>> measurement list, calculating the expected TPM PCRs, and comparing the
>> calculated PCR values with the physical TPM. Testing is important to
>> ensure the TPM is initialized prior to IMA. Failure to validate the
>> IMA measurement list may indicate IMA went into TPM bypass mode, like
>> in this case.
> Thanks for the explanation Mimi. It's lucky that the TPM drivers can
> be loaded early enough!
>
> Should we add something like this to security/integrity/ima/Kconfig?
>
> select I2C_OPAL if PPC_POWERNV
>
> It's generally frowned upon to select user visible symbols, but IMA
> does this for the TCG options already.
I think yes, but in drivers/i2c/Kconfig and not in IMA Kconfig.
IMA is dependent on TPM, and it is specified in IMA Kconfig.
TPM NUVOTON driver has its dependency on I2C, which is taken care in
drivers/char/tpm/Kconfig
It is I2C driver which is dependent on I2C_OPAL for its functioning on
POWERNV Systems.
Thanks & Regards,
- Nayna
^ permalink raw reply
* Re: [PATCH] linux: configure CONFIG_I2C_OPAL as in-built.
From: Mimi Zohar @ 2020-09-29 12:19 UTC (permalink / raw)
To: Joel Stanley, linuxppc-dev; +Cc: openpower-firmware, Nayna Jain, klaus
In-Reply-To: <CACPK8XdeAzbXcm2w6kJuAQzckdyFQ2P9h+fC36ZArpkubqC1mg@mail.gmail.com>
Hi Joel,
On Tue, 2020-09-29 at 06:14 +0000, Joel Stanley wrote:
> On Fri, 25 Sep 2020 at 18:19, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > Hi Nayna,
> >
> > On Wed, 2020-09-23 at 14:25 -0400, Nayna Jain wrote:
> > > Currently, skiroot_defconfig CONFIG_I2C_OPAL is built as a loadable
> > > module rather than builtin, even if CONFIG_I2C=y is defined. This
> > > results in a delay in the TPM initialization, causing IMA to go into
> > > TPM bypass mode. As a result, the IMA measurements are added to the
> > > measurement list, but do not extend the TPM. Because of this, it is
> > > impossible to verify or attest to the system's integrity, either from
> > > skiroot or the target Host OS.
> >
> > The patch description is good, but perhaps we could provide a bit more
> > context before.
> >
> > The concept of trusted boot requires the measurement to be added to the
> > measurement list and extend the TPM, prior to allowing access to the
> > file. By allowing access to a file before its measurement is included
> > in the measurement list and extended into the TPM PCR, a malicious file
> > could potentially prevent its own measurement from being added. As the
> > PCRs are tamper proof, measuring and extending the TPM prior to giving
> > access to the file, guarantees that all file measurements are included
> > in the measurement list, including the malicious file.
> >
> > IMA needs to be enabled before any files are accessed in order to
> > verify a file's integrity and extend the TPM with the file
> > measurement. Queueing file measurements breaks the measure and extend,
> > before usage, trusted boot paradigm.
> >
> > The ima-evm-utils package includes a test for walking the IMA
> > measurement list, calculating the expected TPM PCRs, and comparing the
> > calculated PCR values with the physical TPM. Testing is important to
> > ensure the TPM is initialized prior to IMA. Failure to validate the
> > IMA measurement list may indicate IMA went into TPM bypass mode, like
> > in this case.
>
> Thanks for the explanation Mimi. It's lucky that the TPM drivers can
> be loaded early enough!
>
> Should we add something like this to security/integrity/ima/Kconfig?
>
> select I2C_OPAL if PPC_POWERNV
>
> It's generally frowned upon to select user visible symbols, but IMA
> does this for the TCG options already.
The other examples enable the TPM. I2C_OPAL is dependent on I2C being
builtin. I'm not sure if this select is complete, nor if this is where
it belongs.
Mimi
^ permalink raw reply
* [PATCH -next] ASoC: fsl: imx-mc13783: use devm_snd_soc_register_card()
From: Qinglang Miao @ 2020-09-29 11:29 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team
Cc: alsa-devel, Qinglang Miao, linuxppc-dev, linux-kernel,
linux-arm-kernel
Using devm_snd_soc_register_card() can make the code
shorter and cleaner.
Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com>
---
sound/soc/fsl/imx-mc13783.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/sound/soc/fsl/imx-mc13783.c b/sound/soc/fsl/imx-mc13783.c
index dd9c1ac81..d9dca7bbc 100644
--- a/sound/soc/fsl/imx-mc13783.c
+++ b/sound/soc/fsl/imx-mc13783.c
@@ -96,7 +96,7 @@ static int imx_mc13783_probe(struct platform_device *pdev)
imx_mc13783.dev = &pdev->dev;
- ret = snd_soc_register_card(&imx_mc13783);
+ ret = devm_snd_soc_register_card(&pdev->dev, &imx_mc13783);
if (ret) {
dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n",
ret);
@@ -140,19 +140,11 @@ static int imx_mc13783_probe(struct platform_device *pdev)
return ret;
}
-static int imx_mc13783_remove(struct platform_device *pdev)
-{
- snd_soc_unregister_card(&imx_mc13783);
-
- return 0;
-}
-
static struct platform_driver imx_mc13783_audio_driver = {
.driver = {
.name = "imx_mc13783",
},
.probe = imx_mc13783_probe,
- .remove = imx_mc13783_remove
};
module_platform_driver(imx_mc13783_audio_driver);
--
2.23.0
^ permalink raw reply related
* RE: [PATCH v2 1/2] ASoC: fsl_xcvr: Add XCVR ASoC CPU DAI driver
From: Viorel Suman (OSS) @ 2020-09-29 9:27 UTC (permalink / raw)
To: Philipp Zabel, Viorel Suman (OSS)
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
Matthias Schiffer, Timur Tabi, Xiubo Li, Shengjiu Wang,
linuxppc-dev@lists.ozlabs.org, Takashi Iwai, Rob Herring,
Liam Girdwood, Nicolin Chen, Mark Brown, dl-linux-imx,
Viorel Suman, Cosmin-Gabriel Samoila, Jaroslav Kysela,
Fabio Estevam, linux-kernel@vger.kernel.org
In-Reply-To: <20200922120854.GA15104@pengutronix.de>
Hi Philipp,
Thank you for your review, please check my comments inline.
/Viorel
> -----Original Message-----
> From: Philipp Zabel [mailto:pza@pengutronix.de]
> Sent: Tuesday, September 22, 2020 3:09 PM
> To: Viorel Suman (OSS) <viorel.suman@oss.nxp.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>; Mark Brown
> <broonie@kernel.org>; Rob Herring <robh+dt@kernel.org>; Jaroslav Kysela
> <perex@perex.cz>; Takashi Iwai <tiwai@suse.com>; Timur Tabi
> <timur@kernel.org>; Nicolin Chen <nicoleotsuka@gmail.com>; Xiubo Li
> <Xiubo.Lee@gmail.com>; Fabio Estevam <festevam@gmail.com>; Shengjiu
> Wang <shengjiu.wang@gmail.com>; Viorel Suman <viorel.suman@nxp.com>;
> Matthias Schiffer <matthias.schiffer@ew.tq-group.com>; Cosmin-Gabriel
> Samoila <cosmin.samoila@nxp.com>; alsa-devel@alsa-project.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org; dl-linux-imx <linux-imx@nxp.com>; Viorel Suman
> <viorel.suman@gmail.com>
> Subject: Re: [PATCH v2 1/2] ASoC: fsl_xcvr: Add XCVR ASoC CPU DAI driver
>
> On Mon, Sep 21, 2020 at 10:08:11PM +0300, Viorel Suman (OSS) wrote:
> > From: Viorel Suman <viorel.suman@nxp.com>
> >
> > XCVR (Audio Transceiver) is a on-chip functional module found on
> > i.MX8MP. It support HDMI2.1 eARC, HDMI1.4 ARC and SPDIF.
> >
> > Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
> > ---
> > sound/soc/fsl/Kconfig | 10 +
> > sound/soc/fsl/Makefile | 2 +
> > sound/soc/fsl/fsl_xcvr.c | 1343
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > sound/soc/fsl/fsl_xcvr.h | 266 +++++++++
> > 4 files changed, 1621 insertions(+)
> > create mode 100644 sound/soc/fsl/fsl_xcvr.c create mode 100644
> > sound/soc/fsl/fsl_xcvr.h
> >
> > diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig index
> > 3f76ff7..d04b64d 100644
> > --- a/sound/soc/fsl/Kconfig
> > +++ b/sound/soc/fsl/Kconfig
> > @@ -95,6 +95,16 @@ config SND_SOC_FSL_EASRC
> > destination sample rate. It is a new design module compare with the
> > old ASRC.
> >
> > +config SND_SOC_FSL_XCVR
> > + tristate "NXP Audio Transceiver (XCVR) module support"
> > + select REGMAP_MMIO
> > + select SND_SOC_IMX_PCM_DMA if SND_IMX_SOC != n
> > + select SND_SOC_GENERIC_DMAENGINE_PCM
> > + help
> > + Say Y if you want to add Audio Transceiver (XCVR) support for NXP
> > + iMX CPUs. XCVR is a digital module that supports HDMI2.1 eARC,
> > + HDMI1.4 ARC and SPDIF.
> > +
> > config SND_SOC_FSL_UTILS
> > tristate
> >
> > diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile index
> > b835eeb..1d2231f 100644
> > --- a/sound/soc/fsl/Makefile
> > +++ b/sound/soc/fsl/Makefile
> > @@ -25,6 +25,7 @@ snd-soc-fsl-utils-objs := fsl_utils.o
> > snd-soc-fsl-dma-objs := fsl_dma.o snd-soc-fsl-mqs-objs := fsl_mqs.o
> > snd-soc-fsl-easrc-objs := fsl_easrc.o
> > +snd-soc-fsl-xcvr-objs := fsl_xcvr.o
> >
> > obj-$(CONFIG_SND_SOC_FSL_AUDMIX) += snd-soc-fsl-audmix.o
> > obj-$(CONFIG_SND_SOC_FSL_ASOC_CARD) += snd-soc-fsl-asoc-card.o @@
> > -38,6 +39,7 @@ obj-$(CONFIG_SND_SOC_FSL_UTILS) += snd-soc-fsl-utils.o
> > obj-$(CONFIG_SND_SOC_FSL_MQS) += snd-soc-fsl-mqs.o
> > obj-$(CONFIG_SND_SOC_FSL_EASRC) += snd-soc-fsl-easrc.o
> > obj-$(CONFIG_SND_SOC_POWERPC_DMA) += snd-soc-fsl-dma.o
> > +obj-$(CONFIG_SND_SOC_FSL_XCVR) += snd-soc-fsl-xcvr.o
> >
> > # MPC5200 Platform Support
> > obj-$(CONFIG_SND_MPC52xx_DMA) += mpc5200_dma.o diff --git
> > a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c new file mode
> > 100644 index 00000000..7391bca
> > --- /dev/null
> > +++ b/sound/soc/fsl/fsl_xcvr.c
> > @@ -0,0 +1,1343 @@
> [...]
> > +static int fsl_xcvr_probe(struct platform_device *pdev) {
> > + struct device *dev = &pdev->dev;
> > + struct device_node *np = dev->of_node;
> > + const struct of_device_id *of_id;
> > + struct fsl_xcvr *xcvr;
> > + struct resource *ram_res, *regs_res, *rx_res, *tx_res;
> > + void __iomem *regs;
> > + int ret, irq;
> > +
> > + of_id = of_match_device(fsl_xcvr_dt_ids, dev);
> > + if (!of_id)
> > + return -EINVAL;
> > +
> > + xcvr = devm_kzalloc(dev, sizeof(*xcvr), GFP_KERNEL);
> > + if (!xcvr)
> > + return -ENOMEM;
> > +
> > + xcvr->pdev = pdev;
> > + xcvr->ipg_clk = devm_clk_get(dev, "ipg");
> > + if (IS_ERR(xcvr->ipg_clk)) {
> > + dev_err(dev, "failed to get ipg clock\n");
> > + return PTR_ERR(xcvr->ipg_clk);
> > + }
> > +
> > + xcvr->phy_clk = devm_clk_get(dev, "phy");
> > + if (IS_ERR(xcvr->phy_clk)) {
> > + dev_err(dev, "failed to get phy clock\n");
> > + return PTR_ERR(xcvr->phy_clk);
> > + }
> > +
> > + xcvr->spba_clk = devm_clk_get(dev, "spba");
> > + if (IS_ERR(xcvr->spba_clk)) {
> > + dev_err(dev, "failed to get spba clock\n");
> > + return PTR_ERR(xcvr->spba_clk);
> > + }
> > +
> > + xcvr->pll_ipg_clk = devm_clk_get(dev, "pll_ipg");
> > + if (IS_ERR(xcvr->pll_ipg_clk)) {
> > + dev_err(dev, "failed to get pll_ipg clock\n");
> > + return PTR_ERR(xcvr->pll_ipg_clk);
> > + }
> > +
> > + ram_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "ram");
> > + xcvr->ram_addr = devm_ioremap_resource(dev, ram_res);
> > + if (IS_ERR(xcvr->ram_addr))
> > + return PTR_ERR(xcvr->ram_addr);
> > +
> > + regs_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "regs");
> > + regs = devm_ioremap_resource(dev, regs_res);
> > + if (IS_ERR(regs))
> > + return PTR_ERR(regs);
> > +
> > + xcvr->regmap = devm_regmap_init_mmio_clk(dev, NULL, regs,
> > + &fsl_xcvr_regmap_cfg);
> > + if (IS_ERR(xcvr->regmap)) {
> > + dev_err(dev, "failed to init XCVR regmap: %ld\n",
> > + PTR_ERR(xcvr->regmap));
> > + return PTR_ERR(xcvr->regmap);
> > + }
> > +
> > + xcvr->reset = of_reset_control_get(np, NULL);
>
> Please use devm_reset_control_get_exclusive().
Done in V3.
>
> [...]
> > +static __maybe_unused int fsl_xcvr_runtime_resume(struct device *dev)
> > +{
> > + struct fsl_xcvr *xcvr = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + ret = clk_prepare_enable(xcvr->ipg_clk);
> > + if (ret) {
> > + dev_err(dev, "failed to start IPG clock.\n");
> > + return ret;
> > + }
> > +
> > + ret = clk_prepare_enable(xcvr->pll_ipg_clk);
> > + if (ret) {
> > + dev_err(dev, "failed to start PLL IPG clock.\n");
> > + goto stop_ipg_clk;
> > + }
> > +
> > + ret = clk_prepare_enable(xcvr->phy_clk);
> > + if (ret) {
> > + dev_err(dev, "failed to start PHY clock: %d\n", ret);
> > + goto stop_pll_ipg_clk;
> > + }
> > +
> > + ret = clk_prepare_enable(xcvr->spba_clk);
> > + if (ret) {
> > + dev_err(dev, "failed to start SPBA clock.\n");
> > + goto stop_phy_clk;
> > + }
> > +
> > + regcache_cache_only(xcvr->regmap, false);
> > + regcache_mark_dirty(xcvr->regmap);
> > + ret = regcache_sync(xcvr->regmap);
> > +
> > + if (ret) {
> > + dev_err(dev, "failed to sync regcache.\n");
> > + goto stop_spba_clk;
> > + }
> > +
> > + reset_control_assert(xcvr->reset);
> > + reset_control_deassert(xcvr->reset);
>
> No delay required between the two?
Not required. Just to keep things in proper context - in
V3 I moved reset_control_assert call into runtime_suspend.
/Viorel
^ permalink raw reply
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