mbox series

[v4,00/14] Add Andes AE350 fdt driver support

Message ID 20221014003252.17765-1-peterlin@andestech.com
Headers show
Series Add Andes AE350 fdt driver support | expand

Message

Yu Chien Peter Lin Oct. 14, 2022, 12:32 a.m. UTC
We move the devices used by AE350 to fdt driver framework, reuse
existing driver and add timer, ipi and reset drivers.
The OpenSBI domain support is also enabled. As of now, we can use
Kconfig to manage these configurations that can support variants of
Andes platform and SoC like Kendryte K510.

Changes v1 -> v2
  - Rename Andes-specific devices (PLMT, PLICSW) compatible string
  - Update ISA string in CPU nodes
Changes v2 -> v3
  - Add PATCH1 and PATCH2 to fix typos
  - Add PATCH3 which factors out a function for adding mtimer and ipi
    device regions to root domain
  - Remove unused variable "num_src" and "riscv,ndev" property of ipi 
    device node
Changes v3 -> v4
  - Add PATCH3 to fix grammar
  - Move plmt_{cold,warm}_timer_init() to andes_plmt.c and define
    fdt_plmt_cold_timer_init() which adds PLMT region and sets
    timer device
  - Move plicsw_{cold,warm}_ipi_init() to andes_plicsw.c and define
    fdt_plicsw_cold_ipi_init() which adds PLICSW region and sets
    ipi device
 

Yu Chien Peter Lin (14):
  include: sbi: Fix typo in comment
  lib: sbi: Fix typo in comment
  include: sbi: Fix grammar in comment
  lib: sbi: Add sbi_domain_root_add_memrange() API
  platform: andes/ae350: Remove enabling cache from an350_final_init
  platform: andes/ae350: Use kconfig to set platform version and default
    name
  platform: andes/ae350: Use fdt serial driver
  lib: utils/timer: Add Andes fdt timer support
  lib: utils/reset: Add Andes fdt reset driver support
  platform: andes/ae350: Use fdt irqchip driver
  platform: andes/ae350: Add fw_platform_init for platform
    initialization
  lib: utils/ipi: Add Andes fdt ipi driver support
  platform: andes/ae350: Add AE350 domain support
  docs: andes-ae350.md: Update ae350 documentation for fdt driver
    support

 docs/platform/andes-ae350.md            | 184 +++++++++++++++++++++++-
 include/sbi/sbi_domain.h                |  16 ++-
 include/sbi/sbi_scratch.h               |   2 +-
 include/sbi_utils/fdt/fdt_helper.h      |   6 +
 include/sbi_utils/ipi/andes_plicsw.h    |  46 ++++++
 include/sbi_utils/timer/aclint_mtimer.h |   2 +
 include/sbi_utils/timer/andes_plmt.h    |  29 ++++
 lib/sbi/sbi_domain.c                    |  27 ++++
 lib/sbi/sbi_expected_trap.S             |   2 +-
 lib/utils/fdt/fdt_helper.c              | 108 ++++++++++++++
 lib/utils/ipi/Kconfig                   |   9 ++
 lib/utils/ipi/andes_plicsw.c            | 137 ++++++++++++++++++
 lib/utils/ipi/fdt_ipi_plicsw.c          |  47 ++++++
 lib/utils/ipi/objects.mk                |   4 +
 lib/utils/reset/Kconfig                 |   4 +
 lib/utils/reset/fdt_reset_atcwdt200.c   | 122 ++++++++++++++++
 lib/utils/reset/objects.mk              |   3 +
 lib/utils/timer/Kconfig                 |   9 ++
 lib/utils/timer/aclint_mtimer.c         |  50 ++-----
 lib/utils/timer/andes_plmt.c            | 104 ++++++++++++++
 lib/utils/timer/fdt_timer_plmt.c        |  51 +++++++
 lib/utils/timer/objects.mk              |   4 +
 platform/andes/ae350/Kconfig            |  30 +++-
 platform/andes/ae350/objects.mk         |   2 +-
 platform/andes/ae350/platform.c         | 165 +++++++++------------
 platform/andes/ae350/platform.h         |  17 ---
 platform/andes/ae350/plicsw.c           | 139 ------------------
 platform/andes/ae350/plicsw.h           |  44 ------
 platform/andes/ae350/plmt.c             | 107 --------------
 platform/andes/ae350/plmt.h             |  17 ---
 30 files changed, 1021 insertions(+), 466 deletions(-)
 create mode 100644 include/sbi_utils/ipi/andes_plicsw.h
 create mode 100644 include/sbi_utils/timer/andes_plmt.h
 create mode 100644 lib/utils/ipi/andes_plicsw.c
 create mode 100644 lib/utils/ipi/fdt_ipi_plicsw.c
 create mode 100644 lib/utils/reset/fdt_reset_atcwdt200.c
 create mode 100644 lib/utils/timer/andes_plmt.c
 create mode 100644 lib/utils/timer/fdt_timer_plmt.c
 delete mode 100644 platform/andes/ae350/plicsw.c
 delete mode 100644 platform/andes/ae350/plicsw.h
 delete mode 100644 platform/andes/ae350/plmt.c
 delete mode 100644 platform/andes/ae350/plmt.h

