mbox series

[v3,0/6] bus/memory: Add Baikal-T1 SoC APB/AXI/L2 drivers

Message ID 20200526125928.17096-1-Sergey.Semin@baikalelectronics.ru
Headers show
Series bus/memory: Add Baikal-T1 SoC APB/AXI/L2 drivers | expand

Message

Serge Semin May 26, 2020, 12:59 p.m. UTC
Baikal-T1 SoC CPU is based on two MIPS Warrior P5600 cores. Their main
memory Non-Coherent IO interface is connected to the OCP2AXI bridge, which
in turn is then connected to the DW AMBA 3 AXI Interconnect (so called
Main Interconnect) with nine masters and four slaves ports. Main
Interconnect is responsible for the AXI-bus traffic arbitration (QoS) and
its routing from one component to another. In addition there is a Errors
Handler Block (EHB) accesible by means of the Baikal-T1 SoC System
Controller responsible to detect AXI protocol errors and device not
responding situations built on top the interconnect. Baikal-T1 AXI-bus
driver included in this patchset will be responsible for working with that
functionality, though currently it doesn't support QoS tuning. Instead
it's capable of detecting the error events, reporting an info about them
to the system log, injecting artificial errors to test the driver
functionality. Since AXI Interconnect doesn't provide a way to find out
which devices are connected to it, so its DT node is supposed to be
compatible with "simple-bus" driver, while sub-nodes shall represent the
masters attached to the bus.

One of the AXI Interconnect slaves is an AXI-APB bridge used to access the
Baikal-T1 SoC subsystems CSRs. MMIO request from CPU and DMAC masters are
routed there if they are detected to be within [0x08000000 0x1FFFFFFF]
range of the physical memory. In case if an attempted APB transaction
stays with no response for a pre-defined time it will be detected by the
APB-bus Errors Handler Block (EHB), which will raise an interrupt, then
the bus gets freed for a next operation. The APB-bus driver provides the
interrupt handler to detect the erroneous address, update an errors
counter and prints an error message about the faulty address. The counter
and the APB-bus operations timeout can be accessed via corresponding sysfs
nodes. A dedicated sysfs-node can be also used to artificially cause the
bus errors described above. Since APB-bus is a platform bus, it doesn't
provide a way to detect slave devices connected to it, so similarly to the
AXI-bus it's also supposed to be compatible with "simple-bus" driver.

Aside from PCIe/SATA/DDR/I2C/EHB/CPU/reboot specific settings the
Baikal-T1 System Controller provides a MIPS P5600 CM2 L2-cache tuning
block. It is responsible for the setting up the Tag/Data/WS L2-to-RAM
latencies. The last small patch in this patchset provides a driver and
DT-schema-based binding for the described device. So that the latencies
can be tuned up by means of dedicated DT properties and sysfs nodes.

This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
0e698dfa2822 ("Linux 5.7-rc4")
tag: v5.7-rc4

Changelog v2 (AXI/APB bus):
- Assign dual GPL/BSD licenses to the bindings.
- Use single lined copyright headers in the bindings.
- Replace "additionalProperties: false" property with
  "unevaluatedProperties: false" in the bindings.
- Don't use a multi-arg clock phandle reference in DT binding examples.
  Thus remove includes from there.
- Fix some commit message and Kconfig help text spelling.
- Move drivers from soc to the bus subsystem.
- Convert a simple EHB drivers to the Baikal-T1 AXI and APB bus ones.
- Convert APB bus driver to using regmap MMIO API.
- Use syscon regmap to access the AXI-bus erroneous address.
- Add reset line support.
- Add Main Interconnect clock support to the AXI-bus driver.
- Remove probe-status info string printout.
- Discard of_match_ptr() macro utilization.
- Don't print error-message if no platform IRQ found. Just return an error.
- Use generic FIELD_{GET,PREP} macros instead of handwritten ones in the
  AXI-bus driver.

