Re: [PATCH v4 2/2] PCI/PM: Fix pci_pm_suspend_noirq() to disable PTM
On Tue, 2022-04-26 at 11:50 -0500, Bjorn Helgaas wrote:
> On Mon, Apr 25, 2022 at 11:32:54AM -0700, David E. Box wrote:
> > On Sat, 2022-04-23 at 10:01 -0500, Bjorn Helgaas wrote:
> > > On Sat, Apr 23, 2022 at 12:43:14AM +, Jingar, Rajvi wrote:
> > > > > -Original Message-
> > > > > From: Bjorn Helgaas
> > > > > On Thu, Apr 14, 2022 at 07:54:02PM +0200, Rafael J. Wysocki wrote:
> > > > > > On 3/25/2022 8:50 PM, Rajvi Jingar wrote:
> > > > > > > For the PCIe devices (like nvme) that do not go into D3 state
> > > > > > > still
> > > > > > > need to
> > > > > > > disable PTM on PCIe root ports to allow the port to enter a lower-
> > > > > > > power PM
> > > > > > > state and the SoC to reach a lower-power idle state as a whole.
> > > > > > > Move
> > > > > > > the
> > > > > > > pci_disable_ptm() out of pci_prepare_to_sleep() as this code path
> > > > > > > is
> > > > > > > not
> > > > > > > followed for devices that do not go into D3. This patch fixes the
> > > > > > > issue
> > > > > > > seen on Dell XPS 9300 with Ice Lake CPU and Dell Precision 5530
> > > > > > > with
> > > > > > > Coffee
> > > > > > > Lake CPU platforms to get improved residency in low power idle
> > > > > > > states.
> > > > > > >
> > > > > > > Fixes: a697f072f5da ("PCI: Disable PTM during suspend to save
> > > > > > > power")
> > > > > > > Signed-off-by: Rajvi Jingar
> > > > > > > Suggested-by: David E. Box
> > > > > > > ---
> > > > > > > drivers/pci/pci-driver.c | 10 ++
> > > > > > > drivers/pci/pci.c| 10 --
> > > > > > > 2 files changed, 10 insertions(+), 10 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > > > > > > index 8b55a90126a2..ab733374a260 100644
> > > > > > > --- a/drivers/pci/pci-driver.c
> > > > > > > +++ b/drivers/pci/pci-driver.c
> > > > > > > @@ -847,6 +847,16 @@ static int pci_pm_suspend_noirq(struct device
> > > > > > > *dev)
> > > > > > > if (!pci_dev->state_saved) {
> > > > > > > pci_save_state(pci_dev);
> > > > > > > + /*
> > > > > > > + * There are systems (for example, Intel mobile chips
> > > > > > > since
> > > > > Coffee
> > > > > > > + * Lake) where the power drawn while suspended can be
> > > > > significantly
> > > > > > > + * reduced by disabling PTM on PCIe root ports as this
> > > > > > > allows the
> > > > > > > + * port to enter a lower-power PM state and the SoC to
> > > > > > > reach a
> > > > > > > + * lower-power idle state as a whole.
> > > > > > > + */
> > > > > > > + if (pci_pcie_type(pci_dev) == PCI_EXP_TYPE_ROOT_PORT)
> > > > > > > + pci_disable_ptm(pci_dev);
> > > > >
> > > > > Why is disabling PTM dependent on pci_dev->state_saved? The
> > > > > point of this is to change the behavior of the device, and it
> > > > > seems like we want to do that regardless of whether the driver
> > > > > has used pci_save_state().
> > > >
> > > > Because we use the saved state to restore PTM on the root port.
> > > > And it's under this condition that the root port state gets
> > > > saved.
> > >
> > > Yes, I understand that pci_restore_ptm_state() depends on a
> > > previous call to pci_save_ptm_state().
> > >
> > > The point I'm trying to make is that pci_disable_ptm() changes the
> > > state of the device, and that state change should not depend on
> > > whether the driver has used pci_save_state().
> >
> > We do it here because D3 depends on whether the device state was
> > saved by the driver.
> >
> > if (!pci_dev->state_saved) {
> > pci_save_state(pci_dev);
> >
> > /* disable PTM here */
> >
> > if (pci_power_manageable(pci_dev))
> > pci_prepare_to_sleep(pci_dev);
> > }
> >
> >
> > If we disable PTM before the check, we will have saved "PTM
> > disabled" as the restore state. And we can't do it after the check
> > as the device will be in D3.
>
> Are you suggesting that PTM should be left enabled if the driver
> called pci_save_state(), but disabled otherwise? I don't see the
> rationale for that.
No. I was saying that because pci_power_manageable() depends on the state not
being saved, we depended on it too ...
>
> I don't understand all the paths through pci_pm_suspend_noirq() (e.g.,
> skip_bus_pm), but for this one, I think we could do something like
> this:
>
> driver_saved = pci_dev->state_saved;
> if (!driver_saved)
> pci_save_state(pci_dev);
>
> pci_disable_ptm(pci_dev);
>
> if (!driver_saved) {
> if (pci_power_manageable(pci_dev))
> pci_prepare_to_sleep(pci_dev);
> }
... but this solution gets us away from dependency. We'll make this change.
>
> Or I guess one could argue that a driver calling pci_save_state() is
> implicitly taking responsibility for all PCI-related suspend work, and
> it should be disabling PTM itself. But that doesn't really seem
> maintainable.
>
[PATCH AUTOSEL 5.10 8/9] powerpc/perf: Fix 32bit compile
From: Alexey Kardashevskiy [ Upstream commit bb82c574691daf8f7fa9a160264d15c5804cb769 ] The "read_bhrb" global symbol is only called under CONFIG_PPC64 of arch/powerpc/perf/core-book3s.c but it is compiled for both 32 and 64 bit anyway (and LLVM fails to link this on 32bit). This fixes it by moving bhrb.o to obj64 targets. Signed-off-by: Alexey Kardashevskiy Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/20220421025756.571995-1-...@ozlabs.ru Signed-off-by: Sasha Levin --- arch/powerpc/perf/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile index c02854dea2b2..da9f60ede97b 100644 --- a/arch/powerpc/perf/Makefile +++ b/arch/powerpc/perf/Makefile @@ -5,11 +5,11 @@ ifdef CONFIG_COMPAT obj-$(CONFIG_PERF_EVENTS) += callchain_32.o endif -obj-$(CONFIG_PPC_PERF_CTRS)+= core-book3s.o bhrb.o +obj-$(CONFIG_PPC_PERF_CTRS)+= core-book3s.o obj64-$(CONFIG_PPC_PERF_CTRS) += ppc970-pmu.o power5-pmu.o \ power5+-pmu.o power6-pmu.o power7-pmu.o \ isa207-common.o power8-pmu.o power9-pmu.o \ - generic-compat-pmu.o power10-pmu.o + generic-compat-pmu.o power10-pmu.o bhrb.o obj32-$(CONFIG_PPC_PERF_CTRS) += mpc7450-pmu.o obj-$(CONFIG_PPC_POWERNV) += imc-pmu.o -- 2.35.1
[PATCH AUTOSEL 5.15 12/15] powerpc/perf: Fix 32bit compile
From: Alexey Kardashevskiy [ Upstream commit bb82c574691daf8f7fa9a160264d15c5804cb769 ] The "read_bhrb" global symbol is only called under CONFIG_PPC64 of arch/powerpc/perf/core-book3s.c but it is compiled for both 32 and 64 bit anyway (and LLVM fails to link this on 32bit). This fixes it by moving bhrb.o to obj64 targets. Signed-off-by: Alexey Kardashevskiy Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/20220421025756.571995-1-...@ozlabs.ru Signed-off-by: Sasha Levin --- arch/powerpc/perf/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile index 2f46e31c7612..4f53d0b97539 100644 --- a/arch/powerpc/perf/Makefile +++ b/arch/powerpc/perf/Makefile @@ -3,11 +3,11 @@ obj-y += callchain.o callchain_$(BITS).o perf_regs.o obj-$(CONFIG_COMPAT) += callchain_32.o -obj-$(CONFIG_PPC_PERF_CTRS)+= core-book3s.o bhrb.o +obj-$(CONFIG_PPC_PERF_CTRS)+= core-book3s.o obj64-$(CONFIG_PPC_PERF_CTRS) += ppc970-pmu.o power5-pmu.o \ power5+-pmu.o power6-pmu.o power7-pmu.o \ isa207-common.o power8-pmu.o power9-pmu.o \ - generic-compat-pmu.o power10-pmu.o + generic-compat-pmu.o power10-pmu.o bhrb.o obj32-$(CONFIG_PPC_PERF_CTRS) += mpc7450-pmu.o obj-$(CONFIG_PPC_POWERNV) += imc-pmu.o -- 2.35.1
[PATCH AUTOSEL 5.17 18/22] powerpc/perf: Fix 32bit compile
From: Alexey Kardashevskiy [ Upstream commit bb82c574691daf8f7fa9a160264d15c5804cb769 ] The "read_bhrb" global symbol is only called under CONFIG_PPC64 of arch/powerpc/perf/core-book3s.c but it is compiled for both 32 and 64 bit anyway (and LLVM fails to link this on 32bit). This fixes it by moving bhrb.o to obj64 targets. Signed-off-by: Alexey Kardashevskiy Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/20220421025756.571995-1-...@ozlabs.ru Signed-off-by: Sasha Levin --- arch/powerpc/perf/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile index 2f46e31c7612..4f53d0b97539 100644 --- a/arch/powerpc/perf/Makefile +++ b/arch/powerpc/perf/Makefile @@ -3,11 +3,11 @@ obj-y += callchain.o callchain_$(BITS).o perf_regs.o obj-$(CONFIG_COMPAT) += callchain_32.o -obj-$(CONFIG_PPC_PERF_CTRS)+= core-book3s.o bhrb.o +obj-$(CONFIG_PPC_PERF_CTRS)+= core-book3s.o obj64-$(CONFIG_PPC_PERF_CTRS) += ppc970-pmu.o power5-pmu.o \ power5+-pmu.o power6-pmu.o power7-pmu.o \ isa207-common.o power8-pmu.o power9-pmu.o \ - generic-compat-pmu.o power10-pmu.o + generic-compat-pmu.o power10-pmu.o bhrb.o obj32-$(CONFIG_PPC_PERF_CTRS) += mpc7450-pmu.o obj-$(CONFIG_PPC_POWERNV) += imc-pmu.o -- 2.35.1
[PATCH v6] PCI hotplug: rpaphp: Error out on busy status from get-sensor-state
When certain PHB HW failure causes phyp to recover PHB, it marks the PE
state as temporarily unavailable until recovery is complete. This also
triggers an EEH handler in Linux which needs to notify drivers, and perform
recovery. But before notifying the driver about the PCI error it uses
get_adapter_state()->get-sensor-state() operation of the hotplug_slot to
determine if the slot contains a device or not. if the slot is empty, the
recovery is skipped entirely.
However on certain PHB failures, the rtas call get-sensor-state() returns
extended busy error (9902) until PHB is recovered by phyp. Once PHB is
recovered, the get-sensor-state() returns success with correct presence
status. The RTAS call interface rtas_get_sensor() loops over the rtas call
on extended delay return code (9902) until the return value is either
success (0) or error (-1). This causes the EEH handler to get stuck for ~6
seconds before it could notify that the pci error has been detected and
stop any active operations. Hence with running I/O traffic, during this 6
seconds, the network driver continues its operation and hits a timeout
(netdev watchdog). On timeouts, network driver go into ffdc capture mode
and reset path assuming the PCI device is in fatal condition. This
sometimes causes EEH recovery to fail. This impacts the ssh connection and
leads to the system being inaccessible.
[52732.244731] DEBUG: ibm_read_slot_reset_state2()
[52732.244762] DEBUG: ret = 0, rets[0]=5, rets[1]=1, rets[2]=4000, rets[3]=>
[52732.244798] DEBUG: in eeh_slot_presence_check
[52732.244804] DEBUG: error state check
[52732.244807] DEBUG: Is slot hotpluggable
[52732.244810] DEBUG: hotpluggable ops ?
[52732.244953] DEBUG: Calling ops->get_adapter_status
[52732.244958] DEBUG: calling rpaphp_get_sensor_state
[52736.564262] [ cut here ]
[52736.564299] NETDEV WATCHDOG: enP64p1s0f3 (tg3): transmit queue 0 timed o>
[52736.564324] WARNING: CPU: 1442 PID: 0 at net/sched/sch_generic.c:478 dev>
[...]
[52736.564505] NIP [c0c32368] dev_watchdog+0x438/0x440
[52736.564513] LR [c0c32364] dev_watchdog+0x434/0x440
To avoid this issue, fix the pci hotplug driver (rpaphp) to return an error
if the slot presence state can not be detected immediately while PE is in
EEH recovery state. Current implementation uses rtas_get_sensor() API which
blocks the slot check state until rtas call returns success. Change
rpaphp_get_sensor_state() to invoke rtas_call(get-sensor-state) directly
only if the respective pe is in EEH recovery state, and take actions based
on rtas return status.
In normal cases (non-EEH case) rpaphp_get_sensor_state() will continue to
invoke rtas_get_sensor() as it was earlier with no change in existing
behavior.
Signed-off-by: Mahesh Salgaonkar
Reviewed-by: Nathan Lynch
---
Change in v6:
- Fixed typo's in the patch description as per review comments.
Change in v5:
- Fixup #define macros with parentheses around the values.
Change in V4:
- Error out on sensor busy only if pe is going through EEH recovery instead
of always error out.
Change in V3:
- Invoke rtas_call(get-sensor-state) directly from
rpaphp_get_sensor_state() directly and do special handling.
- See v2 at
https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-November/237336.html
Change in V2:
- Alternate approach to fix the EEH issue instead of delaying slot presence
check proposed at
https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-November/236956.html
Also refer:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-November/237027.html
---
drivers/pci/hotplug/rpaphp_pci.c | 100 +-
1 file changed, 97 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/hotplug/rpaphp_pci.c b/drivers/pci/hotplug/rpaphp_pci.c
index c380bdacd1466..e463e915a052a 100644
--- a/drivers/pci/hotplug/rpaphp_pci.c
+++ b/drivers/pci/hotplug/rpaphp_pci.c
@@ -18,12 +18,107 @@
#include "../pci.h"/* for pci_add_new_bus */
#include "rpaphp.h"
+/*
+ * RTAS call get-sensor-state(DR_ENTITY_SENSE) return values as per PAPR:
+ *-1: Hardware Error
+ *-2: RTAS_BUSY
+ *-3: Invalid sensor. RTAS Parameter Error.
+ * -9000: Need DR entity to be powered up and unisolated before RTAS call
+ * -9001: Need DR entity to be powered up, but not unisolated, before RTAS call
+ * -9002: DR entity unusable
+ * 990x: Extended delay - where x is a number in the range of 0-5
+ */
+#define RTAS_HARDWARE_ERROR(-1)
+#define RTAS_INVALID_SENSOR(-3)
+#define SLOT_UNISOLATED(-9000)
+#define SLOT_NOT_UNISOLATED(-9001)
+#define SLOT_NOT_USABLE(-9002)
+
+static int rtas_to_errno(int rtas_rc)
+{
+ int rc;
+
+ switch (rtas_rc) {
+ case RTAS_HARDWARE_ERROR:
+ rc = -EIO;
+ break;
+ case RTAS_INVALID_SENSOR:
+ rc = -EINVAL;
+ break;
+ case SLOT_UNISOLATED:
+ case SLOT_NOT_UNISOLATED:
Re: [PATCH v4 2/2] PCI/PM: Fix pci_pm_suspend_noirq() to disable PTM
On Mon, Apr 25, 2022 at 11:32:54AM -0700, David E. Box wrote:
> On Sat, 2022-04-23 at 10:01 -0500, Bjorn Helgaas wrote:
> > On Sat, Apr 23, 2022 at 12:43:14AM +, Jingar, Rajvi wrote:
> > > > -Original Message-
> > > > From: Bjorn Helgaas
> > > > On Thu, Apr 14, 2022 at 07:54:02PM +0200, Rafael J. Wysocki wrote:
> > > > > On 3/25/2022 8:50 PM, Rajvi Jingar wrote:
> > > > > > For the PCIe devices (like nvme) that do not go into D3 state still
> > > > > > need to
> > > > > > disable PTM on PCIe root ports to allow the port to enter a lower-
> > > > > > power PM
> > > > > > state and the SoC to reach a lower-power idle state as a whole. Move
> > > > > > the
> > > > > > pci_disable_ptm() out of pci_prepare_to_sleep() as this code path is
> > > > > > not
> > > > > > followed for devices that do not go into D3. This patch fixes the
> > > > > > issue
> > > > > > seen on Dell XPS 9300 with Ice Lake CPU and Dell Precision 5530 with
> > > > > > Coffee
> > > > > > Lake CPU platforms to get improved residency in low power idle
> > > > > > states.
> > > > > >
> > > > > > Fixes: a697f072f5da ("PCI: Disable PTM during suspend to save
> > > > > > power")
> > > > > > Signed-off-by: Rajvi Jingar
> > > > > > Suggested-by: David E. Box
> > > > > > ---
> > > > > > drivers/pci/pci-driver.c | 10 ++
> > > > > > drivers/pci/pci.c| 10 --
> > > > > > 2 files changed, 10 insertions(+), 10 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > > > > > index 8b55a90126a2..ab733374a260 100644
> > > > > > --- a/drivers/pci/pci-driver.c
> > > > > > +++ b/drivers/pci/pci-driver.c
> > > > > > @@ -847,6 +847,16 @@ static int pci_pm_suspend_noirq(struct device
> > > > > > *dev)
> > > > > > if (!pci_dev->state_saved) {
> > > > > > pci_save_state(pci_dev);
> > > > > > + /*
> > > > > > +* There are systems (for example, Intel mobile chips
> > > > > > since
> > > > Coffee
> > > > > > +* Lake) where the power drawn while suspended can be
> > > > significantly
> > > > > > +* reduced by disabling PTM on PCIe root ports as this
> > > > > > allows the
> > > > > > +* port to enter a lower-power PM state and the SoC to
> > > > > > reach a
> > > > > > +* lower-power idle state as a whole.
> > > > > > +*/
> > > > > > + if (pci_pcie_type(pci_dev) == PCI_EXP_TYPE_ROOT_PORT)
> > > > > > + pci_disable_ptm(pci_dev);
> > > >
> > > > Why is disabling PTM dependent on pci_dev->state_saved? The
> > > > point of this is to change the behavior of the device, and it
> > > > seems like we want to do that regardless of whether the driver
> > > > has used pci_save_state().
> > >
> > > Because we use the saved state to restore PTM on the root port.
> > > And it's under this condition that the root port state gets
> > > saved.
> >
> > Yes, I understand that pci_restore_ptm_state() depends on a
> > previous call to pci_save_ptm_state().
> >
> > The point I'm trying to make is that pci_disable_ptm() changes the
> > state of the device, and that state change should not depend on
> > whether the driver has used pci_save_state().
>
> We do it here because D3 depends on whether the device state was
> saved by the driver.
>
> if (!pci_dev->state_saved) {
> pci_save_state(pci_dev);
>
> /* disable PTM here */
>
> if (pci_power_manageable(pci_dev))
> pci_prepare_to_sleep(pci_dev);
> }
>
>
> If we disable PTM before the check, we will have saved "PTM
> disabled" as the restore state. And we can't do it after the check
> as the device will be in D3.
Are you suggesting that PTM should be left enabled if the driver
called pci_save_state(), but disabled otherwise? I don't see the
rationale for that.
I don't understand all the paths through pci_pm_suspend_noirq() (e.g.,
skip_bus_pm), but for this one, I think we could do something like
this:
driver_saved = pci_dev->state_saved;
if (!driver_saved)
pci_save_state(pci_dev);
pci_disable_ptm(pci_dev);
if (!driver_saved) {
if (pci_power_manageable(pci_dev))
pci_prepare_to_sleep(pci_dev);
}
Or I guess one could argue that a driver calling pci_save_state() is
implicitly taking responsibility for all PCI-related suspend work, and
it should be disabling PTM itself. But that doesn't really seem
maintainable.
> As to disabling PTM on all devices, I see no problem with this, but the
> reasoning is different. We disabled the root port PTM for power savings.
The power saving is good. I'm trying to make the argument that we
need to disable PTM on all devices for correctness.
If we disable PTM on the root port, are we guaranteed that it will
never receive a PTM Request from a downstream device? Per PCIe r6.0,
sec 6.21.3, such a request would cause an Unsupported Request error.
I sort of expect that if we're
Re: [PATCH v5 05/10] serial: termbits: ADDRB to indicate 9th bit addressing mode
On Tue, 26 Apr 2022, Greg KH wrote: > On Tue, Apr 26, 2022 at 05:01:31PM +0300, Ilpo Järvinen wrote: > > > > ADDRB value is the same for all archs (it's just this octal vs hex > > notation I've followed as per the nearby defines within the same file > > which makes them look different). > > > > Should I perhaps add to my cleanup list conversion of all those octal ones > > to hex? > > Argh, yes, please, let's do that now, I totally missed that. Will let > us see how to unify them as well. Unifying them might turn out impractical, here's a rough idea now many copies ... | uniq -c finds for the defines (based on more aggressively cleaned up lines than the patch will have): 89 1 74 2 14 3 58 4 11 5 54 6 There just tends to be 1 or 2 archs which are different from the others. ...I'll send the actual octal-to-hex patch once the arch builds complete. -- i.
Re: [PATCH v2 1/2] dt-bindings: interrupt-controller: fsl, ls-extirq: convert to YAML
On 26/04/2022 09:28, Michael Walle wrote: > Am 2022-04-26 08:53, schrieb Krzysztof Kozlowski: >> On 25/04/2022 23:58, Michael Walle wrote: > + reg: > +maxItems: 1 > +description: > + Specifies the Interrupt Polarity Control Register (INTPCR) in > the > + SCFG or the External Interrupt Control Register (IRQCR) in > the > ISC. > + > + interrupt-map: >>> >>> btw. >>> >>> minItems: 12 >>> maxItems: 12 >>> >>> Isn't working here, is that expected? The validator seem to get the >>> count >>> of the elements of one tuple wrong. >>> >>> I.e. >>> arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dtb: >>> interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, >>> 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, >>> 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, >>> 8, >>> 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is >>> too >>> short >> >> >> Works for me (in different schema)... maybe update your dtschema? > > Just updated to the latest one. But I'm still getting the same errors. > > $ dt-validate -V > 2022.4 > > /home/mwalle/repos/b-linux-arm64/arch/arm64/boot/dts/freescale/fsl-ls1088a-qds.dtb: > > interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, > 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, > 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, 8, > 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is too > short > From schema: > /home/mwalle/repos/linux-mw/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml > > How is the length of one entry calculated? If you add maxItems to your original v2 binding example, it works. If you replace your example with ls1088a and use maxItems:12, it works. So maybe something is wrong in your modified patch (which we do not have so we cannot test it)? Best regards, Krzysztof
Re: [PATCH v5 05/10] serial: termbits: ADDRB to indicate 9th bit addressing mode
On Tue, Apr 26, 2022 at 05:01:31PM +0300, Ilpo Järvinen wrote:
> One additional thing below I missed on the first read.
>
> On Tue, 26 Apr 2022, Ilpo Järvinen wrote:
> > On Tue, 26 Apr 2022, Greg KH wrote:
> >
> > > On Tue, Apr 26, 2022 at 03:24:43PM +0300, Ilpo Järvinen wrote:
> > > > Add ADDRB to termbits to indicate 9th bit addressing mode. This change
> > > > is necessary for supporting devices with RS485 multipoint addressing
> > > > [*]. A later patch in the patch series adds support for Synopsys
> > > > Designware UART capable for 9th bit addressing mode. In this mode, 9th
> > > > bit is used to indicate an address (byte) within the communication
> > > > line. The 9th bit addressing mode is selected using ADDRB introduced by
> > > > an earlier patch.
> > > >
> > > > [*] Technically, RS485 is just an electronic spec and does not itself
> > > > specify the 9th bit addressing mode but 9th bit seems at least
> > > > "semi-standard" way to do addressing with RS485.
> > > >
> > > > Cc: linux-...@vger.kernel.org
> > > > Cc: Ivan Kokshaysky
> > > > Cc: Matt Turner
> > > > Cc: linux-al...@vger.kernel.org
> > > > Cc: Thomas Bogendoerfer
> > > > Cc: linux-m...@vger.kernel.org
> > > > Cc: "James E.J. Bottomley"
> > > > Cc: Helge Deller
> > > > Cc: linux-par...@vger.kernel.org
> > > > Cc: Michael Ellerman
> > > > Cc: Benjamin Herrenschmidt
> > > > Cc: Paul Mackerras
> > > > Cc: linuxppc-dev@lists.ozlabs.org
> > > > Cc: "David S. Miller"
> > > > Cc: sparcli...@vger.kernel.org
> > > > Cc: Arnd Bergmann
> > > > Cc: linux-a...@vger.kernel.org
> > > > Cc: linux-...@vger.kernel.org
> > > > Signed-off-by: Ilpo Järvinen
> > > > ---
>
> > > > #define CMSPAR0100 /* mark or space (stick)
> > > > parity */
> > > > #define CRTSCTS 0200 /* flow control */
> > > >
> > > > diff --git a/arch/powerpc/include/uapi/asm/termbits.h
> > > > b/arch/powerpc/include/uapi/asm/termbits.h
> > > > index ed18bc61f63d..c6a033732f39 100644
> > > > --- a/arch/powerpc/include/uapi/asm/termbits.h
> > > > +++ b/arch/powerpc/include/uapi/asm/termbits.h
> > > > @@ -171,6 +171,7 @@ struct ktermios {
> > > > #define HUPCL 0004
> > > >
> > > > #define CLOCAL 0010
> > > > +#define ADDRB 0040/* address bit */
> > > > #define CMSPAR 0100 /* mark or space (stick) parity
> > > > */
> > > > #define CRTSCTS 0200 /* flow control */
> > > >
> > > > diff --git a/arch/sparc/include/uapi/asm/termbits.h
> > > > b/arch/sparc/include/uapi/asm/termbits.h
> > > > index ce5ad5d0f105..5eb1d547b5c4 100644
> > > > --- a/arch/sparc/include/uapi/asm/termbits.h
> > > > +++ b/arch/sparc/include/uapi/asm/termbits.h
> > > > @@ -201,6 +201,7 @@ struct ktermios {
> > > > #define B350 0x1012
> > > > #define B400 0x1013 */
> > > > #define CIBAUD 0x100f /* input baud rate (not used) */
> > > > +#define ADDRB0x2000 /* address bit */
> > > > #define CMSPAR 0x4000 /* mark or space (stick) parity */
> > > > #define CRTSCTS 0x8000 /* flow control */
> > >
> > > Why all the different values? Can't we pick one and use it for all
> > > arches? Having these be different in different arches and userspace
> > > should not be a thing for new fields, right?
>
> ADDRB value is the same for all archs (it's just this octal vs hex
> notation I've followed as per the nearby defines within the same file
> which makes them look different).
>
> Should I perhaps add to my cleanup list conversion of all those octal ones
> to hex?
Argh, yes, please, let's do that now, I totally missed that. Will let
us see how to unify them as well.
thanks,
greg k-h
Re: [PATCH v5 05/10] serial: termbits: ADDRB to indicate 9th bit addressing mode
One additional thing below I missed on the first read.
On Tue, 26 Apr 2022, Ilpo Järvinen wrote:
> On Tue, 26 Apr 2022, Greg KH wrote:
>
> > On Tue, Apr 26, 2022 at 03:24:43PM +0300, Ilpo Järvinen wrote:
> > > Add ADDRB to termbits to indicate 9th bit addressing mode. This change
> > > is necessary for supporting devices with RS485 multipoint addressing
> > > [*]. A later patch in the patch series adds support for Synopsys
> > > Designware UART capable for 9th bit addressing mode. In this mode, 9th
> > > bit is used to indicate an address (byte) within the communication
> > > line. The 9th bit addressing mode is selected using ADDRB introduced by
> > > an earlier patch.
> > >
> > > [*] Technically, RS485 is just an electronic spec and does not itself
> > > specify the 9th bit addressing mode but 9th bit seems at least
> > > "semi-standard" way to do addressing with RS485.
> > >
> > > Cc: linux-...@vger.kernel.org
> > > Cc: Ivan Kokshaysky
> > > Cc: Matt Turner
> > > Cc: linux-al...@vger.kernel.org
> > > Cc: Thomas Bogendoerfer
> > > Cc: linux-m...@vger.kernel.org
> > > Cc: "James E.J. Bottomley"
> > > Cc: Helge Deller
> > > Cc: linux-par...@vger.kernel.org
> > > Cc: Michael Ellerman
> > > Cc: Benjamin Herrenschmidt
> > > Cc: Paul Mackerras
> > > Cc: linuxppc-dev@lists.ozlabs.org
> > > Cc: "David S. Miller"
> > > Cc: sparcli...@vger.kernel.org
> > > Cc: Arnd Bergmann
> > > Cc: linux-a...@vger.kernel.org
> > > Cc: linux-...@vger.kernel.org
> > > Signed-off-by: Ilpo Järvinen
> > > ---
> > > #define CMSPAR0100 /* mark or space (stick) parity
> > > */
> > > #define CRTSCTS 0200 /* flow control */
> > >
> > > diff --git a/arch/powerpc/include/uapi/asm/termbits.h
> > > b/arch/powerpc/include/uapi/asm/termbits.h
> > > index ed18bc61f63d..c6a033732f39 100644
> > > --- a/arch/powerpc/include/uapi/asm/termbits.h
> > > +++ b/arch/powerpc/include/uapi/asm/termbits.h
> > > @@ -171,6 +171,7 @@ struct ktermios {
> > > #define HUPCL0004
> > >
> > > #define CLOCAL 0010
> > > +#define ADDRB0040/* address bit */
> > > #define CMSPAR 0100 /* mark or space (stick) parity
> > > */
> > > #define CRTSCTS0200 /* flow control */
> > >
> > > diff --git a/arch/sparc/include/uapi/asm/termbits.h
> > > b/arch/sparc/include/uapi/asm/termbits.h
> > > index ce5ad5d0f105..5eb1d547b5c4 100644
> > > --- a/arch/sparc/include/uapi/asm/termbits.h
> > > +++ b/arch/sparc/include/uapi/asm/termbits.h
> > > @@ -201,6 +201,7 @@ struct ktermios {
> > > #define B350 0x1012
> > > #define B400 0x1013 */
> > > #define CIBAUD 0x100f /* input baud rate (not used) */
> > > +#define ADDRB 0x2000 /* address bit */
> > > #define CMSPAR 0x4000 /* mark or space (stick) parity */
> > > #define CRTSCTS0x8000 /* flow control */
> >
> > Why all the different values? Can't we pick one and use it for all
> > arches? Having these be different in different arches and userspace
> > should not be a thing for new fields, right?
ADDRB value is the same for all archs (it's just this octal vs hex
notation I've followed as per the nearby defines within the same file
which makes them look different).
Should I perhaps add to my cleanup list conversion of all those octal ones
to hex?
> > > diff --git a/drivers/char/pcmcia/synclink_cs.c
> > > b/drivers/char/pcmcia/synclink_cs.c
> > > index 78baba55a8b5..d179b9b57a25 100644
> > > --- a/drivers/char/pcmcia/synclink_cs.c
> > > +++ b/drivers/char/pcmcia/synclink_cs.c
> > > @@ -2287,6 +2287,8 @@ static void mgslpc_set_termios(struct tty_struct
> > > *tty, struct ktermios *old_term
> > > == RELEVANT_IFLAG(old_termios->c_iflag)))
> > > return;
> > >
> > > + tty->termios.c_cflag &= ~ADDRB;
> >
> > Having to do this for all drivers feels wrong. It isn't needed for any
> > other flag, right?
>
> My understanding is that it would be needed for other flags too, it's just
> that many drivers probably haven't cared enough. Some drivers certainly
> clear a few flags they don't support while others don't clear any but
> it's also challenging to determine it which flags which driver supports.
> How bad the impact is per flag varies.
>
> > That makes me really not like this change as it
> > feels very ackward and
> > yet-another-thing-a-serial-driver-has-to-remember.
>
> It would be nice to have some mask for supported bits per driver. But it
> will be challenging to add at this point and I'm far from sure I could get
> them right per driver even if carefully trying to.
>
> > And as you are wanting to pass this bit to userspace, where is that
> > documented?
>
> Ah, I probably should add it to driver-api/serial/driver.rst too but ADDRB
> is certainly mentioned alongside with other addressing mode documentation
> I wrote for the later changes in this series.
--
i.
Re: [PATCH v5 06/10] serial: General support for multipoint addresses
On Tue, Apr 26, 2022 at 04:36:49PM +0300, Ilpo Järvinen wrote:
> On Tue, 26 Apr 2022, Greg KH wrote:
>
> > On Tue, Apr 26, 2022 at 03:24:44PM +0300, Ilpo Järvinen wrote:
> > > Add generic support for serial multipoint addressing. Two new ioctls
> > > are added. TIOCSADDR is used to indicate the destination/receive
> > > address. TIOCGADDR returns the current address in use. The driver
> > > should implement set_addr and get_addr to support addressing mode.
> > >
> > > Adjust ADDRB clearing to happen only if driver does not provide
> > > set_addr (=the driver doesn't support address mode).
> > >
> > > This change is necessary for supporting devices with RS485 multipoint
> > > addressing [*]. A following patch in the patch series adds support for
> > > Synopsys Designware UART capable for 9th bit addressing mode. In this
> > > mode, 9th bit is used to indicate an address (byte) within the
> > > communication line. The 9th bit addressing mode is selected using ADDRB
> > > introduced by the previous patch.
> > >
> > > Transmit addresses / receiver filter are specified by setting the flags
> > > SER_ADDR_DEST and/or SER_ADDR_RECV. When the user supplies the transmit
> > > address, in the 9bit addressing mode it is sent out immediately with
> > > the 9th bit set to 1. After that, the subsequent normal data bytes are
> > > sent with 9th bit as 0 and they are intended to the device with the
> > > given address. It is up to receiver to enforce the filter using
> > > SER_ADDR_RECV. When userspace has supplied the receive address, the
> > > driver is expected to handle the matching of the address and only data
> > > with that address is forwarded to the user. Both SER_ADDR_DEST and
> > > SER_ADDR_RECV can be given at the same time in a single call if the
> > > addresses are the same.
> > >
> > > The user can clear the receive filter with SER_ADDR_RECV_CLEAR.
> > >
> > > [*] Technically, RS485 is just an electronic spec and does not itself
> > > specify the 9th bit addressing mode but 9th bit seems at least
> > > "semi-standard" way to do addressing with RS485.
> > >
> > > Cc: linux-...@vger.kernel.org
> > > Cc: Ivan Kokshaysky
> > > Cc: Matt Turner
> > > Cc: linux-al...@vger.kernel.org
> > > Cc: Thomas Bogendoerfer
> > > Cc: linux-m...@vger.kernel.org
> > > Cc: "James E.J. Bottomley"
> > > Cc: Helge Deller
> > > Cc: linux-par...@vger.kernel.org
> > > Cc: Michael Ellerman
> > > Cc: Benjamin Herrenschmidt
> > > Cc: Paul Mackerras
> > > Cc: linuxppc-dev@lists.ozlabs.org
> > > Cc: Yoshinori Sato
> > > Cc: Rich Felker
> > > Cc: linux...@vger.kernel.org
> > > Cc: "David S. Miller"
> > > Cc: sparcli...@vger.kernel.org
> > > Cc: Chris Zankel
> > > Cc: Max Filippov
> > > Cc: linux-xte...@linux-xtensa.org
> > > Cc: Arnd Bergmann
> > > Cc: linux-a...@vger.kernel.org
> > > Cc: linux-...@vger.kernel.org
> > > Signed-off-by: Ilpo Järvinen
> > > ---
>
> > > diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
> > > index fa6b16e5fdd8..8cb785ea7087 100644
> > > --- a/include/uapi/linux/serial.h
> > > +++ b/include/uapi/linux/serial.h
> > > @@ -149,4 +149,12 @@ struct serial_iso7816 {
> > > __u32 reserved[5];
> > > };
> > >
> > > +struct serial_addr {
> > > + __u32 flags;
> > > +#define SER_ADDR_RECV(1 << 0)
> > > +#define SER_ADDR_RECV_CLEAR (1 << 1)
> > > +#define SER_ADDR_DEST(1 << 2)
> >
> > You never check for invalid flags being sent to the kernel, which means
> > this api can never change in the future to add new flags :(
>
> Ok, so you mean the general level should to check
> if (...->flags & ~(SER_ADDR_FLAGS_ALL))
> return -EINVAL;
> ?
For any new kernel api you always have to ensure that no "extra" flags
or bits are set and reject it otherwise you can never add any more bits
or flags in the future. This should be in the Documentation/ directory
for how to add new ioctls somewhere.
> There's some code in the driver that detects invalid flag combinations
> (in 10/10) but I guess it doesn't satisfies what you're after. It is
> similar to how serial_rs485 flags is handled, that is, clearing flags it
> didn't handle (when it can) and returning -EINVAL for impossible
> combinations such as getting both RECV and DEST addr at the same time.
> I don't know if serial_rs485 flags is a good example at all, it certainly
> doesn't check whether bits are set where there's no flag defined.
>
> > And what about struct serial_rs485? Shouldn't that be used here
> > instead? Why do we need a new ioctl and structure?
>
> It is possible (Lukas already mentioned that option too). It just means
> this will be available only on RS485 which could well be enough but Andy
> mentioned he has in the past come across addressing mode also with some
> RS232 thing (he didn't remember details anymore and it could be
> insignificant for the real world of today).
This is rs485 so let's keep it attached to that. Lots of people do
Re: [PATCH v5 06/10] serial: General support for multipoint addresses
On Tue, 26 Apr 2022, Greg KH wrote:
> On Tue, Apr 26, 2022 at 03:24:44PM +0300, Ilpo Järvinen wrote:
> > Add generic support for serial multipoint addressing. Two new ioctls
> > are added. TIOCSADDR is used to indicate the destination/receive
> > address. TIOCGADDR returns the current address in use. The driver
> > should implement set_addr and get_addr to support addressing mode.
> >
> > Adjust ADDRB clearing to happen only if driver does not provide
> > set_addr (=the driver doesn't support address mode).
> >
> > This change is necessary for supporting devices with RS485 multipoint
> > addressing [*]. A following patch in the patch series adds support for
> > Synopsys Designware UART capable for 9th bit addressing mode. In this
> > mode, 9th bit is used to indicate an address (byte) within the
> > communication line. The 9th bit addressing mode is selected using ADDRB
> > introduced by the previous patch.
> >
> > Transmit addresses / receiver filter are specified by setting the flags
> > SER_ADDR_DEST and/or SER_ADDR_RECV. When the user supplies the transmit
> > address, in the 9bit addressing mode it is sent out immediately with
> > the 9th bit set to 1. After that, the subsequent normal data bytes are
> > sent with 9th bit as 0 and they are intended to the device with the
> > given address. It is up to receiver to enforce the filter using
> > SER_ADDR_RECV. When userspace has supplied the receive address, the
> > driver is expected to handle the matching of the address and only data
> > with that address is forwarded to the user. Both SER_ADDR_DEST and
> > SER_ADDR_RECV can be given at the same time in a single call if the
> > addresses are the same.
> >
> > The user can clear the receive filter with SER_ADDR_RECV_CLEAR.
> >
> > [*] Technically, RS485 is just an electronic spec and does not itself
> > specify the 9th bit addressing mode but 9th bit seems at least
> > "semi-standard" way to do addressing with RS485.
> >
> > Cc: linux-...@vger.kernel.org
> > Cc: Ivan Kokshaysky
> > Cc: Matt Turner
> > Cc: linux-al...@vger.kernel.org
> > Cc: Thomas Bogendoerfer
> > Cc: linux-m...@vger.kernel.org
> > Cc: "James E.J. Bottomley"
> > Cc: Helge Deller
> > Cc: linux-par...@vger.kernel.org
> > Cc: Michael Ellerman
> > Cc: Benjamin Herrenschmidt
> > Cc: Paul Mackerras
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: Yoshinori Sato
> > Cc: Rich Felker
> > Cc: linux...@vger.kernel.org
> > Cc: "David S. Miller"
> > Cc: sparcli...@vger.kernel.org
> > Cc: Chris Zankel
> > Cc: Max Filippov
> > Cc: linux-xte...@linux-xtensa.org
> > Cc: Arnd Bergmann
> > Cc: linux-a...@vger.kernel.org
> > Cc: linux-...@vger.kernel.org
> > Signed-off-by: Ilpo Järvinen
> > ---
> > diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
> > index fa6b16e5fdd8..8cb785ea7087 100644
> > --- a/include/uapi/linux/serial.h
> > +++ b/include/uapi/linux/serial.h
> > @@ -149,4 +149,12 @@ struct serial_iso7816 {
> > __u32 reserved[5];
> > };
> >
> > +struct serial_addr {
> > + __u32 flags;
> > +#define SER_ADDR_RECV (1 << 0)
> > +#define SER_ADDR_RECV_CLEAR(1 << 1)
> > +#define SER_ADDR_DEST (1 << 2)
>
> You never check for invalid flags being sent to the kernel, which means
> this api can never change in the future to add new flags :(
Ok, so you mean the general level should to check
if (...->flags & ~(SER_ADDR_FLAGS_ALL))
return -EINVAL;
?
There's some code in the driver that detects invalid flag combinations
(in 10/10) but I guess it doesn't satisfies what you're after. It is
similar to how serial_rs485 flags is handled, that is, clearing flags it
didn't handle (when it can) and returning -EINVAL for impossible
combinations such as getting both RECV and DEST addr at the same time.
I don't know if serial_rs485 flags is a good example at all, it certainly
doesn't check whether bits are set where there's no flag defined.
> And what about struct serial_rs485? Shouldn't that be used here
> instead? Why do we need a new ioctl and structure?
It is possible (Lukas already mentioned that option too). It just means
this will be available only on RS485 which could well be enough but Andy
mentioned he has in the past come across addressing mode also with some
RS232 thing (he didn't remember details anymore and it could be
insignificant for the real world of today).
--
i.
Re: [PATCH v5 05/10] serial: termbits: ADDRB to indicate 9th bit addressing mode
On Tue, 26 Apr 2022, Greg KH wrote:
> On Tue, Apr 26, 2022 at 03:24:43PM +0300, Ilpo Järvinen wrote:
> > Add ADDRB to termbits to indicate 9th bit addressing mode. This change
> > is necessary for supporting devices with RS485 multipoint addressing
> > [*]. A later patch in the patch series adds support for Synopsys
> > Designware UART capable for 9th bit addressing mode. In this mode, 9th
> > bit is used to indicate an address (byte) within the communication
> > line. The 9th bit addressing mode is selected using ADDRB introduced by
> > an earlier patch.
> >
> > [*] Technically, RS485 is just an electronic spec and does not itself
> > specify the 9th bit addressing mode but 9th bit seems at least
> > "semi-standard" way to do addressing with RS485.
> >
> > Cc: linux-...@vger.kernel.org
> > Cc: Ivan Kokshaysky
> > Cc: Matt Turner
> > Cc: linux-al...@vger.kernel.org
> > Cc: Thomas Bogendoerfer
> > Cc: linux-m...@vger.kernel.org
> > Cc: "James E.J. Bottomley"
> > Cc: Helge Deller
> > Cc: linux-par...@vger.kernel.org
> > Cc: Michael Ellerman
> > Cc: Benjamin Herrenschmidt
> > Cc: Paul Mackerras
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: "David S. Miller"
> > Cc: sparcli...@vger.kernel.org
> > Cc: Arnd Bergmann
> > Cc: linux-a...@vger.kernel.org
> > Cc: linux-...@vger.kernel.org
> > Signed-off-by: Ilpo Järvinen
> > ---
> > arch/alpha/include/uapi/asm/termbits.h | 1 +
> > arch/mips/include/uapi/asm/termbits.h| 1 +
> > arch/parisc/include/uapi/asm/termbits.h | 1 +
> > arch/powerpc/include/uapi/asm/termbits.h | 1 +
> > arch/sparc/include/uapi/asm/termbits.h | 1 +
> > drivers/char/pcmcia/synclink_cs.c| 2 ++
> > drivers/ipack/devices/ipoctal.c | 2 ++
> > drivers/mmc/core/sdio_uart.c | 2 ++
> > drivers/net/usb/hso.c| 3 ++-
> > drivers/s390/char/tty3270.c | 3 +++
> > drivers/staging/greybus/uart.c | 2 ++
> > drivers/tty/amiserial.c | 6 +-
> > drivers/tty/moxa.c | 1 +
> > drivers/tty/mxser.c | 1 +
> > drivers/tty/serial/serial_core.c | 2 ++
> > drivers/tty/synclink_gt.c| 2 ++
> > drivers/tty/tty_ioctl.c | 2 ++
> > drivers/usb/class/cdc-acm.c | 2 ++
> > drivers/usb/serial/usb-serial.c | 6 --
> > include/uapi/asm-generic/termbits.h | 1 +
> > net/bluetooth/rfcomm/tty.c | 2 ++
> > 21 files changed, 40 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/alpha/include/uapi/asm/termbits.h
> > b/arch/alpha/include/uapi/asm/termbits.h
> > index 4575ba34a0ea..0c123e715486 100644
> > --- a/arch/alpha/include/uapi/asm/termbits.h
> > +++ b/arch/alpha/include/uapi/asm/termbits.h
> > @@ -180,6 +180,7 @@ struct ktermios {
> > #define HUPCL 0004
> >
> > #define CLOCAL 0010
> > +#define ADDRB 0040/* address bit */
> > #define CMSPAR 0100 /* mark or space (stick) parity
> > */
> > #define CRTSCTS 0200 /* flow control */
> >
> > diff --git a/arch/mips/include/uapi/asm/termbits.h
> > b/arch/mips/include/uapi/asm/termbits.h
> > index dfeffba729b7..4732d31b0e4e 100644
> > --- a/arch/mips/include/uapi/asm/termbits.h
> > +++ b/arch/mips/include/uapi/asm/termbits.h
> > @@ -182,6 +182,7 @@ struct ktermios {
> > #define B350 0010016
> > #define B400 0010017
> > #define CIBAUD 00200360 /* input baud rate */
> > +#define ADDRB0040 /* address bit */
> > #define CMSPAR 0100 /* mark or space (stick) parity */
> > #define CRTSCTS 0200 /* flow control */
> >
> > diff --git a/arch/parisc/include/uapi/asm/termbits.h
> > b/arch/parisc/include/uapi/asm/termbits.h
> > index 40e920f8d683..d6bbd10d92ba 100644
> > --- a/arch/parisc/include/uapi/asm/termbits.h
> > +++ b/arch/parisc/include/uapi/asm/termbits.h
> > @@ -159,6 +159,7 @@ struct ktermios {
> > #define B350 0010016
> > #define B400 0010017
> > #define CIBAUD00200360 /* input baud rate */
> > +#define ADDRB0040 /* address bit */
>
> tabs where the rest were not?
>
> > #define CMSPAR0100 /* mark or space (stick) parity */
> > #define CRTSCTS 0200 /* flow control */
> >
> > diff --git a/arch/powerpc/include/uapi/asm/termbits.h
> > b/arch/powerpc/include/uapi/asm/termbits.h
> > index ed18bc61f63d..c6a033732f39 100644
> > --- a/arch/powerpc/include/uapi/asm/termbits.h
> > +++ b/arch/powerpc/include/uapi/asm/termbits.h
> > @@ -171,6 +171,7 @@ struct ktermios {
> > #define HUPCL 0004
> >
> > #define CLOCAL 0010
> > +#define ADDRB 0040/* address bit */
> > #define CMSPAR 0100 /* mark or space (stick) parity
> > */
> > #define CRTSCTS 0200 /* flow control */
> >
> >
Re: [PATCH v5 06/10] serial: General support for multipoint addresses
On Tue, Apr 26, 2022 at 03:24:44PM +0300, Ilpo Järvinen wrote: > Add generic support for serial multipoint addressing. Two new ioctls > are added. TIOCSADDR is used to indicate the destination/receive > address. TIOCGADDR returns the current address in use. The driver > should implement set_addr and get_addr to support addressing mode. > > Adjust ADDRB clearing to happen only if driver does not provide > set_addr (=the driver doesn't support address mode). > > This change is necessary for supporting devices with RS485 multipoint > addressing [*]. A following patch in the patch series adds support for > Synopsys Designware UART capable for 9th bit addressing mode. In this > mode, 9th bit is used to indicate an address (byte) within the > communication line. The 9th bit addressing mode is selected using ADDRB > introduced by the previous patch. > > Transmit addresses / receiver filter are specified by setting the flags > SER_ADDR_DEST and/or SER_ADDR_RECV. When the user supplies the transmit > address, in the 9bit addressing mode it is sent out immediately with > the 9th bit set to 1. After that, the subsequent normal data bytes are > sent with 9th bit as 0 and they are intended to the device with the > given address. It is up to receiver to enforce the filter using > SER_ADDR_RECV. When userspace has supplied the receive address, the > driver is expected to handle the matching of the address and only data > with that address is forwarded to the user. Both SER_ADDR_DEST and > SER_ADDR_RECV can be given at the same time in a single call if the > addresses are the same. > > The user can clear the receive filter with SER_ADDR_RECV_CLEAR. > > [*] Technically, RS485 is just an electronic spec and does not itself > specify the 9th bit addressing mode but 9th bit seems at least > "semi-standard" way to do addressing with RS485. > > Cc: linux-...@vger.kernel.org > Cc: Ivan Kokshaysky > Cc: Matt Turner > Cc: linux-al...@vger.kernel.org > Cc: Thomas Bogendoerfer > Cc: linux-m...@vger.kernel.org > Cc: "James E.J. Bottomley" > Cc: Helge Deller > Cc: linux-par...@vger.kernel.org > Cc: Michael Ellerman > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: linuxppc-dev@lists.ozlabs.org > Cc: Yoshinori Sato > Cc: Rich Felker > Cc: linux...@vger.kernel.org > Cc: "David S. Miller" > Cc: sparcli...@vger.kernel.org > Cc: Chris Zankel > Cc: Max Filippov > Cc: linux-xte...@linux-xtensa.org > Cc: Arnd Bergmann > Cc: linux-a...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Signed-off-by: Ilpo Järvinen > --- > .../driver-api/serial/serial-rs485.rst| 23 ++- > arch/alpha/include/uapi/asm/ioctls.h | 3 + > arch/mips/include/uapi/asm/ioctls.h | 3 + > arch/parisc/include/uapi/asm/ioctls.h | 3 + > arch/powerpc/include/uapi/asm/ioctls.h| 3 + > arch/sh/include/uapi/asm/ioctls.h | 3 + > arch/sparc/include/uapi/asm/ioctls.h | 3 + > arch/xtensa/include/uapi/asm/ioctls.h | 3 + > drivers/tty/serial/8250/8250_core.c | 2 + > drivers/tty/serial/serial_core.c | 62 ++- > drivers/tty/tty_io.c | 2 + > include/linux/serial_core.h | 6 ++ > include/uapi/asm-generic/ioctls.h | 3 + > include/uapi/linux/serial.h | 8 +++ > 14 files changed, 125 insertions(+), 2 deletions(-) > > diff --git a/Documentation/driver-api/serial/serial-rs485.rst > b/Documentation/driver-api/serial/serial-rs485.rst > index 6bc824f948f9..2f45f007fa5b 100644 > --- a/Documentation/driver-api/serial/serial-rs485.rst > +++ b/Documentation/driver-api/serial/serial-rs485.rst > @@ -95,7 +95,28 @@ RS485 Serial Communications > /* Error handling. See errno. */ > } > > -5. References > +5. Multipoint Addressing > + > + > + The Linux kernel provides serial_addr structure to handle addressing > within > + multipoint serial communications line such as RS485. 9th bit addressiong > mode > + is enabled by adding ADDRB flag in termios c_cflag. > + > + Serial core calls device specific set/get_addr in response to TIOCSADDR > and > + TIOCGADDR ioctls with a pointer to serial_addr. Destination and receive > + address can be specified using serial_addr flags field. Receive address > may > + also be cleared using flags. Once an address is set, the communication > + can occur only with the particular device and other peers are filtered > out. > + It is left up to the receiver side to enforce the filtering. > + > + Address flags: > + - SER_ADDR_RECV: Receive (filter) address. > + - SER_ADDR_RECV_CLEAR: Clear receive filter (only for TIOCSADDR). > + - SER_ADDR_DEST: Destination address. > + > + Note: not all devices supporting RS485 support multipoint addressing. > + > +6. References > = > > [1] include/uapi/linux/serial.h > diff --git
Re: [PATCH v5 05/10] serial: termbits: ADDRB to indicate 9th bit addressing mode
On Tue, Apr 26, 2022 at 03:24:43PM +0300, Ilpo Järvinen wrote:
> Add ADDRB to termbits to indicate 9th bit addressing mode. This change
> is necessary for supporting devices with RS485 multipoint addressing
> [*]. A later patch in the patch series adds support for Synopsys
> Designware UART capable for 9th bit addressing mode. In this mode, 9th
> bit is used to indicate an address (byte) within the communication
> line. The 9th bit addressing mode is selected using ADDRB introduced by
> an earlier patch.
>
> [*] Technically, RS485 is just an electronic spec and does not itself
> specify the 9th bit addressing mode but 9th bit seems at least
> "semi-standard" way to do addressing with RS485.
>
> Cc: linux-...@vger.kernel.org
> Cc: Ivan Kokshaysky
> Cc: Matt Turner
> Cc: linux-al...@vger.kernel.org
> Cc: Thomas Bogendoerfer
> Cc: linux-m...@vger.kernel.org
> Cc: "James E.J. Bottomley"
> Cc: Helge Deller
> Cc: linux-par...@vger.kernel.org
> Cc: Michael Ellerman
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: "David S. Miller"
> Cc: sparcli...@vger.kernel.org
> Cc: Arnd Bergmann
> Cc: linux-a...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Ilpo Järvinen
> ---
> arch/alpha/include/uapi/asm/termbits.h | 1 +
> arch/mips/include/uapi/asm/termbits.h| 1 +
> arch/parisc/include/uapi/asm/termbits.h | 1 +
> arch/powerpc/include/uapi/asm/termbits.h | 1 +
> arch/sparc/include/uapi/asm/termbits.h | 1 +
> drivers/char/pcmcia/synclink_cs.c| 2 ++
> drivers/ipack/devices/ipoctal.c | 2 ++
> drivers/mmc/core/sdio_uart.c | 2 ++
> drivers/net/usb/hso.c| 3 ++-
> drivers/s390/char/tty3270.c | 3 +++
> drivers/staging/greybus/uart.c | 2 ++
> drivers/tty/amiserial.c | 6 +-
> drivers/tty/moxa.c | 1 +
> drivers/tty/mxser.c | 1 +
> drivers/tty/serial/serial_core.c | 2 ++
> drivers/tty/synclink_gt.c| 2 ++
> drivers/tty/tty_ioctl.c | 2 ++
> drivers/usb/class/cdc-acm.c | 2 ++
> drivers/usb/serial/usb-serial.c | 6 --
> include/uapi/asm-generic/termbits.h | 1 +
> net/bluetooth/rfcomm/tty.c | 2 ++
> 21 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/arch/alpha/include/uapi/asm/termbits.h
> b/arch/alpha/include/uapi/asm/termbits.h
> index 4575ba34a0ea..0c123e715486 100644
> --- a/arch/alpha/include/uapi/asm/termbits.h
> +++ b/arch/alpha/include/uapi/asm/termbits.h
> @@ -180,6 +180,7 @@ struct ktermios {
> #define HUPCL0004
>
> #define CLOCAL 0010
> +#define ADDRB0040/* address bit */
> #define CMSPAR 0100 /* mark or space (stick) parity
> */
> #define CRTSCTS0200 /* flow control */
>
> diff --git a/arch/mips/include/uapi/asm/termbits.h
> b/arch/mips/include/uapi/asm/termbits.h
> index dfeffba729b7..4732d31b0e4e 100644
> --- a/arch/mips/include/uapi/asm/termbits.h
> +++ b/arch/mips/include/uapi/asm/termbits.h
> @@ -182,6 +182,7 @@ struct ktermios {
> #define B350 0010016
> #define B400 0010017
> #define CIBAUD 00200360 /* input baud rate */
> +#define ADDRB 0040 /* address bit */
> #define CMSPAR 0100 /* mark or space (stick) parity */
> #define CRTSCTS0200 /* flow control */
>
> diff --git a/arch/parisc/include/uapi/asm/termbits.h
> b/arch/parisc/include/uapi/asm/termbits.h
> index 40e920f8d683..d6bbd10d92ba 100644
> --- a/arch/parisc/include/uapi/asm/termbits.h
> +++ b/arch/parisc/include/uapi/asm/termbits.h
> @@ -159,6 +159,7 @@ struct ktermios {
> #define B350 0010016
> #define B400 0010017
> #define CIBAUD00200360 /* input baud rate */
> +#define ADDRB 0040 /* address bit */
tabs where the rest were not?
> #define CMSPAR0100 /* mark or space (stick) parity */
> #define CRTSCTS 0200 /* flow control */
>
> diff --git a/arch/powerpc/include/uapi/asm/termbits.h
> b/arch/powerpc/include/uapi/asm/termbits.h
> index ed18bc61f63d..c6a033732f39 100644
> --- a/arch/powerpc/include/uapi/asm/termbits.h
> +++ b/arch/powerpc/include/uapi/asm/termbits.h
> @@ -171,6 +171,7 @@ struct ktermios {
> #define HUPCL0004
>
> #define CLOCAL 0010
> +#define ADDRB0040/* address bit */
> #define CMSPAR 0100 /* mark or space (stick) parity
> */
> #define CRTSCTS0200 /* flow control */
>
> diff --git a/arch/sparc/include/uapi/asm/termbits.h
> b/arch/sparc/include/uapi/asm/termbits.h
> index ce5ad5d0f105..5eb1d547b5c4 100644
> --- a/arch/sparc/include/uapi/asm/termbits.h
> +++
Re: [PATCH] KVM: powerpc: remove extraneous asterisk from rm_host_ipi_action comment
On 4/26/22 14:47, Bagas Sanjaya wrote:
> Link: https://lore.kernel.org/linux-doc/202204252334.cd2isiii-...@intel.com/
> Reported-by: kernel test robot
> Cc: Suresh Warrier
> Cc: Paul Mackerras
> Cc: Anders Roxell
> Cc: Greg Kroah-Hartman
> Cc: Arnd Bergmann
> Cc: Segher Boessenkool
> Cc: Michael Ellerman
> Cc: Benjamin Herrenschmidt
> Cc: Nicholas Piggin
> Cc: Fabiano Rosas
> Cc: Paolo Bonzini
> Cc: Alexey Kardashevskiy
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: k...@vger.kernel.org
> Cc: sta...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Bagas Sanjaya
Oops, I forgot Fixes: 0c2a66062470cd ("KVM: PPC: Book3S HV: Host side kick VCPU
when poked by real-mode KVM")
tag.
--
An old man doll... just what I always wanted! - Clara
[PATCH v5 06/10] serial: General support for multipoint addresses
Add generic support for serial multipoint addressing. Two new ioctls are added. TIOCSADDR is used to indicate the destination/receive address. TIOCGADDR returns the current address in use. The driver should implement set_addr and get_addr to support addressing mode. Adjust ADDRB clearing to happen only if driver does not provide set_addr (=the driver doesn't support address mode). This change is necessary for supporting devices with RS485 multipoint addressing [*]. A following patch in the patch series adds support for Synopsys Designware UART capable for 9th bit addressing mode. In this mode, 9th bit is used to indicate an address (byte) within the communication line. The 9th bit addressing mode is selected using ADDRB introduced by the previous patch. Transmit addresses / receiver filter are specified by setting the flags SER_ADDR_DEST and/or SER_ADDR_RECV. When the user supplies the transmit address, in the 9bit addressing mode it is sent out immediately with the 9th bit set to 1. After that, the subsequent normal data bytes are sent with 9th bit as 0 and they are intended to the device with the given address. It is up to receiver to enforce the filter using SER_ADDR_RECV. When userspace has supplied the receive address, the driver is expected to handle the matching of the address and only data with that address is forwarded to the user. Both SER_ADDR_DEST and SER_ADDR_RECV can be given at the same time in a single call if the addresses are the same. The user can clear the receive filter with SER_ADDR_RECV_CLEAR. [*] Technically, RS485 is just an electronic spec and does not itself specify the 9th bit addressing mode but 9th bit seems at least "semi-standard" way to do addressing with RS485. Cc: linux-...@vger.kernel.org Cc: Ivan Kokshaysky Cc: Matt Turner Cc: linux-al...@vger.kernel.org Cc: Thomas Bogendoerfer Cc: linux-m...@vger.kernel.org Cc: "James E.J. Bottomley" Cc: Helge Deller Cc: linux-par...@vger.kernel.org Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: linuxppc-dev@lists.ozlabs.org Cc: Yoshinori Sato Cc: Rich Felker Cc: linux...@vger.kernel.org Cc: "David S. Miller" Cc: sparcli...@vger.kernel.org Cc: Chris Zankel Cc: Max Filippov Cc: linux-xte...@linux-xtensa.org Cc: Arnd Bergmann Cc: linux-a...@vger.kernel.org Cc: linux-...@vger.kernel.org Signed-off-by: Ilpo Järvinen --- .../driver-api/serial/serial-rs485.rst| 23 ++- arch/alpha/include/uapi/asm/ioctls.h | 3 + arch/mips/include/uapi/asm/ioctls.h | 3 + arch/parisc/include/uapi/asm/ioctls.h | 3 + arch/powerpc/include/uapi/asm/ioctls.h| 3 + arch/sh/include/uapi/asm/ioctls.h | 3 + arch/sparc/include/uapi/asm/ioctls.h | 3 + arch/xtensa/include/uapi/asm/ioctls.h | 3 + drivers/tty/serial/8250/8250_core.c | 2 + drivers/tty/serial/serial_core.c | 62 ++- drivers/tty/tty_io.c | 2 + include/linux/serial_core.h | 6 ++ include/uapi/asm-generic/ioctls.h | 3 + include/uapi/linux/serial.h | 8 +++ 14 files changed, 125 insertions(+), 2 deletions(-) diff --git a/Documentation/driver-api/serial/serial-rs485.rst b/Documentation/driver-api/serial/serial-rs485.rst index 6bc824f948f9..2f45f007fa5b 100644 --- a/Documentation/driver-api/serial/serial-rs485.rst +++ b/Documentation/driver-api/serial/serial-rs485.rst @@ -95,7 +95,28 @@ RS485 Serial Communications /* Error handling. See errno. */ } -5. References +5. Multipoint Addressing + + + The Linux kernel provides serial_addr structure to handle addressing within + multipoint serial communications line such as RS485. 9th bit addressiong mode + is enabled by adding ADDRB flag in termios c_cflag. + + Serial core calls device specific set/get_addr in response to TIOCSADDR and + TIOCGADDR ioctls with a pointer to serial_addr. Destination and receive + address can be specified using serial_addr flags field. Receive address may + also be cleared using flags. Once an address is set, the communication + can occur only with the particular device and other peers are filtered out. + It is left up to the receiver side to enforce the filtering. + + Address flags: + - SER_ADDR_RECV: Receive (filter) address. + - SER_ADDR_RECV_CLEAR: Clear receive filter (only for TIOCSADDR). + - SER_ADDR_DEST: Destination address. + + Note: not all devices supporting RS485 support multipoint addressing. + +6. References = [1] include/uapi/linux/serial.h diff --git a/arch/alpha/include/uapi/asm/ioctls.h b/arch/alpha/include/uapi/asm/ioctls.h index 971311605288..500cab3e1d6b 100644 --- a/arch/alpha/include/uapi/asm/ioctls.h +++ b/arch/alpha/include/uapi/asm/ioctls.h @@ -125,4 +125,7 @@ #define TIOCMIWAIT 0x545C /* wait for a change on serial input line(s) */ #define
[PATCH v5 05/10] serial: termbits: ADDRB to indicate 9th bit addressing mode
Add ADDRB to termbits to indicate 9th bit addressing mode. This change
is necessary for supporting devices with RS485 multipoint addressing
[*]. A later patch in the patch series adds support for Synopsys
Designware UART capable for 9th bit addressing mode. In this mode, 9th
bit is used to indicate an address (byte) within the communication
line. The 9th bit addressing mode is selected using ADDRB introduced by
an earlier patch.
[*] Technically, RS485 is just an electronic spec and does not itself
specify the 9th bit addressing mode but 9th bit seems at least
"semi-standard" way to do addressing with RS485.
Cc: linux-...@vger.kernel.org
Cc: Ivan Kokshaysky
Cc: Matt Turner
Cc: linux-al...@vger.kernel.org
Cc: Thomas Bogendoerfer
Cc: linux-m...@vger.kernel.org
Cc: "James E.J. Bottomley"
Cc: Helge Deller
Cc: linux-par...@vger.kernel.org
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: linuxppc-dev@lists.ozlabs.org
Cc: "David S. Miller"
Cc: sparcli...@vger.kernel.org
Cc: Arnd Bergmann
Cc: linux-a...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Ilpo Järvinen
---
arch/alpha/include/uapi/asm/termbits.h | 1 +
arch/mips/include/uapi/asm/termbits.h| 1 +
arch/parisc/include/uapi/asm/termbits.h | 1 +
arch/powerpc/include/uapi/asm/termbits.h | 1 +
arch/sparc/include/uapi/asm/termbits.h | 1 +
drivers/char/pcmcia/synclink_cs.c| 2 ++
drivers/ipack/devices/ipoctal.c | 2 ++
drivers/mmc/core/sdio_uart.c | 2 ++
drivers/net/usb/hso.c| 3 ++-
drivers/s390/char/tty3270.c | 3 +++
drivers/staging/greybus/uart.c | 2 ++
drivers/tty/amiserial.c | 6 +-
drivers/tty/moxa.c | 1 +
drivers/tty/mxser.c | 1 +
drivers/tty/serial/serial_core.c | 2 ++
drivers/tty/synclink_gt.c| 2 ++
drivers/tty/tty_ioctl.c | 2 ++
drivers/usb/class/cdc-acm.c | 2 ++
drivers/usb/serial/usb-serial.c | 6 --
include/uapi/asm-generic/termbits.h | 1 +
net/bluetooth/rfcomm/tty.c | 2 ++
21 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/arch/alpha/include/uapi/asm/termbits.h
b/arch/alpha/include/uapi/asm/termbits.h
index 4575ba34a0ea..0c123e715486 100644
--- a/arch/alpha/include/uapi/asm/termbits.h
+++ b/arch/alpha/include/uapi/asm/termbits.h
@@ -180,6 +180,7 @@ struct ktermios {
#define HUPCL 0004
#define CLOCAL 0010
+#define ADDRB 0040/* address bit */
#define CMSPAR 0100 /* mark or space (stick) parity */
#define CRTSCTS 0200 /* flow control */
diff --git a/arch/mips/include/uapi/asm/termbits.h
b/arch/mips/include/uapi/asm/termbits.h
index dfeffba729b7..4732d31b0e4e 100644
--- a/arch/mips/include/uapi/asm/termbits.h
+++ b/arch/mips/include/uapi/asm/termbits.h
@@ -182,6 +182,7 @@ struct ktermios {
#define B350 0010016
#define B400 0010017
#define CIBAUD 00200360 /* input baud rate */
+#define ADDRB0040 /* address bit */
#define CMSPAR 0100 /* mark or space (stick) parity */
#define CRTSCTS 0200 /* flow control */
diff --git a/arch/parisc/include/uapi/asm/termbits.h
b/arch/parisc/include/uapi/asm/termbits.h
index 40e920f8d683..d6bbd10d92ba 100644
--- a/arch/parisc/include/uapi/asm/termbits.h
+++ b/arch/parisc/include/uapi/asm/termbits.h
@@ -159,6 +159,7 @@ struct ktermios {
#define B350 0010016
#define B400 0010017
#define CIBAUD00200360 /* input baud rate */
+#define ADDRB0040 /* address bit */
#define CMSPAR0100 /* mark or space (stick) parity */
#define CRTSCTS 0200 /* flow control */
diff --git a/arch/powerpc/include/uapi/asm/termbits.h
b/arch/powerpc/include/uapi/asm/termbits.h
index ed18bc61f63d..c6a033732f39 100644
--- a/arch/powerpc/include/uapi/asm/termbits.h
+++ b/arch/powerpc/include/uapi/asm/termbits.h
@@ -171,6 +171,7 @@ struct ktermios {
#define HUPCL 0004
#define CLOCAL 0010
+#define ADDRB 0040/* address bit */
#define CMSPAR 0100 /* mark or space (stick) parity */
#define CRTSCTS 0200 /* flow control */
diff --git a/arch/sparc/include/uapi/asm/termbits.h
b/arch/sparc/include/uapi/asm/termbits.h
index ce5ad5d0f105..5eb1d547b5c4 100644
--- a/arch/sparc/include/uapi/asm/termbits.h
+++ b/arch/sparc/include/uapi/asm/termbits.h
@@ -201,6 +201,7 @@ struct ktermios {
#define B350 0x1012
#define B400 0x1013 */
#define CIBAUD 0x100f /* input baud rate (not used) */
+#define ADDRB0x2000 /* address bit */
#define CMSPAR 0x4000 /* mark or space (stick) parity */
#define CRTSCTS 0x8000 /* flow control */
diff --git a/drivers/char/pcmcia/synclink_cs.c
Re: [PATCH] KVM: powerpc: remove extraneous asterisk from rm_host_ipi_action comment
On Tue, Apr 26, 2022 at 02:47:51PM +0700, Bagas Sanjaya wrote: > kernel test robot reported kernel-doc warning for rm_host_ipi_action(): > > arch/powerpc/kvm/book3s_hv_rm_xics.c:887: warning: This comment > starts with '/**', but isn't a kernel-doc comment. Refer > Documentation/doc-guide/kernel-doc.rst > > Since the function is static, remove the extraneous (second) asterisk at > the head of function comment. > > Link: https://lore.kernel.org/linux-doc/202204252334.cd2isiii-...@intel.com/ > Reported-by: kernel test robot > Cc: Suresh Warrier > Cc: Paul Mackerras > Cc: Anders Roxell > Cc: Greg Kroah-Hartman > Cc: Arnd Bergmann > Cc: Segher Boessenkool Please do not Cc: me on random patches, I get enough mail already :-) Segher
serial hang in qemu-system-ppc64 -M pseries
When I cut and paste 80-ish characters of text into the Linux serial console, it reads 16 characters and stops. When I hit space, it reads another 16 characters, and if I keep at it will eventually catch up without losing data. If I type, every character shows up immediately. (On other qemu targets and kernels I can cut and paste an entire uuencoded binary and it goes through just fine in one go, but this target hangs with big pastes until I hit keys.) Is this a qemu-side bug, or a kernel-side bug? Kernel config attached (linux 5.18-rc3 or thereabouts), qemu invocation is: qemu-system-ppc64 -M pseries -vga none -nographic -no-reboot -m 256 -kernel vmlinux -initrd powerpc64leroot.cpio.gz -append "panic=1 HOST=powerpc64le console=hvc0" Rob linux-fullconfig.gz Description: application/gzip
[PATCH] KVM: powerpc: remove extraneous asterisk from rm_host_ipi_action comment
kernel test robot reported kernel-doc warning for rm_host_ipi_action(): arch/powerpc/kvm/book3s_hv_rm_xics.c:887: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst Since the function is static, remove the extraneous (second) asterisk at the head of function comment. Link: https://lore.kernel.org/linux-doc/202204252334.cd2isiii-...@intel.com/ Reported-by: kernel test robot Cc: Suresh Warrier Cc: Paul Mackerras Cc: Anders Roxell Cc: Greg Kroah-Hartman Cc: Arnd Bergmann Cc: Segher Boessenkool Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Nicholas Piggin Cc: Fabiano Rosas Cc: Paolo Bonzini Cc: Alexey Kardashevskiy Cc: linuxppc-dev@lists.ozlabs.org Cc: k...@vger.kernel.org Cc: sta...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Signed-off-by: Bagas Sanjaya --- arch/powerpc/kvm/book3s_hv_rm_xics.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/book3s_hv_rm_xics.c b/arch/powerpc/kvm/book3s_hv_rm_xics.c index 587c33fc45640f..6e16bd751c8423 100644 --- a/arch/powerpc/kvm/book3s_hv_rm_xics.c +++ b/arch/powerpc/kvm/book3s_hv_rm_xics.c @@ -883,7 +883,7 @@ long kvmppc_deliver_irq_passthru(struct kvm_vcpu *vcpu, /* --- Non-real mode XICS-related built-in routines --- */ -/** +/* * Host Operations poked by RM KVM */ static void rm_host_ipi_action(int action, void *data) base-commit: d615b5416f8a1afeb82d13b238f8152c572d59c0 -- An old man doll... just what I always wanted! - Clara
Re: [PATCH v2] tty/hvc_opal: simplify if-if to if-else
On 26. 04. 22, 9:10, Wan Jiabing wrote:
Use if and else instead of if(A) and if (!A).
Reviewed-by: Jiri Slaby
Signed-off-by: Wan Jiabing
---
Change log:
v2:
- add braces to the if block.
---
drivers/tty/hvc/hvc_opal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index 84776bc641e6..794c7b18aa06 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -342,9 +342,9 @@ void __init hvc_opal_init_early(void)
* path, so we hard wire it
*/
opal = of_find_node_by_path("/ibm,opal/consoles");
- if (opal)
+ if (opal) {
pr_devel("hvc_opal: Found consoles in new location\n");
- if (!opal) {
+ } else {
opal = of_find_node_by_path("/ibm,opal");
if (opal)
pr_devel("hvc_opal: "
--
js
suse labs
Re: [PATCH v2 1/2] dt-bindings: interrupt-controller: fsl, ls-extirq: convert to YAML
Am 2022-04-26 08:53, schrieb Krzysztof Kozlowski: On 25/04/2022 23:58, Michael Walle wrote: + reg: +maxItems: 1 +description: + Specifies the Interrupt Polarity Control Register (INTPCR) in the + SCFG or the External Interrupt Control Register (IRQCR) in the ISC. + + interrupt-map: btw. minItems: 12 maxItems: 12 Isn't working here, is that expected? The validator seem to get the count of the elements of one tuple wrong. I.e. arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dtb: interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, 8, 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is too short Works for me (in different schema)... maybe update your dtschema? Just updated to the latest one. But I'm still getting the same errors. $ dt-validate -V 2022.4 /home/mwalle/repos/b-linux-arm64/arch/arm64/boot/dts/freescale/fsl-ls1088a-qds.dtb: interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, 8, 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is too short From schema: /home/mwalle/repos/linux-mw/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml How is the length of one entry calculated? -michael
[PATCH v2] tty/hvc_opal: simplify if-if to if-else
Use if and else instead of if(A) and if (!A).
Signed-off-by: Wan Jiabing
---
Change log:
v2:
- add braces to the if block.
---
drivers/tty/hvc/hvc_opal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index 84776bc641e6..794c7b18aa06 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -342,9 +342,9 @@ void __init hvc_opal_init_early(void)
* path, so we hard wire it
*/
opal = of_find_node_by_path("/ibm,opal/consoles");
- if (opal)
+ if (opal) {
pr_devel("hvc_opal: Found consoles in new location\n");
- if (!opal) {
+ } else {
opal = of_find_node_by_path("/ibm,opal");
if (opal)
pr_devel("hvc_opal: "
--
2.35.1
Re: [PATCH v2 1/2] dt-bindings: interrupt-controller: fsl, ls-extirq: convert to YAML
On 25/04/2022 23:58, Michael Walle wrote: >>> + reg: >>> +maxItems: 1 >>> +description: >>> + Specifies the Interrupt Polarity Control Register (INTPCR) in >>> the >>> + SCFG or the External Interrupt Control Register (IRQCR) in the >>> ISC. >>> + >>> + interrupt-map: > > btw. > > minItems: 12 > maxItems: 12 > > Isn't working here, is that expected? The validator seem to get the > count > of the elements of one tuple wrong. > > I.e. > arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dtb: > interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, > 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, > 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, 8, > 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is too > short Works for me (in different schema)... maybe update your dtschema? > >>> +description: Specifies the mapping from external interrupts to >>> GIC interrupts. >>> + >>> + interrupt-map-mask: >>> +items: >>> + - const: 0x >> >> This looks highly permissive mask and should be instead defined per >> variant, for example (quickly looking at DTS): >> 0x7 for ls1021 >> 0xf for ls1043a and ls1088a > > Just that I understand it correctly, the result of the AND with that > mask is then looked up in the interrupt-map (the first entry there)? Yes, the child (first) interrupt specifier. Since address-cells are 0, this will be bit-AND with [0-5]. >> You might need to correct the DTS. Some confirmation from someone with >> datasheet would be good. > > According to their datasheets they have the following number of external > IRQs: > - ls1021a has 6, > - ls1043a has 12, > - ls1046a has 12, > - ls1088a has 12, > - ls2080a has 12, > - lx2160a has 12. > > That is what I need to confirm, right? Yes. > > Is there a better way than the following snippet: > > properties: >interrupt-map-mask: true > > allOf: >- if: >properties: > compatible: >contains: > enum: >- fsl,ls1021a-extirq > then: >properties: > interrupt-map-mask: >items: > - const: 0x7 > - const: 0 >- if: >properties: > compatible: >contains: > enum: >- fsl,ls1043a-extirq >- fsl,ls1046a-extirq >- fsl,ls1088a-extirq >- fsl,ls2080a-extirq >- fsl,lx2160a-extirq > then: >properties: > interrupt-map-mask: >items: > - const: 0xf > - const: 0 > Exactly like this, looks good. Thank you. Best regards, Krzysztof
[PATCH V2] selftests/powerpc/pmu: Fix unsigned function returning negative constant
The function __perf_reg_mask has an unsigned return type, but returns a
negative constant to indicate an error condition. So we change unsigned
to int.
Fixes: 5f6c3061af7c ("selftests/powerpc/pmu: Add utility functions to post
process the mmap buffer")
Signed-off-by: Haowen Bai
---
V1->V2: add fix tag.
tools/testing/selftests/powerpc/pmu/sampling_tests/misc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/misc.c
b/tools/testing/selftests/powerpc/pmu/sampling_tests/misc.c
index fca054bbc094..c01a31d5f4ee 100644
--- a/tools/testing/selftests/powerpc/pmu/sampling_tests/misc.c
+++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/misc.c
@@ -274,7 +274,7 @@ u64 *get_intr_regs(struct event *event, void *sample_buff)
return intr_regs;
}
-static const unsigned int __perf_reg_mask(const char *register_name)
+static const int __perf_reg_mask(const char *register_name)
{
if (!strcmp(register_name, "R0"))
return 0;
--
2.7.4
Re: [PATCH] selftests/powerpc/pmu: Fix unsigned function returning negative constant
On 4/24/22 13:56, Haowen Bai wrote:
> The function __perf_reg_mask has an unsigned return type, but returns a
> negative constant to indicate an error condition. So we change unsigned
> to int.
>
> Signed-off-by: Haowen Bai
> ---
> tools/testing/selftests/powerpc/pmu/sampling_tests/misc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/misc.c
> b/tools/testing/selftests/powerpc/pmu/sampling_tests/misc.c
> index fca054bbc094..c01a31d5f4ee 100644
> --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/misc.c
> +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/misc.c
> @@ -274,7 +274,7 @@ u64 *get_intr_regs(struct event *event, void *sample_buff)
> return intr_regs;
> }
>
> -static const unsigned int __perf_reg_mask(const char *register_name)
> +static const int __perf_reg_mask(const char *register_name)
> {
Hi Haowen,
Thanks for correcting it. Can you also add fix tag with corresponding
commit id details.
Other than that patch looks good to me.
Reviewed-by: Kajol Jain
Thanks,
Kajol Jain
> if (!strcmp(register_name, "R0"))
> return 0;