Comments

Anup Patel Oct. 23, 2022, 5:12 a.m. UTC | #1
On Fri, Oct 14, 2022 at 6:03 AM Yu Chien Peter Lin
<peterlin@andestech.com> wrote:
>
> We move the devices used by AE350 to fdt driver framework, reuse
> existing driver and add timer, ipi and reset drivers.
> The OpenSBI domain support is also enabled. As of now, we can use
> Kconfig to manage these configurations that can support variants of
> Andes platform and SoC like Kendryte K510.
>
> Changes v1 -> v2
>   - Rename Andes-specific devices (PLMT, PLICSW) compatible string
>   - Update ISA string in CPU nodes
> Changes v2 -> v3
>   - Add PATCH1 and PATCH2 to fix typos
>   - Add PATCH3 which factors out a function for adding mtimer and ipi
>     device regions to root domain
>   - Remove unused variable "num_src" and "riscv,ndev" property of ipi
>     device node
> Changes v3 -> v4
>   - Add PATCH3 to fix grammar
>   - Move plmt_{cold,warm}_timer_init() to andes_plmt.c and define
>     fdt_plmt_cold_timer_init() which adds PLMT region and sets
>     timer device
>   - Move plicsw_{cold,warm}_ipi_init() to andes_plicsw.c and define
>     fdt_plicsw_cold_ipi_init() which adds PLICSW region and sets
>     ipi device
>
>
> Yu Chien Peter Lin (14):
>   include: sbi: Fix typo in comment
>   lib: sbi: Fix typo in comment
>   include: sbi: Fix grammar in comment
>   lib: sbi: Add sbi_domain_root_add_memrange() API
>   platform: andes/ae350: Remove enabling cache from an350_final_init
>   platform: andes/ae350: Use kconfig to set platform version and default
>     name
>   platform: andes/ae350: Use fdt serial driver
>   lib: utils/timer: Add Andes fdt timer support
>   lib: utils/reset: Add Andes fdt reset driver support
>   platform: andes/ae350: Use fdt irqchip driver
>   platform: andes/ae350: Add fw_platform_init for platform
>     initialization
>   lib: utils/ipi: Add Andes fdt ipi driver support
>   platform: andes/ae350: Add AE350 domain support
>   docs: andes-ae350.md: Update ae350 documentation for fdt driver
>     support

Applied this series to the riscv/opensbi repo.

Any thoughts on using the generic platform ?

Thanks,
Anup