Changelog v2 (l2 driver):
- Fix some commit message and Kconfig help text spelling.
- Move the driver to the memory subsystem.
- Assign dual GPL/BSD license to the DT binding.
- Use single lined copyright header in the binding.
- Discard reg property and syscon compatible string.
- Move "allOf" restrictions to the root level of the properties.
- The DT node is supposed to be a child of the Baikal-T1 system controller
  node. So regmap will be fetched from there.
- Use generic FIELD_{GET,PREP} macro.
- Remove probe-status info string printout.
- Since the driver depends on the OF config we can remove of_match_ptr()
  macro utilization.

Changelog v3:
- Combine l2 and AXI/APB bus patches in a single patchset.
- Retrieve AXI-bus QoS registers by resource name "qos".
- Discard CONFIG_OF dependency since there is none at compile-time.
- Add syscon EHB registers range to the AXI-bus reg property as optional
  entry.
- Fix invalid of_property_read_u32() return value test in the l2-ctl
  driver.
- Get the reg property back into the l2-ctl DT bindings even though the
  driver is using the parental syscon regmap.
- The l2-ctl DT schema will live separately from the system controller,
  but the corresponding sub-node of the later DT schema will $ref this one.
- Set non-default latencies in the l2-ctl DT example.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Olof Johansson <olof@lixom.net>
Cc: linux-mips@vger.kernel.org
Cc: soc@kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Serge Semin (6):
  dt-bindings: bus: Add Baikal-T1 AXI-bus binding
  dt-bindings: bus: Add Baikal-T1 APB-bus binding
  dt-bindings: memory: Add Baikal-T1 L2-cache Control Block binding
  bus: Add Baikal-T1 AXI-bus driver
  bus: Add Baikal-T1 APB-bus driver
  memory: Add Baikal-T1 L2-cache Control Block driver

 .../bindings/bus/baikal,bt1-apb.yaml          |  90 ++++
 .../bindings/bus/baikal,bt1-axi.yaml          | 107 +++++
 .../memory-controllers/baikal,bt1-l2-ctl.yaml |  63 +++
 drivers/bus/Kconfig                           |  30 ++
 drivers/bus/Makefile                          |   2 +
 drivers/bus/bt1-apb.c                         | 421 ++++++++++++++++++
 drivers/bus/bt1-axi.c                         | 318 +++++++++++++
 drivers/memory/Kconfig                        |  11 +
 drivers/memory/Makefile                       |   1 +
 drivers/memory/bt1-l2-ctl.c                   | 322 ++++++++++++++
 10 files changed, 1365 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/baikal,bt1-apb.yaml
 create mode 100644 Documentation/devicetree/bindings/bus/baikal,bt1-axi.yaml
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
 create mode 100644 drivers/bus/bt1-apb.c
 create mode 100644 drivers/bus/bt1-axi.c
 create mode 100644 drivers/memory/bt1-l2-ctl.c

Comments

Serge Semin May 26, 2020, 1:08 p.m. UTC | #1
Rob,
Could you pay attention to this patchset? There is only one DT binding left
without your tag:
[PATCH v3 3/6] dt-bindings: memory: Add Baikal-T1 L2-cache Control Block binding
After we get it, Arnd will merge the series into the soc repository.

Thanks
-Sergey


