[PATCH 2/2 RESEND] ARM: prima2: add NetWork on Chip driver for atlas7
Arnd Bergmann
arnd at arndb.de
Wed Apr 15 08:53:12 PDT 2015
On Tuesday 14 April 2015 11:55:56 Barry Song wrote:
> diff --git a/Documentation/devicetree/bindings/bus/atlas7-noc.txt b/Documentation/devicetree/bindings/bus/atlas7-noc.txt
> new file mode 100644
> index 0000000..449ddb5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/atlas7-noc.txt
> @@ -0,0 +1,34 @@
> +Device tree bindings for CSRatlas7 NoC(Network on Chip)
> +
> +CSR atlas7 uses a NoC bus, SoC is splitted into mutiple MACROs. Every MACRO
> +holds some hardware modules. For each MACRO
> +properties:
> +- compatible : Should be "arteris, flexnoc"
> +- #address-cells: should be 1
> +- #size-cells: should be 1
> +- ranges : the child address space are mapped 1:1 onto the parent address space
> +
> +Sub-nodes:
> +All the devices connected to noc are described using sub-node to noc. For
> +example, AUDMSCM MARCO includes multimediam nodes such as KAS, AC97, IACC, I2S,
> +USP0~3, LVDS.
> +For each MARCO, there is at least a firewall sub-node. This firewall can detect
> +illegal hardware access for security protection.
> +
> +Firewall sub-nodes:
> +properties:
> +- compatible : Should be one of
> + "sirf,nocfw-cpum",
> + "sirf,nocfw-cgum",
> + "sirf,nocfw-btm",
> + "sirf,nocfw-gnssm",
> + "sirf,nocfw-gpum",
> + "sirf,nocfw-mediam",
> + "sirf,nocfw-vdifm",
> + "sirf,nocfw-audiom",
> + "sirf,nocfw-ddrm",
> + "sirf,nocfw-rtcm",
> + "sirf,nocfw-dramfw",
> + "sirf,nocfw-spramfw"
> +- reg: A resource specifier for the register space
> +- interrupts : Should be the interrupt number - optional
I think we should have a more generic binding here, which describes the
differences between the instances of this bus in separate properties
rather than keying them off the compatible string.
That way you get a simpler driver that is automatically reusable
across chip generations, potentially using small extensions, but
not per-chip instance lists.
> diff --git a/arch/arm/mach-prima2/common.c b/arch/arm/mach-prima2/common.c
> index 8cadb30..4a9dcad 100644
> --- a/arch/arm/mach-prima2/common.c
> +++ b/arch/arm/mach-prima2/common.c
> @@ -15,6 +15,13 @@
> #include <linux/of_platform.h>
> #include "common.h"
>
> +static void __init sirfsoc_init_mach(void)
> +{
> + of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> +
> + sirfsoc_noc_init();
> +}
> +
> static void __init sirfsoc_init_late(void)
> {
> sirfsoc_pm_init();
> @@ -60,6 +67,7 @@ static const char *const atlas7_dt_match[] __initconst = {
> DT_MACHINE_START(ATLAS7_DT, "Generic ATLAS7 (Flattened Device Tree)")
> /* Maintainer: Barry Song <baohua.song at csr.com> */
> .smp = smp_ops(sirfsoc_smp_ops),
> + .init_machine = sirfsoc_init_mach,
> .dt_compat = atlas7_dt_match,
> MACHINE_END
> #endif
I think we can avoid adding this part.
> +#define ddrm_SecureState_ReadClr0 0x1054
> +#define ddrm_SecureState_ReadSts0 0x1058
> +
> +#define ddrm_SecureState_ReadSet1 0x105C
> +#define ddrm_SecureState_ReadClr1 0x1060
> +#define ddrm_SecureState_ReadSts1 0x1064
> +
> +#define ddrm_SecureState_ReadSet2 0x1068
> +#define ddrm_SecureState_ReadClr2 0x106C
> +#define ddrm_SecureState_ReadSts2 0x1070
> +
> +#define ddrm_SecureState_ReadSet3 0x1074
> +#define ddrm_SecureState_ReadClr3 0x1078
> +#define ddrm_SecureState_ReadSts3 0x107C
> +
> +
> +#define ddrm_SecureState_WriteSet0 0x1080
> +#define ddrm_SecureState_WriteClr0 0x1084
> +#define ddrm_SecureState_WriteSts0 0x1088
> +
> +#define ddrm_SecureState_WriteSet1 0x108C
> +#define ddrm_SecureState_WriteClr1 0x1090
> +#define ddrm_SecureState_WriteSts1 0x1094
> +
> +#define ddrm_SecureState_WriteSet2 0x1098
> +#define ddrm_SecureState_WriteClr2 0x109C
> +#define ddrm_SecureState_WriteSts2 0x10A0
> +
> +#define ddrm_SecureState_WriteSet3 0x10A4
> +#define ddrm_SecureState_WriteClr3 0x10A8
> +#define ddrm_SecureState_WriteSts3 0x10AC
> +
> +struct dramfw_reg_secure_t {
> + u32 readset;
> + u32 readclr;
> + u32 writeset;
> + u32 writeclr;
> +};
>
Can you move the dram controller parts into a child driver at
drivers/memory?
> +static struct dramfw_reg_secure_t dramfw_reg_secure_list[] = {
> + {ddrm_SecureState_ReadSet0, ddrm_SecureState_ReadClr0,
> + ddrm_SecureState_WriteSet0, ddrm_SecureState_WriteClr0},
> + {ddrm_SecureState_ReadSet1, ddrm_SecureState_ReadClr1,
> + ddrm_SecureState_WriteSet1, ddrm_SecureState_WriteClr1},
> + {ddrm_SecureState_ReadSet2, ddrm_SecureState_ReadClr2,
> + ddrm_SecureState_WriteSet2, ddrm_SecureState_WriteClr2},
> + {ddrm_SecureState_ReadSet3, ddrm_SecureState_ReadClr3,
> + ddrm_SecureState_WriteSet3, ddrm_SecureState_WriteClr3}
> +};
> +
> +struct noc_info_t {
> + const char *desc;
> +};
> +
> +static struct noc_info_t noc_initator_id_list[] = {
> + {"dmac2_ac97_aux_fifo"},
> + {"kas_dram"},
> + {"afe_cvd_vip0"},
> + {"usp0_axi_i"},
> + {"sgx"},
> + {"sdr"},
> + {"dmac2_usp1rx"},
> + {"dmac2_usp1tx"},
This table seems artificially soc specific.
> +enum NOC_MACRO_IDX {
> + CPUM_IDX = 0,
> + CGUM_IDX,
> + BTM_IDX,
> + GNSSM_IDX,
> + GPUM_IDX,
> + MEDIAM_IDX,
> + VDIFM_IDX,
> + AUDIOM_IDX,
> + DDRM_IDX,
> + RTCM_IDX,
> + DRAMFW_IDX,
> + SPRFW_IDX,
> +};
> +
> +/*register firewall offset based on macro index*/
> +static u32 noc_regfw_offset_list[10] = {0x1050, 0x50, 0x1050, 0x1050,
> + 0x1050, 0x1050, 0x2050, 0x2050, 0x2050, 0x2050};
> +
> +/*dram firewall sched port:six*/
> +#define FW_A7 0x0
> +#define FW_DDR_BE 0x4000
> +#define FW_DDR_RTLL 0x8000
> +#define FW_DDR_RT 0xC000
> +#define FW_DDR_SGX 0x10000
> +#define FW_DDR_VXD 0x14000
> +
> +#define NOC_MACRO_NUM 12
> +#define NOC_MACRO_NAME_LEN 12
> +
> +struct noc_macro {
> + void __iomem *mbase;
> + spinlock_t lock;
> + u32 idx;
> + u32 irq;
> + u32 errlogoff;
> + u32 faultenoff;
> + char name[NOC_MACRO_NAME_LEN];
> + int (*init_macro)(struct platform_device *);
> +};
> +
> +static int noc_macro_init(struct platform_device *);
> +static int noc_spram_firewall_init(struct platform_device *);
> +static int noc_dram_firewall_init(struct platform_device *);
> +static int noc_a7_init(struct platform_device *);
In general, please try to avoid forward declarations within on C file,
and just reorder the code appropriately.
> +static struct noc_macro noc_macro_list[] = {
> + {
> + .name = "cpum",
> + .idx = CPUM_IDX,
> + .errlogoff = NOC_CPUM_ERRLOG,
> + .faultenoff = NOC_CPUM_FAULTEN,
> + .init_macro = noc_a7_init,
> + }, {
> + .name = "cgum",
> + .idx = CGUM_IDX,
> + }, {
> + .name = "btm",
> + .idx = BTM_IDX,
> + }, {
> + .name = "gnssm",
> + .idx = GNSSM_IDX,
> + }, {
> + .name = "gpum",
> + .idx = GPUM_IDX,
> + }, {
> + .name = "mediam",
> + .idx = MEDIAM_IDX,
> + }, {
> + .name = "vdifm",
> + .idx = VDIFM_IDX,
> + }, {
> + .name = "audiom",
> + .idx = AUDIOM_IDX,
> + .errlogoff = NOC_AUDMSCM_ERRLOG,
> + .faultenoff = NOC_AUDMSCM_FAULTEN,
> + .init_macro = noc_macro_init,
> + }, {
> + .name = "ddrm",
> + .idx = DDRM_IDX,
> + .errlogoff = NOC_DDRM_ERRLOG,
> + .faultenoff = NOC_DDRM_FAULTEN,
> + .init_macro = noc_macro_init,
> + }, {
> + .name = "rtcm",
> + .idx = RTCM_IDX,
> + .errlogoff = NOC_RTCM_ERRLOG,
> + .faultenoff = NOC_RTCM_FAULTEN,
> + .init_macro = noc_macro_init,
> + }, {
> + .name = "dramfw",
> + .idx = DRAMFW_IDX,
> + .init_macro = noc_dram_firewall_init,
> + }, {
> + .name = "spramfw",
> + .idx = SPRFW_IDX,
> + .init_macro = noc_spram_firewall_init,
> + },
> +};
The index values here could easily be DT properties, and you already
have a name for each from the device node. The fault enable and
error log can also be simple boolean properties.
> +static ssize_t regfw_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct noc_macro *nocm;
> +
> + u32 idx, ns, a7, cssi, m3, kas;
> +
> + if (sscanf(buf, "%x %x %x %x %x %x\n", &idx, &ns, &a7,
> + &cssi, &m3, &kas) != 6 || idx > 9)
> + return -EINVAL;
> +
> + nocm = &noc_macro_list[idx];
> + noc_regfw_set(nocm->mbase,
> + noc_regfw_offset_list[idx], ns, a7, cssi, m3, kas);
> + return len;
> +}
> +
> +static DEVICE_ATTR_WO(regfw);
Any new sysfs files need a full description in Documentation/ABI.
Without that documentation, it's also hard to tell whether this ABI
is good.
> +__init int sirfsoc_noc_init(void)
> +{
> + struct device_node *np;
> + const struct of_device_id *match;
> + struct noc_macro *nocm;
> + struct platform_device *pdev;
> +
> + for_each_matching_node_and_match(np, sirfsoc_nocfw_ids, &match) {
> + if (!of_device_is_available(np))
> + continue;
> +
> + nocm = (struct noc_macro *)match->data;
> + nocm->mbase = of_iomap(np, 0);
> + if (!nocm->mbase)
> + return -ENOMEM;
> +
> + spin_lock_init(&nocm->lock);
> + pdev = of_find_device_by_node(np);
> + platform_set_drvdata(pdev, nocm);
> +
> + hook_fault_code(8, noc_abort_handler, SIGBUS, 0,
> + "external abort on non-linefetch");
> +
> + hook_fault_code(22, noc_abort_handler, SIGBUS, 0,
> + "imprecise external abort");
> +
> + if (nocm->init_macro)
> + nocm->init_macro(pdev);
> + }
This should become simpler if you have a platform driver for the
base device and use of_platform_populate to create its child devices.
Arnd
More information about the linux-arm-kernel
mailing list