>
>  docs/platform/andes-ae350.md            | 184 +++++++++++++++++++++++-
>  include/sbi/sbi_domain.h                |  16 ++-
>  include/sbi/sbi_scratch.h               |   2 +-
>  include/sbi_utils/fdt/fdt_helper.h      |   6 +
>  include/sbi_utils/ipi/andes_plicsw.h    |  46 ++++++
>  include/sbi_utils/timer/aclint_mtimer.h |   2 +
>  include/sbi_utils/timer/andes_plmt.h    |  29 ++++
>  lib/sbi/sbi_domain.c                    |  27 ++++
>  lib/sbi/sbi_expected_trap.S             |   2 +-
>  lib/utils/fdt/fdt_helper.c              | 108 ++++++++++++++
>  lib/utils/ipi/Kconfig                   |   9 ++
>  lib/utils/ipi/andes_plicsw.c            | 137 ++++++++++++++++++
>  lib/utils/ipi/fdt_ipi_plicsw.c          |  47 ++++++
>  lib/utils/ipi/objects.mk                |   4 +
>  lib/utils/reset/Kconfig                 |   4 +
>  lib/utils/reset/fdt_reset_atcwdt200.c   | 122 ++++++++++++++++
>  lib/utils/reset/objects.mk              |   3 +
>  lib/utils/timer/Kconfig                 |   9 ++
>  lib/utils/timer/aclint_mtimer.c         |  50 ++-----
>  lib/utils/timer/andes_plmt.c            | 104 ++++++++++++++
>  lib/utils/timer/fdt_timer_plmt.c        |  51 +++++++
>  lib/utils/timer/objects.mk              |   4 +
>  platform/andes/ae350/Kconfig            |  30 +++-
>  platform/andes/ae350/objects.mk         |   2 +-
>  platform/andes/ae350/platform.c         | 165 +++++++++------------
>  platform/andes/ae350/platform.h         |  17 ---
>  platform/andes/ae350/plicsw.c           | 139 ------------------
>  platform/andes/ae350/plicsw.h           |  44 ------
>  platform/andes/ae350/plmt.c             | 107 --------------
>  platform/andes/ae350/plmt.h             |  17 ---
>  30 files changed, 1021 insertions(+), 466 deletions(-)
>  create mode 100644 include/sbi_utils/ipi/andes_plicsw.h
>  create mode 100644 include/sbi_utils/timer/andes_plmt.h
>  create mode 100644 lib/utils/ipi/andes_plicsw.c
>  create mode 100644 lib/utils/ipi/fdt_ipi_plicsw.c
>  create mode 100644 lib/utils/reset/fdt_reset_atcwdt200.c
>  create mode 100644 lib/utils/timer/andes_plmt.c
>  create mode 100644 lib/utils/timer/fdt_timer_plmt.c
>  delete mode 100644 platform/andes/ae350/plicsw.c
>  delete mode 100644 platform/andes/ae350/plicsw.h
>  delete mode 100644 platform/andes/ae350/plmt.c
>  delete mode 100644 platform/andes/ae350/plmt.h
>
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
Anup Patel Oct. 24, 2022, 8:10 a.m. UTC | #2
On Mon, Oct 24, 2022 at 12:43 PM Yu-Chien Peter Lin
<peterlin@andestech.com> wrote:
>
> On Sun, Oct 23, 2022 at 10:42:52AM +0530, Anup Patel wrote:
> > On Fri, Oct 14, 2022 at 6:03 AM Yu Chien Peter Lin
> > <peterlin@andestech.com> wrote:
> > >
> > > We move the devices used by AE350 to fdt driver framework, reuse
> > > existing driver and add timer, ipi and reset drivers.
> > > The OpenSBI domain support is also enabled. As of now, we can use
> > > Kconfig to manage these configurations that can support variants of
> > > Andes platform and SoC like Kendryte K510.
> > >
> > > Changes v1 -> v2
> > >   - Rename Andes-specific devices (PLMT, PLICSW) compatible string
> > >   - Update ISA string in CPU nodes
> > > Changes v2 -> v3
> > >   - Add PATCH1 and PATCH2 to fix typos
> > >   - Add PATCH3 which factors out a function for adding mtimer and ipi
> > >     device regions to root domain
> > >   - Remove unused variable "num_src" and "riscv,ndev" property of ipi
> > >     device node
> > > Changes v3 -> v4
> > >   - Add PATCH3 to fix grammar
> > >   - Move plmt_{cold,warm}_timer_init() to andes_plmt.c and define
> > >     fdt_plmt_cold_timer_init() which adds PLMT region and sets
> > >     timer device
> > >   - Move plicsw_{cold,warm}_ipi_init() to andes_plicsw.c and define
> > >     fdt_plicsw_cold_ipi_init() which adds PLICSW region and sets
> > >     ipi device
> > >
> > >
> > > Yu Chien Peter Lin (14):
> > >   include: sbi: Fix typo in comment
> > >   lib: sbi: Fix typo in comment
> > >   include: sbi: Fix grammar in comment
> > >   lib: sbi: Add sbi_domain_root_add_memrange() API
> > >   platform: andes/ae350: Remove enabling cache from an350_final_init
> > >   platform: andes/ae350: Use kconfig to set platform version and default
> > >     name
> > >   platform: andes/ae350: Use fdt serial driver
> > >   lib: utils/timer: Add Andes fdt timer support
> > >   lib: utils/reset: Add Andes fdt reset driver support
> > >   platform: andes/ae350: Use fdt irqchip driver
> > >   platform: andes/ae350: Add fw_platform_init for platform
> > >     initialization
> > >   lib: utils/ipi: Add Andes fdt ipi driver support
> > >   platform: andes/ae350: Add AE350 domain support
> > >   docs: andes-ae350.md: Update ae350 documentation for fdt driver
> > >     support
> >
> > Applied this series to the riscv/opensbi repo.
> >
> > Any thoughts on using the generic platform ?
> >
> > Thanks,
> > Anup
>
> Hi Anup,
>
> The FW_TEXT_START on AE350 is different from generic platform.
> Will it become a configuration?