On Tue, May 26, 2020 at 03:59:22PM +0300, Serge Semin wrote:
> Baikal-T1 SoC CPU is based on two MIPS Warrior P5600 cores. Their main
> memory Non-Coherent IO interface is connected to the OCP2AXI bridge, which
> in turn is then connected to the DW AMBA 3 AXI Interconnect (so called
> Main Interconnect) with nine masters and four slaves ports. Main
> Interconnect is responsible for the AXI-bus traffic arbitration (QoS) and
> its routing from one component to another. In addition there is a Errors
> Handler Block (EHB) accesible by means of the Baikal-T1 SoC System
> Controller responsible to detect AXI protocol errors and device not
> responding situations built on top the interconnect. Baikal-T1 AXI-bus
> driver included in this patchset will be responsible for working with that
> functionality, though currently it doesn't support QoS tuning. Instead
> it's capable of detecting the error events, reporting an info about them
> to the system log, injecting artificial errors to test the driver
> functionality. Since AXI Interconnect doesn't provide a way to find out
> which devices are connected to it, so its DT node is supposed to be
> compatible with "simple-bus" driver, while sub-nodes shall represent the
> masters attached to the bus.
> 
> One of the AXI Interconnect slaves is an AXI-APB bridge used to access the
> Baikal-T1 SoC subsystems CSRs. MMIO request from CPU and DMAC masters are
> routed there if they are detected to be within [0x08000000 0x1FFFFFFF]
> range of the physical memory. In case if an attempted APB transaction
> stays with no response for a pre-defined time it will be detected by the
> APB-bus Errors Handler Block (EHB), which will raise an interrupt, then
> the bus gets freed for a next operation. The APB-bus driver provides the
> interrupt handler to detect the erroneous address, update an errors
> counter and prints an error message about the faulty address. The counter
> and the APB-bus operations timeout can be accessed via corresponding sysfs
> nodes. A dedicated sysfs-node can be also used to artificially cause the
> bus errors described above. Since APB-bus is a platform bus, it doesn't
> provide a way to detect slave devices connected to it, so similarly to the
> AXI-bus it's also supposed to be compatible with "simple-bus" driver.
> 
> Aside from PCIe/SATA/DDR/I2C/EHB/CPU/reboot specific settings the
> Baikal-T1 System Controller provides a MIPS P5600 CM2 L2-cache tuning
> block. It is responsible for the setting up the Tag/Data/WS L2-to-RAM
> latencies. The last small patch in this patchset provides a driver and
> DT-schema-based binding for the described device. So that the latencies
> can be tuned up by means of dedicated DT properties and sysfs nodes.
> 
> This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
> 0e698dfa2822 ("Linux 5.7-rc4")
> tag: v5.7-rc4
> 
> Changelog v2 (AXI/APB bus):
> - Assign dual GPL/BSD licenses to the bindings.
> - Use single lined copyright headers in the bindings.
> - Replace "additionalProperties: false" property with
>   "unevaluatedProperties: false" in the bindings.
> - Don't use a multi-arg clock phandle reference in DT binding examples.
>   Thus remove includes from there.
> - Fix some commit message and Kconfig help text spelling.
> - Move drivers from soc to the bus subsystem.
> - Convert a simple EHB drivers to the Baikal-T1 AXI and APB bus ones.
> - Convert APB bus driver to using regmap MMIO API.
> - Use syscon regmap to access the AXI-bus erroneous address.
> - Add reset line support.
> - Add Main Interconnect clock support to the AXI-bus driver.
> - Remove probe-status info string printout.
> - Discard of_match_ptr() macro utilization.
> - Don't print error-message if no platform IRQ found. Just return an error.
> - Use generic FIELD_{GET,PREP} macros instead of handwritten ones in the
>   AXI-bus driver.
> 
> Changelog v2 (l2 driver):
> - Fix some commit message and Kconfig help text spelling.
> - Move the driver to the memory subsystem.
> - Assign dual GPL/BSD license to the DT binding.
> - Use single lined copyright header in the binding.
> - Discard reg property and syscon compatible string.
> - Move "allOf" restrictions to the root level of the properties.
> - The DT node is supposed to be a child of the Baikal-T1 system controller
>   node. So regmap will be fetched from there.
> - Use generic FIELD_{GET,PREP} macro.
> - Remove probe-status info string printout.
> - Since the driver depends on the OF config we can remove of_match_ptr()
>   macro utilization.
> 
> Changelog v3:
> - Combine l2 and AXI/APB bus patches in a single patchset.
> - Retrieve AXI-bus QoS registers by resource name "qos".
> - Discard CONFIG_OF dependency since there is none at compile-time.
> - Add syscon EHB registers range to the AXI-bus reg property as optional
>   entry.
> - Fix invalid of_property_read_u32() return value test in the l2-ctl
>   driver.
> - Get the reg property back into the l2-ctl DT bindings even though the
>   driver is using the parental syscon regmap.
> - The l2-ctl DT schema will live separately from the system controller,
>   but the corresponding sub-node of the later DT schema will $ref this one.
> - Set non-default latencies in the l2-ctl DT example.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
> Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
> Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
> Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
> Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: linux-mips@vger.kernel.org
> Cc: soc@kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> Serge Semin (6):
>   dt-bindings: bus: Add Baikal-T1 AXI-bus binding
>   dt-bindings: bus: Add Baikal-T1 APB-bus binding
>   dt-bindings: memory: Add Baikal-T1 L2-cache Control Block binding
>   bus: Add Baikal-T1 AXI-bus driver
>   bus: Add Baikal-T1 APB-bus driver
>   memory: Add Baikal-T1 L2-cache Control Block driver
> 
>  .../bindings/bus/baikal,bt1-apb.yaml          |  90 ++++
>  .../bindings/bus/baikal,bt1-axi.yaml          | 107 +++++
>  .../memory-controllers/baikal,bt1-l2-ctl.yaml |  63 +++
>  drivers/bus/Kconfig                           |  30 ++
>  drivers/bus/Makefile                          |   2 +
>  drivers/bus/bt1-apb.c                         | 421 ++++++++++++++++++
>  drivers/bus/bt1-axi.c                         | 318 +++++++++++++
>  drivers/memory/Kconfig                        |  11 +
>  drivers/memory/Makefile                       |   1 +
>  drivers/memory/bt1-l2-ctl.c                   | 322 ++++++++++++++
>  10 files changed, 1365 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/baikal,bt1-apb.yaml
>  create mode 100644 Documentation/devicetree/bindings/bus/baikal,bt1-axi.yaml
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
>  create mode 100644 drivers/bus/bt1-apb.c
>  create mode 100644 drivers/bus/bt1-axi.c
>  create mode 100644 drivers/memory/bt1-l2-ctl.c
> 
> -- 
> 2.26.2
>
Arnd Bergmann May 28, 2020, 12:14 p.m. UTC | #2
On Thu, May 28, 2020 at 12:01 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tuesday, May 26, 2020, Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote:
>>
>> AXI3-bus is the main communication bus connecting all high-speed
>> peripheral IP-cores with RAM controller and MIPS P5600 cores on Baikal-T1
>> SoC. Bus traffic arbitration is done by means of DW AMBA 3 AXI
>> Interconnect (so called AXI Main Interconnect) routing IO requests from
>> one SoC block to another. This driver provides a way to detect any bus
>> protocol errors and device not responding situations by means of an
>> embedded on top of the interconnect errors handler block (EHB). AXI
>> Interconnect QoS arbitration tuning is currently unsupported.
>> The bus doesn't provide a way to detect the interconnected devices,
>> so they are supposed to be statically defined like by means of the
>> simple-bus sub-nodes.
>
>
>
> Few comments in case if you need a new version. Main point is about sysfs_streq().

I've applied the patch now and folded in fixes for the build warnings and
errors pointed out by the test robot, but I did not include the changes you
suggested.

Serge, could you send a follow-up patch to address those?

     Arnd
Serge Semin May 28, 2020, 12:27 p.m. UTC | #3
On Thu, May 28, 2020 at 02:14:58PM +0200, Arnd Bergmann wrote:
> On Thu, May 28, 2020 at 12:01 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Tuesday, May 26, 2020, Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote:
> >>
> >> AXI3-bus is the main communication bus connecting all high-speed
> >> peripheral IP-cores with RAM controller and MIPS P5600 cores on Baikal-T1
> >> SoC. Bus traffic arbitration is done by means of DW AMBA 3 AXI
> >> Interconnect (so called AXI Main Interconnect) routing IO requests from
> >> one SoC block to another. This driver provides a way to detect any bus
> >> protocol errors and device not responding situations by means of an
> >> embedded on top of the interconnect errors handler block (EHB). AXI
> >> Interconnect QoS arbitration tuning is currently unsupported.
> >> The bus doesn't provide a way to detect the interconnected devices,
> >> so they are supposed to be statically defined like by means of the
> >> simple-bus sub-nodes.
> >
> >
> >
> > Few comments in case if you need a new version. Main point is about sysfs_streq().
> 
> I've applied the patch now and folded in fixes for the build warnings and
> errors pointed out by the test robot, but I did not include the changes you
> suggested.