The generic platform has FW_PIC enabled so you can load and
run generic firmwares from any address.

> I think we would keep the platform specific files in andes/ae350
> until we have to refactor the duplicate code.

You can add platform specific overrides to generic platforms which
are selected based on compatible string in the root DT node.

Regards,
Anup

>
> Thanks,
> Peter Lin
>
> >
> > >
> > >  docs/platform/andes-ae350.md            | 184 +++++++++++++++++++++++-
> > >  include/sbi/sbi_domain.h                |  16 ++-
> > >  include/sbi/sbi_scratch.h               |   2 +-
> > >  include/sbi_utils/fdt/fdt_helper.h      |   6 +
> > >  include/sbi_utils/ipi/andes_plicsw.h    |  46 ++++++
> > >  include/sbi_utils/timer/aclint_mtimer.h |   2 +
> > >  include/sbi_utils/timer/andes_plmt.h    |  29 ++++
> > >  lib/sbi/sbi_domain.c                    |  27 ++++
> > >  lib/sbi/sbi_expected_trap.S             |   2 +-
> > >  lib/utils/fdt/fdt_helper.c              | 108 ++++++++++++++
> > >  lib/utils/ipi/Kconfig                   |   9 ++
> > >  lib/utils/ipi/andes_plicsw.c            | 137 ++++++++++++++++++
> > >  lib/utils/ipi/fdt_ipi_plicsw.c          |  47 ++++++
> > >  lib/utils/ipi/objects.mk                |   4 +
> > >  lib/utils/reset/Kconfig                 |   4 +
> > >  lib/utils/reset/fdt_reset_atcwdt200.c   | 122 ++++++++++++++++
> > >  lib/utils/reset/objects.mk              |   3 +
> > >  lib/utils/timer/Kconfig                 |   9 ++
> > >  lib/utils/timer/aclint_mtimer.c         |  50 ++-----
> > >  lib/utils/timer/andes_plmt.c            | 104 ++++++++++++++
> > >  lib/utils/timer/fdt_timer_plmt.c        |  51 +++++++
> > >  lib/utils/timer/objects.mk              |   4 +
> > >  platform/andes/ae350/Kconfig            |  30 +++-
> > >  platform/andes/ae350/objects.mk         |   2 +-
> > >  platform/andes/ae350/platform.c         | 165 +++++++++------------
> > >  platform/andes/ae350/platform.h         |  17 ---
> > >  platform/andes/ae350/plicsw.c           | 139 ------------------
> > >  platform/andes/ae350/plicsw.h           |  44 ------
> > >  platform/andes/ae350/plmt.c             | 107 --------------
> > >  platform/andes/ae350/plmt.h             |  17 ---
> > >  30 files changed, 1021 insertions(+), 466 deletions(-)
> > >  create mode 100644 include/sbi_utils/ipi/andes_plicsw.h
> > >  create mode 100644 include/sbi_utils/timer/andes_plmt.h
> > >  create mode 100644 lib/utils/ipi/andes_plicsw.c
> > >  create mode 100644 lib/utils/ipi/fdt_ipi_plicsw.c
> > >  create mode 100644 lib/utils/reset/fdt_reset_atcwdt200.c
> > >  create mode 100644 lib/utils/timer/andes_plmt.c
> > >  create mode 100644 lib/utils/timer/fdt_timer_plmt.c
> > >  delete mode 100644 platform/andes/ae350/plicsw.c
> > >  delete mode 100644 platform/andes/ae350/plicsw.h
> > >  delete mode 100644 platform/andes/ae350/plmt.c
> > >  delete mode 100644 platform/andes/ae350/plmt.h
> > >
> > > --
> > > 2.34.1
> > >
> > >
> > > --
> > > opensbi mailing list
> > > opensbi@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/opensbi
Yu Chien Peter Lin Oct. 24, 2022, 3:10 p.m. UTC | #3
On Sun, Oct 23, 2022 at 10:42:52AM +0530, Anup Patel wrote:
> On Fri, Oct 14, 2022 at 6:03 AM Yu Chien Peter Lin
> <peterlin@andestech.com> wrote:
> >
> > We move the devices used by AE350 to fdt driver framework, reuse
> > existing driver and add timer, ipi and reset drivers.
> > The OpenSBI domain support is also enabled. As of now, we can use
> > Kconfig to manage these configurations that can support variants of
> > Andes platform and SoC like Kendryte K510.
> >
> > Changes v1 -> v2
> >   - Rename Andes-specific devices (PLMT, PLICSW) compatible string
> >   - Update ISA string in CPU nodes
> > Changes v2 -> v3
> >   - Add PATCH1 and PATCH2 to fix typos
> >   - Add PATCH3 which factors out a function for adding mtimer and ipi
> >     device regions to root domain
> >   - Remove unused variable "num_src" and "riscv,ndev" property of ipi
> >     device node
> > Changes v3 -> v4
> >   - Add PATCH3 to fix grammar
> >   - Move plmt_{cold,warm}_timer_init() to andes_plmt.c and define
> >     fdt_plmt_cold_timer_init() which adds PLMT region and sets
> >     timer device
> >   - Move plicsw_{cold,warm}_ipi_init() to andes_plicsw.c and define
> >     fdt_plicsw_cold_ipi_init() which adds PLICSW region and sets
> >     ipi device
> >
> >
> > Yu Chien Peter Lin (14):
> >   include: sbi: Fix typo in comment
> >   lib: sbi: Fix typo in comment
> >   include: sbi: Fix grammar in comment
> >   lib: sbi: Add sbi_domain_root_add_memrange() API
> >   platform: andes/ae350: Remove enabling cache from an350_final_init
> >   platform: andes/ae350: Use kconfig to set platform version and default
> >     name
> >   platform: andes/ae350: Use fdt serial driver
> >   lib: utils/timer: Add Andes fdt timer support
> >   lib: utils/reset: Add Andes fdt reset driver support
> >   platform: andes/ae350: Use fdt irqchip driver
> >   platform: andes/ae350: Add fw_platform_init for platform
> >     initialization
> >   lib: utils/ipi: Add Andes fdt ipi driver support
> >   platform: andes/ae350: Add AE350 domain support
> >   docs: andes-ae350.md: Update ae350 documentation for fdt driver
> >     support
> 
> Applied this series to the riscv/opensbi repo.
> 
> Any thoughts on using the generic platform ?
> 
> Thanks,
> Anup

Hi Anup,

The FW_TEXT_START on AE350 is different from generic platform.
Will it become a configuration?
I think we would keep the platform specific files in andes/ae350
until we have to refactor the duplicate code.

Thanks,
Peter Lin

> 
> >
> >  docs/platform/andes-ae350.md            | 184 +++++++++++++++++++++++-
> >  include/sbi/sbi_domain.h                |  16 ++-
> >  include/sbi/sbi_scratch.h               |   2 +-
> >  include/sbi_utils/fdt/fdt_helper.h      |   6 +
> >  include/sbi_utils/ipi/andes_plicsw.h    |  46 ++++++
> >  include/sbi_utils/timer/aclint_mtimer.h |   2 +
> >  include/sbi_utils/timer/andes_plmt.h    |  29 ++++
> >  lib/sbi/sbi_domain.c                    |  27 ++++
> >  lib/sbi/sbi_expected_trap.S             |   2 +-
> >  lib/utils/fdt/fdt_helper.c              | 108 ++++++++++++++
> >  lib/utils/ipi/Kconfig                   |   9 ++
> >  lib/utils/ipi/andes_plicsw.c            | 137 ++++++++++++++++++
> >  lib/utils/ipi/fdt_ipi_plicsw.c          |  47 ++++++
> >  lib/utils/ipi/objects.mk                |   4 +
> >  lib/utils/reset/Kconfig                 |   4 +
> >  lib/utils/reset/fdt_reset_atcwdt200.c   | 122 ++++++++++++++++
> >  lib/utils/reset/objects.mk              |   3 +
> >  lib/utils/timer/Kconfig                 |   9 ++
> >  lib/utils/timer/aclint_mtimer.c         |  50 ++-----
> >  lib/utils/timer/andes_plmt.c            | 104 ++++++++++++++
> >  lib/utils/timer/fdt_timer_plmt.c        |  51 +++++++
> >  lib/utils/timer/objects.mk              |   4 +
> >  platform/andes/ae350/Kconfig            |  30 +++-
> >  platform/andes/ae350/objects.mk         |   2 +-
> >  platform/andes/ae350/platform.c         | 165 +++++++++------------
> >  platform/andes/ae350/platform.h         |  17 ---
> >  platform/andes/ae350/plicsw.c           | 139 ------------------
> >  platform/andes/ae350/plicsw.h           |  44 ------
> >  platform/andes/ae350/plmt.c             | 107 --------------
> >  platform/andes/ae350/plmt.h             |  17 ---
> >  30 files changed, 1021 insertions(+), 466 deletions(-)
> >  create mode 100644 include/sbi_utils/ipi/andes_plicsw.h
> >  create mode 100644 include/sbi_utils/timer/andes_plmt.h
> >  create mode 100644 lib/utils/ipi/andes_plicsw.c
> >  create mode 100644 lib/utils/ipi/fdt_ipi_plicsw.c
> >  create mode 100644 lib/utils/reset/fdt_reset_atcwdt200.c
> >  create mode 100644 lib/utils/timer/andes_plmt.c
> >  create mode 100644 lib/utils/timer/fdt_timer_plmt.c
> >  delete mode 100644 platform/andes/ae350/plicsw.c
> >  delete mode 100644 platform/andes/ae350/plicsw.h
> >  delete mode 100644 platform/andes/ae350/plmt.c
> >  delete mode 100644 platform/andes/ae350/plmt.h
> >
> > --
> > 2.34.1
> >
> >
> > --
> > opensbi mailing list
> > opensbi@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
Anup Patel Oct. 27, 2022, 12:43 p.m. UTC | #4
On Thu, Oct 27, 2022 at 5:54 PM Yu-Chien Peter Lin
<peterlin@andestech.com> wrote:
>
> > > > Applied this series to the riscv/opensbi repo.
> > > >
> > > > Any thoughts on using the generic platform ?
> > > >
> > > > Thanks,
> > > > Anup
> > >
> > > Hi Anup,
> > >
> > > The FW_TEXT_START on AE350 is different from generic platform.
> > > Will it become a configuration?
> >
> > The generic platform has FW_PIC enabled so you can load and
> > run generic firmwares from any address.
> >
> > > I think we would keep the platform specific files in andes/ae350
> > > until we have to refactor the duplicate code.
> >
> > You can add platform specific overrides to generic platforms which
> > are selected based on compatible string in the root DT node.
> >
> > Regards,
> > Anup
>
> Hi Anup,
>
> We're considering using generic platform, but I don't find any
> vendor_ext_provider is implemented as a platform_override hook,
> does it mean we need to drop it if we move to generic platform?