Are you saying that the build-errors and warnings have already been fixed by
you, right? If so could you please give me a link to the repo with those
commits, so I'd work with the up-to-date code?

> 
> Serge, could you send a follow-up patch to address those?

Ok.

-Sergey

> 
>      Arnd
Serge Semin May 28, 2020, 12:37 p.m. UTC | #4
On Thu, May 28, 2020 at 02:14:58PM +0200, Arnd Bergmann wrote:
> On Thu, May 28, 2020 at 12:01 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Tuesday, May 26, 2020, Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote:
> >>
> >> AXI3-bus is the main communication bus connecting all high-speed
> >> peripheral IP-cores with RAM controller and MIPS P5600 cores on Baikal-T1
> >> SoC. Bus traffic arbitration is done by means of DW AMBA 3 AXI
> >> Interconnect (so called AXI Main Interconnect) routing IO requests from
> >> one SoC block to another. This driver provides a way to detect any bus
> >> protocol errors and device not responding situations by means of an
> >> embedded on top of the interconnect errors handler block (EHB). AXI
> >> Interconnect QoS arbitration tuning is currently unsupported.
> >> The bus doesn't provide a way to detect the interconnected devices,
> >> so they are supposed to be statically defined like by means of the
> >> simple-bus sub-nodes.
> >
> >
> >
> > Few comments in case if you need a new version. Main point is about sysfs_streq().
> 
> I've applied the patch now and folded in fixes for the build warnings and
> errors pointed out by the test robot, but I did not include the changes you
> suggested.

On the other hand if you haven't pushed the patches to the public repo yet,
I could just resend the series. So have you?

-Sergey

> 
> Serge, could you send a follow-up patch to address those?


> 
>      Arnd
Arnd Bergmann May 28, 2020, 12:44 p.m. UTC | #5
On Thu, May 28, 2020 at 2:27 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> On Thu, May 28, 2020 at 02:14:58PM +0200, Arnd Bergmann wrote:
> > On Thu, May 28, 2020 at 12:01 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Tuesday, May 26, 2020, Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote:
> > >>
> > >> AXI3-bus is the main communication bus connecting all high-speed
> > >> peripheral IP-cores with RAM controller and MIPS P5600 cores on Baikal-T1
> > >> SoC. Bus traffic arbitration is done by means of DW AMBA 3 AXI
> > >> Interconnect (so called AXI Main Interconnect) routing IO requests from
> > >> one SoC block to another. This driver provides a way to detect any bus
> > >> protocol errors and device not responding situations by means of an
> > >> embedded on top of the interconnect errors handler block (EHB). AXI
> > >> Interconnect QoS arbitration tuning is currently unsupported.
> > >> The bus doesn't provide a way to detect the interconnected devices,
> > >> so they are supposed to be statically defined like by means of the
> > >> simple-bus sub-nodes.
> > >
> > >
> > >
> > > Few comments in case if you need a new version. Main point is about sysfs_streq().
> >
> > I've applied the patch now and folded in fixes for the build warnings and
> > errors pointed out by the test robot, but I did not include the changes you
> > suggested.
>
> Are you saying that the build-errors and warnings have already been fixed by
> you, right? If so could you please give me a link to the repo with those
> commits, so I'd work with the up-to-date code?

I've pushed it to https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git/,

I made a local "baikal/drivers" branch with just your patches, updated
to address the build reports. This is merged into the "arm/drivers"
branch that contains all driver specific changes across all SoCs and
this is what I'll send to Linus next week.