vendor_ext_provider() is available in platform_override of generic
platform.

Please check the latest OpenSBI sources.

Regards,
Anup

>
> Best regards,
> Peter Lin
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
Yu Chien Peter Lin Oct. 27, 2022, 8:21 p.m. UTC | #5
> > > Applied this series to the riscv/opensbi repo.
> > >
> > > Any thoughts on using the generic platform ?
> > >
> > > Thanks,
> > > Anup
> >
> > Hi Anup,
> >
> > The FW_TEXT_START on AE350 is different from generic platform.
> > Will it become a configuration?
> 
> The generic platform has FW_PIC enabled so you can load and
> run generic firmwares from any address.
> 
> > I think we would keep the platform specific files in andes/ae350
> > until we have to refactor the duplicate code.
> 
> You can add platform specific overrides to generic platforms which
> are selected based on compatible string in the root DT node.
> 
> Regards,
> Anup

Hi Anup,

We're considering using generic platform, but I don't find any
vendor_ext_provider is implemented as a platform_override hook,
does it mean we need to drop it if we move to generic platform?

Best regards,
Peter Lin
Yu Chien Peter Lin Oct. 28, 2022, 8:43 a.m. UTC | #6
On Thu, Oct 27, 2022 at 06:13:48PM +0530, Anup Patel wrote:
> On Thu, Oct 27, 2022 at 5:54 PM Yu-Chien Peter Lin
> <peterlin@andestech.com> wrote:
> >
> > > > > Applied this series to the riscv/opensbi repo.
> > > > >
> > > > > Any thoughts on using the generic platform ?
> > > > >
> > > > > Thanks,
> > > > > Anup
> > > >
> > > > Hi Anup,
> > > >
> > > > The FW_TEXT_START on AE350 is different from generic platform.
> > > > Will it become a configuration?
> > >
> > > The generic platform has FW_PIC enabled so you can load and
> > > run generic firmwares from any address.
> > >
> > > > I think we would keep the platform specific files in andes/ae350
> > > > until we have to refactor the duplicate code.
> > >
> > > You can add platform specific overrides to generic platforms which
> > > are selected based on compatible string in the root DT node.
> > >
> > > Regards,
> > > Anup
> >
> > Hi Anup,
> >
> > We're considering using generic platform, but I don't find any
> > vendor_ext_provider is implemented as a platform_override hook,
> > does it mean we need to drop it if we move to generic platform?
> 
> vendor_ext_provider() is available in platform_override of generic
> platform.
> 
> Please check the latest OpenSBI sources.
> 
> Regards,
> Anup
> 

Hi Anup,

Sorry, I know there is vendor_ext_provider in platform_override,
except AE350, no other platforms actually have such hook in their
platform specific files, so I was wondering if the current acceptance
policy won't let us keep that function even in platform/andes/ae350.

Thanks,
Peter Lin
Anup Patel Oct. 28, 2022, 4:38 p.m. UTC | #7
On Fri, Oct 28, 2022 at 6:16 AM Yu-Chien Peter Lin
<peterlin@andestech.com> wrote:
>
> On Thu, Oct 27, 2022 at 06:13:48PM +0530, Anup Patel wrote:
> > On Thu, Oct 27, 2022 at 5:54 PM Yu-Chien Peter Lin
> > <peterlin@andestech.com> wrote:
> > >
> > > > > > Applied this series to the riscv/opensbi repo.
> > > > > >
> > > > > > Any thoughts on using the generic platform ?
> > > > > >
> > > > > > Thanks,
> > > > > > Anup
> > > > >
> > > > > Hi Anup,
> > > > >
> > > > > The FW_TEXT_START on AE350 is different from generic platform.
> > > > > Will it become a configuration?
> > > >
> > > > The generic platform has FW_PIC enabled so you can load and
> > > > run generic firmwares from any address.
> > > >
> > > > > I think we would keep the platform specific files in andes/ae350
> > > > > until we have to refactor the duplicate code.
> > > >
> > > > You can add platform specific overrides to generic platforms which
> > > > are selected based on compatible string in the root DT node.
> > > >
> > > > Regards,
> > > > Anup
> > >
> > > Hi Anup,
> > >
> > > We're considering using generic platform, but I don't find any
> > > vendor_ext_provider is implemented as a platform_override hook,
> > > does it mean we need to drop it if we move to generic platform?
> >
> > vendor_ext_provider() is available in platform_override of generic
> > platform.
> >
> > Please check the latest OpenSBI sources.
> >
> > Regards,
> > Anup
> >
>
> Hi Anup,
>
> Sorry, I know there is vendor_ext_provider in platform_override,
> except AE350, no other platforms actually have such hook in their
> platform specific files, so I was wondering if the current acceptance
> policy won't let us keep that function even in platform/andes/ae350.