There is also the "for-next" branch that contains all arm/* branches,
and this is what gets pulled into linux-next, so your patches will show
up there tomorrow as well.

You can normally check the status of any submission to soc@kernel.org
at https://patchwork.kernel.org/project/linux-soc/list/, but it seems that
has not picked up the status yet, and I'll have to update it manually.

       Arnd
Serge Semin May 28, 2020, 1 p.m. UTC | #6
On Thu, May 28, 2020 at 02:44:32PM +0200, Arnd Bergmann wrote:
> On Thu, May 28, 2020 at 2:27 PM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > On Thu, May 28, 2020 at 02:14:58PM +0200, Arnd Bergmann wrote:
> > > On Thu, May 28, 2020 at 12:01 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Tuesday, May 26, 2020, Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote:
> > > >>
> > > >> AXI3-bus is the main communication bus connecting all high-speed
> > > >> peripheral IP-cores with RAM controller and MIPS P5600 cores on Baikal-T1
> > > >> SoC. Bus traffic arbitration is done by means of DW AMBA 3 AXI
> > > >> Interconnect (so called AXI Main Interconnect) routing IO requests from
> > > >> one SoC block to another. This driver provides a way to detect any bus
> > > >> protocol errors and device not responding situations by means of an
> > > >> embedded on top of the interconnect errors handler block (EHB). AXI
> > > >> Interconnect QoS arbitration tuning is currently unsupported.
> > > >> The bus doesn't provide a way to detect the interconnected devices,
> > > >> so they are supposed to be statically defined like by means of the
> > > >> simple-bus sub-nodes.
> > > >
> > > >
> > > >
> > > > Few comments in case if you need a new version. Main point is about sysfs_streq().
> > >
> > > I've applied the patch now and folded in fixes for the build warnings and
> > > errors pointed out by the test robot, but I did not include the changes you
> > > suggested.
> >
> > Are you saying that the build-errors and warnings have already been fixed by
> > you, right? If so could you please give me a link to the repo with those
> > commits, so I'd work with the up-to-date code?
> 

> I've pushed it to https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git/,
> 
> I made a local "baikal/drivers" branch with just your patches, updated
> to address the build reports. This is merged into the "arm/drivers"
> branch that contains all driver specific changes across all SoCs and
> this is what I'll send to Linus next week.
> 
> There is also the "for-next" branch that contains all arm/* branches,
> and this is what gets pulled into linux-next, so your patches will show
> up there tomorrow as well.

I see. Thanks for explanation. I've monitored the soc/soc.git a few minutes
ago, but the gitolite web-interface showed me that the last update was made
9 hours ago. So I thought you could be using some another repo. Now I see
the fresh updates and the drivers being merges.

I'll address the Andy's comments and send a follow-up patches with fixes shortly
today.

BTW I don't see the next commit:
[PATCH v3 3/6] dt-bindings: memory: Add Baikal-T1 L2-cache Control Block binding
in the baikal/drivers, so the l2-ctl binding hasn't been merged into the
arm/drivers and for-next branches. You must have missed that patch. Could you
please make sure it's also merged in?

Thanks
-Sergey

> 
> You can normally check the status of any submission to soc@kernel.org
> at https://patchwork.kernel.org/project/linux-soc/list/, but it seems that
> has not picked up the status yet, and I'll have to update it manually.
> 
>        Arnd
Serge Semin May 28, 2020, 1:13 p.m. UTC | #7
On Thu, May 28, 2020 at 02:17:17PM +0200, Arnd Bergmann wrote:
> On Tue, May 26, 2020 at 2:59 PM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > Baikal-T1 AXI-APB bridge is used to access the SoC subsystem CSRs.
> > IO requests are routed to this bus by means of the DW AMBA 3 AXI
> > Interconnect. In case if an attempted APB transaction stays with no
> > response for a pre-defined time an interrupt occurs and the bus gets
> > freed for a next operation. This driver provides the interrupt handler
> > to detect the erroneous address, prints an error message about the
> > address fault, updates an errors counter. The counter and the APB-bus
> > operations timeout can be accessed via corresponding sysfs nodes.
> > A dedicated sysfs-node can be also used to artificially cause the
> > bus errors described above.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Olof Johansson <olof@lixom.net>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: linux-mips@vger.kernel.org
> > Cc: soc@kernel.org
> > Cc: devicetree@vger.kernel.org
> >
> > ---
> 
> Applied with this fixup:

I'm afraid linux/io.h is also needed here.(

-Sergey

> 
> --- a/drivers/bus/bt1-apb.c
> +++ b/drivers/bus/bt1-apb.c
> @@ -16,6 +16,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/interrupt.h>
>  #include <linux/nmi.h>
> +#include <linux/of.h>
>  #include <linux/regmap.h>
>  #include <linux/clk.h>
>  #include <linux/reset.h>
> @@ -309,13 +310,13 @@ static ssize_t timeout_store(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(timeout);
> 
> -static int inject_error_show(struct device *dev, struct device_attribute *attr,
> +static ssize_t inject_error_show(struct device *dev, struct
> device_attribute *attr,
>                              char *buf)
>  {
>         return scnprintf(buf, PAGE_SIZE, "Error injection: nodev irq\n");
>  }
> 
> -static int inject_error_store(struct device *dev,
> +static ssize_t inject_error_store(struct device *dev,
>                               struct device_attribute *attr,
>                               const char *data, size_t count)
>  {
Serge Semin May 28, 2020, 1:40 p.m. UTC | #8
On Thu, May 28, 2020 at 01:00:57AM +0300, Andy Shevchenko wrote:
> On Tuesday, May 26, 2020, Serge Semin <Sergey.Semin@baikalelectronics.ru>
> wrote:
> 
> > AXI3-bus is the main communication bus connecting all high-speed
> > peripheral IP-cores with RAM controller and MIPS P5600 cores on Baikal-T1
> > SoC. Bus traffic arbitration is done by means of DW AMBA 3 AXI
> > Interconnect (so called AXI Main Interconnect) routing IO requests from
> > one SoC block to another. This driver provides a way to detect any bus
> > protocol errors and device not responding situations by means of an
> > embedded on top of the interconnect errors handler block (EHB). AXI
> > Interconnect QoS arbitration tuning is currently unsupported.
> > The bus doesn't provide a way to detect the interconnected devices,
> > so they are supposed to be statically defined like by means of the
> > simple-bus sub-nodes.
> 
> 
> 
> Few comments in case if you need a new version. Main point is about
> sysfs_streq().

Hello, Andy. Thanks for your comments. I'll address most of them in a follow-up
patches. See just one note below.

> 
> 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Olof Johansson <olof@lixom.net>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: linux-mips@vger.kernel.org
> > Cc: soc@kernel.org
> > Cc: devicetree@vger.kernel.org
> >

[nip]

> > +
> > +static void bt1_axi_clear_data(void *data)
> > +{
> > +       struct bt1_axi *axi = data;
> > +       struct platform_device *pdev = to_platform_device(axi->dev);
> > +
> > +       platform_set_drvdata(pdev, NULL);
> 
> 

> Doesn't device driver core do this already?

It doesn't on remove. This cleanups the drvdata pointer when the driver is
unloaded at the moment of remove() callback calling. This is a good
practice to leave the device the same as it has been before usage including
the pointer value. In this case if theoretically someone (though very
unlikely, but anyway) would try to use the pointer without having it
initialized, the NULL-dereference would pop up, otherwise we may
corrupt someones memory, which is very nasty.

-Sergey
Andy Shevchenko May 28, 2020, 1:51 p.m. UTC | #9
On Thu, May 28, 2020 at 4:40 PM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> On Thu, May 28, 2020 at 01:00:57AM +0300, Andy Shevchenko wrote:
> > On Tuesday, May 26, 2020, Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > wrote:
> >
> > > AXI3-bus is the main communication bus connecting all high-speed
> > > peripheral IP-cores with RAM controller and MIPS P5600 cores on Baikal-T1
> > > SoC. Bus traffic arbitration is done by means of DW AMBA 3 AXI
> > > Interconnect (so called AXI Main Interconnect) routing IO requests from
> > > one SoC block to another. This driver provides a way to detect any bus
> > > protocol errors and device not responding situations by means of an
> > > embedded on top of the interconnect errors handler block (EHB). AXI
> > > Interconnect QoS arbitration tuning is currently unsupported.
> > > The bus doesn't provide a way to detect the interconnected devices,
> > > so they are supposed to be statically defined like by means of the
> > > simple-bus sub-nodes.
> >
> >
> >
> > Few comments in case if you need a new version. Main point is about
> > sysfs_streq().
>
> Hello, Andy. Thanks for your comments. I'll address most of them in a follow-up
> patches. See just one note below.
>
> >
> >
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > > Cc: Paul Burton <paulburton@kernel.org>
> > > Cc: Olof Johansson <olof@lixom.net>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: linux-mips@vger.kernel.org
> > > Cc: soc@kernel.org
> > > Cc: devicetree@vger.kernel.org
> > >
>
> [nip]
>
> > > +
> > > +static void bt1_axi_clear_data(void *data)
> > > +{
> > > +       struct bt1_axi *axi = data;
> > > +       struct platform_device *pdev = to_platform_device(axi->dev);
> > > +
> > > +       platform_set_drvdata(pdev, NULL);
> >
> >
>
> > Doesn't device driver core do this already?
>
> It doesn't on remove.

__device_release_driver() calls dev_set_drvdata(dev, NULL);
What did I miss?

> This cleanups the drvdata pointer when the driver is
> unloaded at the moment of remove() callback calling. This is a good
> practice to leave the device the same as it has been before usage including
> the pointer value. In this case if theoretically someone (though very
> unlikely, but anyway) would try to use the pointer without having it
> initialized, the NULL-dereference would pop up, otherwise we may
> corrupt someones memory, which is very nasty.

The above is right and I agree with.
Serge Semin May 28, 2020, 1:56 p.m. UTC | #10
On Thu, May 28, 2020 at 04:51:03PM +0300, Andy Shevchenko wrote:
> On Thu, May 28, 2020 at 4:40 PM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > On Thu, May 28, 2020 at 01:00:57AM +0300, Andy Shevchenko wrote:
> > > On Tuesday, May 26, 2020, Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > wrote:
> > >
> > > > AXI3-bus is the main communication bus connecting all high-speed
> > > > peripheral IP-cores with RAM controller and MIPS P5600 cores on Baikal-T1
> > > > SoC. Bus traffic arbitration is done by means of DW AMBA 3 AXI
> > > > Interconnect (so called AXI Main Interconnect) routing IO requests from
> > > > one SoC block to another. This driver provides a way to detect any bus
> > > > protocol errors and device not responding situations by means of an
> > > > embedded on top of the interconnect errors handler block (EHB). AXI
> > > > Interconnect QoS arbitration tuning is currently unsupported.
> > > > The bus doesn't provide a way to detect the interconnected devices,
> > > > so they are supposed to be statically defined like by means of the
> > > > simple-bus sub-nodes.
> > >
> > >
> > >
> > > Few comments in case if you need a new version. Main point is about
> > > sysfs_streq().
> >
> > Hello, Andy. Thanks for your comments. I'll address most of them in a follow-up
> > patches. See just one note below.
> >
> > >
> > >
> > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > > > Cc: Paul Burton <paulburton@kernel.org>
> > > > Cc: Olof Johansson <olof@lixom.net>
> > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > Cc: linux-mips@vger.kernel.org
> > > > Cc: soc@kernel.org
> > > > Cc: devicetree@vger.kernel.org
> > > >
> >
> > [nip]
> >
> > > > +
> > > > +static void bt1_axi_clear_data(void *data)
> > > > +{
> > > > +       struct bt1_axi *axi = data;
> > > > +       struct platform_device *pdev = to_platform_device(axi->dev);
> > > > +
> > > > +       platform_set_drvdata(pdev, NULL);
> > >
> > >
> >
> > > Doesn't device driver core do this already?
> >
> > It doesn't on remove.
> 

> __device_release_driver() calls dev_set_drvdata(dev, NULL);
> What did I miss?
> 
> > This cleanups the drvdata pointer when the driver is
> > unloaded at the moment of remove() callback calling. This is a good
> > practice to leave the device the same as it has been before usage including
> > the pointer value. In this case if theoretically someone (though very
> > unlikely, but anyway) would try to use the pointer without having it
> > initialized, the NULL-dereference would pop up, otherwise we may
> > corrupt someones memory, which is very nasty.
> 
> The above is right and I agree with.

Hm, good point. I missed that part of __device_release_driver(). So do you
suggest to remove that cleanup?

-Sergey

> 
> -- 
> With Best Regards,
> Andy Shevchenko