We generally encourage platform vendors to prefer standard
SBI extensions over custom SBI extensions but it might not
be always possible to have a standard extension for everything
out there.

In other words, we do expect a few platform_override to have
vendor_ext_provider() for platform specific quirks and currently
we are OKAY with it but platform vendor will have to maintain
their vendor_ext_provider() implementation.

Regards,
Anup
Yu Chien Peter Lin Oct. 29, 2022, 10:01 a.m. UTC | #8
On Fri, Oct 28, 2022 at 10:08:21PM +0530, Anup Patel wrote:
> On Fri, Oct 28, 2022 at 6:16 AM Yu-Chien Peter Lin
> <peterlin@andestech.com> wrote:
> >
> > On Thu, Oct 27, 2022 at 06:13:48PM +0530, Anup Patel wrote:
> > > On Thu, Oct 27, 2022 at 5:54 PM Yu-Chien Peter Lin
> > > <peterlin@andestech.com> wrote:
> > > >
> > > > > > > Applied this series to the riscv/opensbi repo.
> > > > > > >
> > > > > > > Any thoughts on using the generic platform ?
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Anup
> > > > > >
> > > > > > Hi Anup,
> > > > > >
> > > > > > The FW_TEXT_START on AE350 is different from generic platform.
> > > > > > Will it become a configuration?
> > > > >
> > > > > The generic platform has FW_PIC enabled so you can load and
> > > > > run generic firmwares from any address.
> > > > >
> > > > > > I think we would keep the platform specific files in andes/ae350
> > > > > > until we have to refactor the duplicate code.
> > > > >
> > > > > You can add platform specific overrides to generic platforms which
> > > > > are selected based on compatible string in the root DT node.
> > > > >
> > > > > Regards,
> > > > > Anup
> > > >
> > > > Hi Anup,
> > > >
> > > > We're considering using generic platform, but I don't find any
> > > > vendor_ext_provider is implemented as a platform_override hook,
> > > > does it mean we need to drop it if we move to generic platform?
> > >
> > > vendor_ext_provider() is available in platform_override of generic
> > > platform.
> > >
> > > Please check the latest OpenSBI sources.
> > >
> > > Regards,
> > > Anup
> > >
> >
> > Hi Anup,
> >
> > Sorry, I know there is vendor_ext_provider in platform_override,
> > except AE350, no other platforms actually have such hook in their
> > platform specific files, so I was wondering if the current acceptance
> > policy won't let us keep that function even in platform/andes/ae350.
> 
> We generally encourage platform vendors to prefer standard
> SBI extensions over custom SBI extensions but it might not
> be always possible to have a standard extension for everything
> out there.
> 
> In other words, we do expect a few platform_override to have
> vendor_ext_provider() for platform specific quirks and currently
> we are OKAY with it but platform vendor will have to maintain
> their vendor_ext_provider() implementation.
> 
> Regards,
> Anup

Hi Anup,

Thanks for your clarification!

Best regards,
Peter Lin