[PATCH] PPC: define the conditions where the ePAPR idle hcall can be supported
From: Stuart Yoder stuart.yo...@freescale.com For 32-bit, CONFIG_EPAPR_PARAVIRT pulls in both epapr_paravirt.c and epapr_hcalls.c which contains the 32-bit paravirt idle loop. For 64-bit, the paravirt idle loop is in idle_book3e.S and that source file is included only if CONFIG_PPC_BOOK3E_64 defined. This patch makes that dependency for 64-bit explicit. Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- arch/powerpc/kernel/epapr_paravirt.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c index f3eab85..d44a571 100644 --- a/arch/powerpc/kernel/epapr_paravirt.c +++ b/arch/powerpc/kernel/epapr_paravirt.c @@ -23,8 +23,10 @@ #include asm/code-patching.h #include asm/machdep.h +#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64) extern void epapr_ev_idle(void); extern u32 epapr_ev_idle_start[]; +#endif bool epapr_paravirt_enabled; @@ -47,11 +49,15 @@ static int __init epapr_paravirt_init(void) for (i = 0; i (len / 4); i++) { patch_instruction(epapr_hypercall_start + i, insts[i]); +#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64) patch_instruction(epapr_ev_idle_start + i, insts[i]); +#endif } +#if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64) if (of_get_property(hyper_node, has-idle, NULL)) ppc_md.power_save = epapr_ev_idle; +#endif epapr_paravirt_enabled = true; -- 1.7.9.7 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] iommu: Add domain window handling functions
On Mon, Feb 4, 2013 at 7:18 AM, Joerg Roedel j...@8bytes.org wrote: Add the iommu_domain_window_enable() and iommu_domain_window_disable() functions to the IOMMU-API. These functions will be used to setup domains that are based on subwindows and not on paging. Signed-off-by: Joerg Roedel j...@8bytes.org --- drivers/iommu/iommu.c | 20 include/linux/iommu.h | 22 ++ 2 files changed, 42 insertions(+) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index ab9dafd..0fdb7db 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -852,6 +852,26 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) } EXPORT_SYMBOL_GPL(iommu_unmap); + +int iommu_domain_window_enable(struct iommu_domain *domain, u32 wnd_nr, + phys_addr_t paddr, size_t size) +{ + if (unlikely(domain-ops-domain_window_enable == NULL)) + return -ENODEV; + + return domain-ops-domain_window_enable(domain, wnd_nr, paddr, size); +} +EXPORT_SYMBOL_GPL(iommu_domain_window_enable); + +void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr) +{ + if (unlikely(domain-ops-domain_window_disable == NULL)) + return; + + return domain-ops-domain_window_disable(domain, wnd_nr); +} +EXPORT_SYMBOL_GPL(iommu_domain_window_disable); + static int __init iommu_init(void) { iommu_group_kset = kset_create_and_add(iommu_groups, diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 26066f5..0cba2d8 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -101,6 +101,12 @@ struct iommu_ops { enum iommu_attr attr, void *data); int (*domain_set_attr)(struct iommu_domain *domain, enum iommu_attr attr, void *data); + + /* Window handling functions */ + int (*domain_window_enable)(struct iommu_domain *domain, u32 wnd_nr, + phys_addr_t paddr, size_t size); + void (*domain_window_disable)(struct iommu_domain *domain, u32 wnd_nr); + unsigned long pgsize_bitmap; }; @@ -158,6 +164,10 @@ extern int iommu_domain_get_attr(struct iommu_domain *domain, enum iommu_attr, extern int iommu_domain_set_attr(struct iommu_domain *domain, enum iommu_attr, void *data); +/* Window handling function prototypes */ +extern int iommu_domain_window_enable(struct iommu_domain *domain, u32 wnd_nr, + phys_addr_t offset, size_t size); +extern void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr); /** * report_iommu_fault() - report about an IOMMU fault to the IOMMU framework * @domain: the iommu domain where the fault has happened @@ -240,6 +250,18 @@ static inline int iommu_unmap(struct iommu_domain *domain, unsigned long iova, return -ENODEV; } +static inline int iommu_domain_window_enable(struct iommu_domain *domain, +u32 wnd_nr, phys_addr_t paddr, +size_t size) +{ + return -ENODEV; +} + +static inline void iommu_domain_window_disable(struct iommu_domain *domain, + u32 wnd_nr) +{ +} + static inline phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, unsigned long iova) { This API looks workable. The one change we need is that the size argument in the enable API needs to be 64 bits. Our window sizes can exceed 4GB. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] iommu: Add domain window handling functions
On Mon, Feb 4, 2013 at 12:56 PM, Joerg Roedel j...@8bytes.org wrote: On Mon, Feb 04, 2013 at 12:10:51PM -0600, Stuart Yoder wrote: On Mon, Feb 4, 2013 at 7:18 AM, Joerg Roedel j...@8bytes.org wrote: +static inline int iommu_domain_window_enable(struct iommu_domain *domain, +u32 wnd_nr, phys_addr_t paddr, +size_t size) +{ + return -ENODEV; +} + +static inline void iommu_domain_window_disable(struct iommu_domain *domain, + u32 wnd_nr) +{ +} + static inline phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, unsigned long iova) { This API looks workable. The one change we need is that the size argument in the enable API needs to be 64 bits. Our window sizes can exceed 4GB. Okay. So if your architecture supports sizes over 2^32 then size_t probably is already 64bits, right? No, on a 32-bit platform size_t would generally be 32-bits. But the PAMU is independent of that. I think we should just make it a u64. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
Have not got through the entire file, but have a few comments... +/* + * Set the PAACE type as primary and set the coherency required domain + * attribute + */ +static void pamu_setup_default_xfer_to_host_ppaace(struct paace *ppaace) +{ + set_bf(ppaace-addr_bitfields, PAACE_AF_PT, PAACE_PT_PRIMARY); + + set_bf(ppaace-domain_attr.to_host.coherency_required, PAACE_DA_HOST_CR, + PAACE_M_COHERENCE_REQ); +} + +/* + * Set the PAACE type as secondary and set the coherency required domain + * attribute. + */ +static void pamu_setup_default_xfer_to_host_spaace(struct paace *spaace) +{ + set_bf(spaace-addr_bitfields, PAACE_AF_PT, PAACE_PT_SECONDARY); + set_bf(spaace-domain_attr.to_host.coherency_required, PAACE_DA_HOST_CR, + PAACE_M_COHERENCE_REQ); +} Can we change the names of the above functions... I know there is some history with the name, but xfer_to_host is confusing. Maybe just call them: pamu_init_paace() pamu_init_spaace() +/** + * pamu_config_spaace() - Sets up SPAACE entry for specified subwindow + * + * @liodn: Logical IO device number + * @subwin_cnt: number of sub-windows associated with dma-window + * @subwin_addr: starting address of subwindow + * @subwin_size: size of subwindow + * @omi: Operation mapping index + * @rpn: real (true physical) page number + * @snoopid: snoop id for hardware coherency -- if ~snoopid == 0 then + * snoopid not defined + * @stashid: cache stash id for associated cpu + * @enable: enable/disable subwindow after reconfiguration + * @prot: sub window permissions + * + * Returns 0 upon success else error code 0 returned + */ +int pamu_config_spaace(int liodn, u32 subwin_cnt, u32 subwin_addr, + phys_addr_t subwin_size, u32 omi, unsigned long rpn, + u32 snoopid, u32 stashid, int enable, int prot) +{ + struct paace *paace; + + /* setup sub-windows */ + if (!subwin_cnt) { + pr_err(Invalid subwindow count\n); + return -EINVAL; + } + + paace = pamu_get_ppaace(liodn); + if (subwin_addr 0 subwin_addr subwin_cnt paace) { Why is the comparison subwin_addr subwin_cnt? Seems wrong... + paace = pamu_get_spaace(paace, subwin_addr - 1); + + if (paace !(paace-addr_bitfields PAACE_V_VALID)) { + pamu_setup_default_xfer_to_host_spaace(paace); + set_bf(paace-addr_bitfields, SPAACE_AF_LIODN, liodn); + } + } + + if (!paace) { + pr_err(Invalid liodn entry\n); + return -ENOENT; + } + + if (!enable prot == PAACE_AP_PERMS_DENIED) { + if (subwin_addr 0) + set_bf(paace-addr_bitfields, PAACE_AF_V, +PAACE_V_INVALID); + else + set_bf(paace-addr_bitfields, PAACE_AF_AP, +prot); + mb(); + return 0; + } Can you add a comment to the above if statement...when is this function called with PAACE_AP_PERMS_DENIED? + if (subwin_size (subwin_size - 1) || subwin_size PAMU_PAGE_SIZE) { + pr_err(subwindow size out of range, or not a power of 2\n); + return -EINVAL; + } + + if (rpn == ULONG_MAX) { + pr_err(real page number out of range\n); + return -EINVAL; + } + + /* window size is 2^(WSE+1) bytes */ + set_bf(paace-win_bitfields, PAACE_WIN_SWSE, + map_addrspace_size_to_wse(subwin_size)); + + set_bf(paace-impl_attr, PAACE_IA_ATM, PAACE_ATM_WINDOW_XLATE); + paace-twbah = rpn 20; + set_bf(paace-win_bitfields, PAACE_WIN_TWBAL, rpn); + set_bf(paace-addr_bitfields, PAACE_AF_AP, prot); + + /* configure snoop id */ + if (~snoopid != 0) + paace-domain_attr.to_host.snpid = snoopid; + + /* set up operation mapping if it's configured */ + if (omi OME_NUMBER_ENTRIES) { + set_bf(paace-impl_attr, PAACE_IA_OTM, PAACE_OTM_INDEXED); + paace-op_encode.index_ot.omi = omi; + } else if (~omi != 0) { + pr_err(bad operation mapping index: %d\n, omi); + return -EINVAL; + } + + if (~stashid != 0) + set_bf(paace-impl_attr, PAACE_IA_CID, stashid); + + smp_wmb(); + + if (enable) + paace-addr_bitfields |= PAACE_V_VALID; + + mb(); + + return 0; +} -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
Some more comments... On Mon, Feb 18, 2013 at 6:52 AM, Varun Sethi varun.se...@freescale.com wrote: +/* Handling access violations */ +#define make64(high, low) (((u64)(high) 32) | (low)) + +struct pamu_isr_data { + void __iomem *pamu_reg_base;/* Base address of PAMU regs*/ + unsigned int count; /* The number of PAMUs */ +}; + +static struct paace *ppaact; +static struct paace *spaact; +static struct ome *omt; + +/* maximum subwindows permitted per liodn */ +unsigned int max_subwindow_count; +/* Number of SPAACT entries */ +unsigned long max_subwins; I don't like that these variables are not static... and they are referenced directly from code in fsl_pamu_domain.c. It would be better if fsl_pamu_domain.c called an accessor function-- like pamu_get_max_subwins. +/* Pool for fspi allocation */ +struct gen_pool *spaace_pool; spaace_pool should be static? I'm wondering if you should change pamu_isr_data into a more general struct analagous to struct intel_iommu. You could put in there the max # of subwins, etc. You could then provide an accessor to get at that data. [cut] +/** + * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows + *required for primary PAACE in the secondary + *PAACE table. + * @subwin_cnt: Number of subwindows to be reserved. + * + * A PPAACE entry may have a number of associated subwindows. A subwindow + * corresponds to a SPAACE entry in the SPAACT table. Each PAACE entry stores + * the index (fspi) of the first SPAACE entry in the SPAACT table. This + * function returns the index of the first SPAACE entry. The remaining + * SPAACE entries are reserved contiguously from that index. + * + * Returns a valid fspi index in the range of 0 - max_subwins on success. + * If no SPAACE entry is available or the allocator can not reserve the required + * number of contiguous entries function returns ULONG_MAX indicating a failure. + * +*/ +static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt) +{ + unsigned long spaace_addr; + + spaace_addr = gen_pool_alloc(spaace_pool, subwin_cnt * sizeof(struct paace)); + if (!spaace_addr) + return ULONG_MAX; + + return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace)); +} In order to keep things symmetric (with the free function) can we just call the above function: pamu_alloc_subwins() +/* Release the subwindows reserved for a particular LIODN */ +void pamu_free_subwins(int liodn) +{ + struct paace *ppaace; + u32 subwin_cnt, size; + + ppaace = pamu_get_ppaace(liodn); + if (!ppaace) { + pr_err(Invalid liodn entry\n); + return; + } + + if (get_bf(ppaace-addr_bitfields, PPAACE_AF_MW)) { + subwin_cnt = 1UL (get_bf(ppaace-impl_attr, PAACE_IA_WCE) + 1); + size = (subwin_cnt - 1) * sizeof(struct paace); + gen_pool_free(spaace_pool, (unsigned long)spaact[ppaace-fspi], size); + set_bf(ppaace-addr_bitfields, PPAACE_AF_MW, 0); + } +} [cut] +/** + * get_stash_id - Returns stash destination id corresponding to a + *cache type and vcpu. + * @stash_dest_hint: L1, L2 or L3 + * @vcpu: vpcu target for a particular cache type. + * + * Returs stash on success or ~(u32)0 on failure. + * + */ +u32 get_stash_id(u32 stash_dest_hint, u32 vcpu) +{ The stash dest is really not a hint, right? It's the requested stash destination. So maybe just drop 'hint' from the name. The CPU here is really a physical CPU number and has nothing to do with vcpus I think. vcpu implies the index is inside a virtual machine...but this API is generic and may or may not be used with KVM. + +/* + * Get the maximum number of PAACT table entries + * and subwindows supported by PAMU + */ +static void get_pamu_cap_values(unsigned long pamu_reg_base) +{ + u32 pc_val; + + pc_val = in_be32((u32 *)(pamu_reg_base + PAMU_PC3)); + /* Maximum number of subwindows per liodn */ + max_subwindow_count = 1 (1 + PAMU_PC3_MWCE(pc_val)); + /* Total number of SPACCT entries */ + max_subwins = PAACE_NUMBER_ENTRIES * max_subwindow_count; +} If you follow the suggestion at the top of this file, this function would become something like-- init_pamu_capabilities(). And then create an accessor function to access max subwins, etc. Also, BTW, I don't see any support for the DOMAIN_ATTR_WINDOWS attribute in your patch. Was that coming in a later patch? [cut +static int __init fsl_pamu_probe(struct platform_device *pdev) +{ + void __iomem *pamu_regs = NULL; + struct ccsr_guts __iomem *guts_regs = NULL; + u32 pamubypenr, pamu_counter; + unsigned long pamu_reg_off; + unsigned long pamu_reg_base; + struct
Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
On Mon, Feb 18, 2013 at 6:52 AM, Varun Sethi varun.se...@freescale.com wrote: [cut] +static phys_addr_t get_phys_addr(struct fsl_dma_domain *dma_domain, unsigned long iova) +{ + u32 win_cnt = dma_domain-win_cnt; + struct dma_window *win_ptr = + dma_domain-win_arr[0]; + struct iommu_domain_geometry *geom; + + geom = dma_domain-iommu_domain-geometry; + + if (!win_cnt || !dma_domain-geom_size) { + pr_err(Number of windows/geometry not configured for the domain\n); + return 0; + } + + if (win_cnt 1) { + u64 subwin_size; + unsigned long subwin_iova; + u32 wnd; + + subwin_size = dma_domain-geom_size ilog2(win_cnt); Could it be just geom_size / win_cnt ?? + subwin_iova = iova ~(subwin_size - 1); + wnd = (subwin_iova - geom-aperture_start) ilog2(subwin_size); + win_ptr = dma_domain-win_arr[wnd]; + } + + if (win_ptr-valid) + return (win_ptr-paddr + (iova (win_ptr-size - 1))); + + return 0; +} + +static int map_liodn_subwins(int liodn, struct fsl_dma_domain *dma_domain) Just call it map_subwins(). They are just sub windows, not liodn sub windows. [cut] +static int map_liodn_win(int liodn, struct fsl_dma_domain *dma_domain) Call it map_win(). [cut] +static struct fsl_dma_domain *iommu_alloc_dma_domain(void) +{ + struct fsl_dma_domain *domain; + + domain = kmem_cache_zalloc(fsl_pamu_domain_cache, GFP_KERNEL); + if (!domain) + return NULL; + + domain-stash_id = ~(u32)0; + domain-snoop_id = ~(u32)0; + domain-win_cnt = max_subwindow_count; To align with my previous comments on fsl_pamu.c, I think instead of referencing a global variable (in fsl_pamu.c) you should be making an accessor API call here to get the max subwindow _count. + domain-geom_size = 0; + + INIT_LIST_HEAD(domain-devices); + + spin_lock_init(domain-domain_lock); + + return domain; +} + +static inline struct device_domain_info *find_domain(struct device *dev) +{ + return dev-archdata.iommu_domain; +} + +static void remove_domain_ref(struct device_domain_info *info, u32 win_cnt) +{ + list_del(info-link); + spin_lock(iommu_lock); + if (win_cnt) + pamu_free_subwins(info-liodn); + pamu_disable_liodn(info-liodn); + spin_unlock(iommu_lock); + spin_lock(device_domain_lock); + info-dev-archdata.iommu_domain = NULL; + kmem_cache_free(iommu_devinfo_cache, info); + spin_unlock(device_domain_lock); +} The above function is literally removing the _device_ reference from the domain. The name implies that it is removing a domain reference. Suggestion is to call it remove_device_ref. Also, the whitespace is messed up there. You have 2 tabs instead of 1. +static void destroy_domain(struct fsl_dma_domain *dma_domain) +{ + struct device_domain_info *info; + + /* Dissociate all the devices from this domain */ + while (!list_empty(dma_domain-devices)) { + info = list_entry(dma_domain-devices.next, + struct device_domain_info, link); + remove_domain_ref(info, dma_domain-win_cnt); + } +} This function is removing all devices from a domain...maybe to be consistent with my suggestion below on detach_domain(), call this detach_all_devices(). We have 2 functions doing almost the same thingone detaches a single device, one detaches all devices. The current names destroy_domain and detach_domain are not as clear to me. +static void detach_domain(struct device *dev, struct fsl_dma_domain *dma_domain) +{ + struct device_domain_info *info; + struct list_head *entry, *tmp; + unsigned long flags; + + spin_lock_irqsave(dma_domain-domain_lock, flags); + /* Remove the device from the domain device list */ + if (!list_empty(dma_domain-devices)) { + list_for_each_safe(entry, tmp, dma_domain-devices) { + info = list_entry(entry, struct device_domain_info, link); + if (info-dev == dev) + remove_domain_ref(info, dma_domain-win_cnt); + } + } + spin_unlock_irqrestore(dma_domain-domain_lock, flags); +} This function is not detaching a domain, but is detaching a device. Call it detach_device(). +static void attach_domain(struct fsl_dma_domain *dma_domain, int liodn, struct device *dev) +{ Same thing here. This is not attaching a domain, but attaching a device. Call it attach_device. + struct device_domain_info *info, *old_domain_info; + + spin_lock(device_domain_lock);
Re: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.
This patch was submitted separately to linuxppc-dev (and was already applied). You don't need it in this patch set, right? Stuart On Mon, Feb 18, 2013 at 6:52 AM, Varun Sethi varun.se...@freescale.com wrote: The pci controller structure has a provision to store the device strcuture pointer of the corresponding platform device. Currently this information is not stored during fsl pci controller initialization. This information is required while dealing with iommu groups for pci devices connected to the fsl pci controller. For the case where the pci devices can't be paritioned, they would fall under the same device group as the pci controller. This patch stores the platform device information in the pci controller structure during initialization. Signed-off-by: Varun Sethi varun.se...@freescale.com --- arch/powerpc/sysdev/fsl_pci.c |9 +++-- arch/powerpc/sysdev/fsl_pci.h |2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c index 92a5915..b393ae7 100644 --- a/arch/powerpc/sysdev/fsl_pci.c +++ b/arch/powerpc/sysdev/fsl_pci.c @@ -421,13 +421,16 @@ void fsl_pcibios_fixup_bus(struct pci_bus *bus) } } -int __init fsl_add_bridge(struct device_node *dev, int is_primary) +int __init fsl_add_bridge(struct platform_device *pdev, int is_primary) { int len; struct pci_controller *hose; struct resource rsrc; const int *bus_range; u8 hdr_type, progif; + struct device_node *dev; + + dev = pdev-dev.of_node; if (!of_device_is_available(dev)) { pr_warning(%s: disabled\n, dev-full_name); @@ -453,6 +456,8 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary) if (!hose) return -ENOMEM; + /* set platform device as the parent */ + hose-parent = pdev-dev; hose-first_busno = bus_range ? bus_range[0] : 0x0; hose-last_busno = bus_range ? bus_range[1] : 0xff; @@ -880,7 +885,7 @@ static int fsl_pci_probe(struct platform_device *pdev) #endif node = pdev-dev.of_node; - ret = fsl_add_bridge(node, fsl_pci_primary == node); + ret = fsl_add_bridge(pdev, fsl_pci_primary == node); #ifdef CONFIG_SWIOTLB if (ret == 0) { diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h index d078537..c495c00 100644 --- a/arch/powerpc/sysdev/fsl_pci.h +++ b/arch/powerpc/sysdev/fsl_pci.h @@ -91,7 +91,7 @@ struct ccsr_pci { __be32 pex_err_cap_r3; /* 0x.e34 - PCIE error capture register 0 */ }; -extern int fsl_add_bridge(struct device_node *dev, int is_primary); +extern int fsl_add_bridge(struct platform_device *pdev, int is_primary); extern void fsl_pcibios_fixup_bus(struct pci_bus *bus); extern int mpc83xx_add_bridge(struct device_node *dev); u64 fsl_pci_immrbar_base(struct pci_controller *hose); -- 1.7.4.1 ___ iommu mailing list io...@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
driver core patches
Hi Greg, Any feedback on the driver core patches in Kim Phillips patch series from mid-October? The 3rd patch of the series has some open issues related to vfio-pci, but the first two are driver core related and am wondering if they are acceptable. [PATCH 1/4] driver core: Add new device_driver flag to allow binding via sysfs only http://www.spinics.net/lists/kvm/msg97198.html [PATCH 2/4] driver core: platform: allow platform drivers to bind to any device http://www.spinics.net/lists/kvm/msg97195.html If you have any issues, we would like to get them addressed. Thanks, Stuart Yoder -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
-Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Monday, June 16, 2014 10:28 AM To: Sethi Varun-B16395 Cc: Thierry Reding; Mark Rutland; devicet...@vger.kernel.org; linux- samsung-...@vger.kernel.org; Pawel Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren; linux-kernel@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob Herring; Kumar Gala; linux- te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm- ker...@lists.infradead.org; Yoder Stuart-B08248 Subject: Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings Hi Varun, On Thu, Jun 05, 2014 at 08:10:19PM +0100, Varun Sethi wrote: The set of StreamIDs that can be generated by a master is fixed in the hardware. The SMMU can then be programmed to map these incoming IDs onto a context ID (or a set of context IDs), which are the IDs used internally by the SMMU to find the page tables etc. The StreamID - ContextID mapping is dynamic and controlled by software. The Master - StreamIDs mapping is fixed in the hardware. The Master - StreamIDs mapping may not always be fixed in the hardware. At, least in our case we plan to program these via software. PCI devices is one place where this mapping would have to be dynamic (based on the topology i.e. if the devices are connected to a bridge etc). Also, we have a hot plug device architecture where the stream ID is software programmable. Other than that, based on the isolation requirements (iommu domain) software programmability offers greater flexibility. For the sake of consistency, I'd really like to treat the master - streamIDs mapping as fixed. If that means setting it once during boot in your case, then so be it (ideally in the firmware). That way, we just describe the fixed mapping to the kernel and the driver doesn't have to worry about changing the mapping, especially given that that's likely to be highly error-prone once the SMMU is in us. Do you have use-cases where you really need to change these mappings dynamically? Yes. In the case of a PCI bus-- you may not know in advance how many PCI devices there are until you probe the bus. We have another FSL proprietary bus we call the fsl-mc bus that is similar. Another thing to consider-- starting with SMMUv2, as you know, there is a new distributed architecture with multiple TBUs and a centralized TCU that walks the SMMU page tables. So instead of sprinkling multiple SMMUs all over an SoC you now have the option a 1 central TCU and sprinkling multiple TBUs around. However, this means that the stream ID namespace is now global and can be pretty limited. In the SMMU implementation we have there are only 64 stream ID total for our Soc. But we have many more masters than that. So we look at stream IDs as really corresponding to an 'isolation context' and not to a bus master. An isolation context is the domain you are trying to isolate with the SMMU. Devices that all belong to the same 'isolation context' can share the same stream ID, since they share the same domain and page tables. So, perhaps by default some/most SMMU masters may have a default stream ID of 0x0 that is used by the host...and that could be represented statically in the device tree. But, we absolutely will need to dynamically set new stream IDs into masters when a new IOMMU 'domain' is created and devices are added to it. All the devices in a domain will share the same stream ID. So whatever we do, let's please have an architecture flexible enough to allow for this. Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
-Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Monday, June 16, 2014 12:04 PM To: Yoder Stuart-B08248 Cc: Sethi Varun-B16395; Thierry Reding; Mark Rutland; devicet...@vger.kernel.org; linux-samsung-...@vger.kernel.org; Pawel Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren; linux- ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob Herring; Kumar Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm- ker...@lists.infradead.org Subject: Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings Hi Stuart, On Mon, Jun 16, 2014 at 05:56:32PM +0100, Stuart Yoder wrote: Do you have use-cases where you really need to change these mappings dynamically? Yes. In the case of a PCI bus-- you may not know in advance how many PCI devices there are until you probe the bus. We have another FSL proprietary bus we call the fsl-mc bus that is similar. For that case, though, you could still describe an algorithmic transformation from RequesterID to StreamID which corresponds to a fixed mapping. Another thing to consider-- starting with SMMUv2, as you know, there is a new distributed architecture with multiple TBUs and a centralized TCU that walks the SMMU page tables. So instead of sprinkling multiple SMMUs all over an SoC you now have the option a 1 central TCU and sprinkling multiple TBUs around. However, this means that the stream ID namespace is now global and can be pretty limited. In the SMMU implementation we have there are only 64 stream ID total for our Soc. But we have many more masters than that. So we look at stream IDs as really corresponding to an 'isolation context' and not to a bus master. An isolation context is the domain you are trying to isolate with the SMMU. Devices that all belong to the same 'isolation context' can share the same stream ID, since they share the same domain and page tables. Ok, this is more compelling. So, perhaps by default some/most SMMU masters may have a default stream ID of 0x0 that is used by the host...and that could be represented statically in the device tree. But, we absolutely will need to dynamically set new stream IDs into masters when a new IOMMU 'domain' is created and devices are added to it. All the devices in a domain will share the same stream ID. So whatever we do, let's please have an architecture flexible enough to allow for this. What is the software interface to the logic that assigns the StreamIDs? Is it part of the SMMU, or a separate device (or set of devices)? For us at the hardware level there are a few different ways that the streamIDs can be set. It is not part of the SMMU. In the cases where there is simply 1 device to 1 streamID (e.g. USB controller) there is an SoC register where you just program the stream ID. In the case of PCI, our PCI controller has a RequesterID-to-streamID table that you set via some PCI controller registers. The way we generally thought it would work was something like this: -u-boot/bootloader makes any static streamID allocation if needed, sets a default streamID (e.g. 0x0) in device and expresses that in the device tree -device tree would express relationship between devices (including bus controllers) and the SMMU through mmu-masters property -u-boot would express the range of unused (or used) streamIDs via a new device tree property so the kernel SMMU driver knows what streamIDs are free -in the SMMU driver a different vendor specific 'add_device' callback could be used to handle our special cases where we need to set/change the stream ID for devices added to a domain Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
-Original Message- From: Sethi Varun-B16395 Sent: Tuesday, June 17, 2014 5:27 AM To: Yoder Stuart-B08248; Will Deacon Cc: Thierry Reding; Mark Rutland; devicet...@vger.kernel.org; linux- samsung-...@vger.kernel.org; Pawel Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren; linux-kernel@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob Herring; Kumar Gala; linux- te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm- ker...@lists.infradead.org Subject: RE: [PATCH v2] devicetree: Add generic IOMMU device tree bindings -Original Message- From: Yoder Stuart-B08248 Sent: Tuesday, June 17, 2014 12:24 AM To: Will Deacon Cc: Sethi Varun-B16395; Thierry Reding; Mark Rutland; devicet...@vger.kernel.org; linux-samsung-...@vger.kernel.org; Pawel Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren; linux- ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob Herring; Kumar Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux- arm- ker...@lists.infradead.org Subject: RE: [PATCH v2] devicetree: Add generic IOMMU device tree bindings -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Monday, June 16, 2014 12:04 PM To: Yoder Stuart-B08248 Cc: Sethi Varun-B16395; Thierry Reding; Mark Rutland; devicet...@vger.kernel.org; linux-samsung-...@vger.kernel.org; Pawel Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren; linux- ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob Herring; Kumar Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm- ker...@lists.infradead.org Subject: Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings Hi Stuart, On Mon, Jun 16, 2014 at 05:56:32PM +0100, Stuart Yoder wrote: Do you have use-cases where you really need to change these mappings dynamically? Yes. In the case of a PCI bus-- you may not know in advance how many PCI devices there are until you probe the bus. We have another FSL proprietary bus we call the fsl-mc bus that is similar. For that case, though, you could still describe an algorithmic transformation from RequesterID to StreamID which corresponds to a fixed mapping. Another thing to consider-- starting with SMMUv2, as you know, there is a new distributed architecture with multiple TBUs and a centralized TCU that walks the SMMU page tables. So instead of sprinkling multiple SMMUs all over an SoC you now have the option a 1 central TCU and sprinkling multiple TBUs around. However, this means that the stream ID namespace is now global and can be pretty limited. In the SMMU implementation we have there are only 64 stream ID total for our Soc. But we have many more masters than that. So we look at stream IDs as really corresponding to an 'isolation context' and not to a bus master. An isolation context is the domain you are trying to isolate with the SMMU. Devices that all belong to the same 'isolation context' can share the same stream ID, since they share the same domain and page tables. Ok, this is more compelling. So, perhaps by default some/most SMMU masters may have a default stream ID of 0x0 that is used by the host...and that could be represented statically in the device tree. But, we absolutely will need to dynamically set new stream IDs into masters when a new IOMMU 'domain' is created and devices are added to it. All the devices in a domain will share the same stream ID. So whatever we do, let's please have an architecture flexible enough to allow for this. What is the software interface to the logic that assigns the StreamIDs? Is it part of the SMMU, or a separate device (or set of devices)? For us at the hardware level there are a few different ways that the streamIDs can be set. It is not part of the SMMU. In the cases where there is simply 1 device to 1 streamID (e.g. USB controller) there is an SoC register where you just program the stream ID. In the case of PCI, our PCI controller has a RequesterID-to-streamID table that you set via some PCI controller registers. The way we generally thought it would work was something like this: -u-boot/bootloader makes any static streamID allocation if needed, sets a default streamID (e.g. 0x0) in device and expresses that in the device tree -device tree would express relationship between devices (including bus controllers) and the SMMU through mmu-masters property -u-boot would express the range of unused (or used) streamIDs via a new device tree property so the kernel SMMU driver knows what streamIDs are free -in the SMMU driver a different vendor specific 'add_device' callback could
RE: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
-Original Message- From: Sethi Varun-B16395 Sent: Tuesday, June 17, 2014 6:22 AM To: Will Deacon Cc: Mark Rutland; devicet...@vger.kernel.org; linux-samsung- s...@vger.kernel.org; Arnd Bergmann; Pawel Moll; Ian Campbell; Grant Grundler; Stephen Warren; Yoder Stuart-B08248; Rob Herring; linux- ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Thierry Reding; Kumar Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm- ker...@lists.infradead.org Subject: RE: [PATCH v2] devicetree: Add generic IOMMU device tree bindings -Original Message- From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- boun...@lists.linux-foundation.org] On Behalf Of Will Deacon Sent: Tuesday, June 17, 2014 4:13 PM To: Sethi Varun-B16395 Cc: Mark Rutland; devicet...@vger.kernel.org; linux-samsung- s...@vger.kernel.org; Arnd Bergmann; Pawel Moll; Ian Campbell; Grant Grundler; Stephen Warren; Yoder Stuart-B08248; Rob Herring; linux- ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Thierry Reding; Kumar Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux- arm- ker...@lists.infradead.org Subject: Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings On Tue, Jun 17, 2014 at 11:26:48AM +0100, Varun Sethi wrote: The way we generally thought it would work was something like this: -u-boot/bootloader makes any static streamID allocation if needed, sets a default streamID (e.g. 0x0) in device and expresses that in the device tree -device tree would express relationship between devices (including bus controllers) and the SMMU through mmu-masters property -u-boot would express the range of unused (or used) streamIDs via a new device tree property so the kernel SMMU driver knows what streamIDs are free -in the SMMU driver a different vendor specific 'add_device' callback could be used to handle our special cases where we need to set/change the stream ID for devices added to a domain Another possibility, could be to program the stream Id in the device registers (reference for the stream ID register can be obtained from the device tree) during device attach. This could be relevant in case of VFIO, when we are assigning multiple devices to a single VM. All the devices can share the same stream ID. I think for simple masters (i.e. those that have all their StreamIDs under control of one driver), then setting something during attach (or add?) based on the DT could work pretty well. The other case is when we have masters behind a bridge (such as a PCI RC). In this case, it might actually be better to ask the bridge for the IDs and let it sort out the allocation itself. That would also move the RequesterID - StreamID mapping out of the SMMU code. What do you think? The PCI bus iommu group creation code would be part of the SMMU driver (it is handled by other IOMMU drivers as well). My understanding is that there would be one is to one correspondence between the requestor ID and the iommu group. May be, we can have an API provided by the PCI bridge (architecture specific) for setting the stream ID. I think Will is suggesting something along those lines-- I think it's a question of where the streamID allocation happens. You could either do something like the following in the SMMU driver when attaching a PCI device: id = alloc_stream_id(...); pci_set_streamid(pci-dev, id); or id = pci_get_streamid(pci-dev); ...i.e the PCI RC could allocate (from some TBD allocator) and set the stream ID itself. Not sure how big a deal it is to extend PCI RC interfaces for something like that. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
-Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Friday, April 04, 2014 3:19 PM To: bhelg...@google.com; linux-...@vger.kernel.org Cc: ag...@suse.de; k...@vger.kernel.org; konrad.w...@oracle.com; kim.phill...@linaro.org; gre...@linuxfoundation.org; Yoder Stuart-B08248; linux-kernel@vger.kernel.org; libvir-l...@redhat.com; iommu@lists.linux- foundation.org; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; christoffer.d...@linaro.org Subject: [PATCH] PCI: Introduce new device binding path using pci_dev.driver_override The driver_override field allows us to specify the driver for a device rather than relying on the driver to provide a positive match of the device. This shortcuts the existing process of looking up the vendor and device ID, adding them to the driver new_id, binding the device, then removing the ID, but it also provides a couple advantages. First, the above existing process allows the driver to bind to any device matching the new_id for the window where it's enabled. This is often not desired, such as the case of trying to bind a single device to a meta driver like pci-stub or vfio-pci. Using driver_override we can do this deterministically using: echo pci-stub /sys/bus/pci/devices/:03:00.0/driver_override echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Previously we could not invoke drivers_probe after adding a device to new_id for a driver as we get non-deterministic behavior whether the driver we intend or the standard driver will claim the device. Now it becomes a deterministic process, only the driver matching driver_override will probe the device. To return the device to the standard driver, we simply clear the driver_override and reprobe the device: echo /sys/bus/pci/devices/:03:00.0/preferred_driver echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (preferred driver = none) or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Signed-off-by: Alex Williamson alex.william...@redhat.com --- Reviewed-by: Stuart Yoder stuart.yo...@freescale.com
RE: [PATCH] driver core: platform: add device binding path 'driver_override'
-Original Message- From: Kim Phillips [mailto:kim.phill...@freescale.com] Sent: Tuesday, April 08, 2014 8:47 PM To: Alex Williamson; gre...@linuxfoundation.org Cc: bhelg...@google.com; linux-...@vger.kernel.org; k...@vger.kernel.org; konrad.w...@oracle.com; Yoder Stuart-B08248; libvir-l...@redhat.com; io...@lists.linux-foundation.org; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; linux-kernel@vger.kernel.org; Guenter Roeck; christoffer.d...@linaro.org Subject: [PATCH] driver core: platform: add device binding path 'driver_override' Needed by platform device drivers, such as the vfio-platform driver [1], in order to bypass the existing OF, ACPI, id_table and name string matches, and successfully be able to be bound to any device, like so: echo vfio-platform /sys/bus/platform/devices/fff51000.ethernet/driver_override echo fff51000.ethernet /sys/bus/platform/devices/fff51000.ethernet/driver/unbind echo fff51000.ethernet /sys/bus/platform/drivers_probe This mimics PCI: Introduce new device binding path using pci_dev.driver_override [2], which is an interface enhancement for more deterministic PCI device binding, e.g., when in the presence of hotplug. [1] http://lkml.iu.edu/hypermail/linux/kernel/1402.1/00177.html [2] http://lists-archives.com/linux-kernel/28030441-pci-introduce-new- device-binding-path-using-pci_dev-driver_override.html Suggested-by: Alex Williamson alex.william...@redhat.com Signed-off-by: Kim Phillips kim.phill...@freescale.com --- Reviewed-by: Stuart Yoder stuart.yo...@freescale.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: mechanism to allow a driver to bind to any device
-Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Wednesday, March 26, 2014 5:09 PM To: Alexander Graf Cc: k...@vger.kernel.org; jan.kis...@siemens.com; will.dea...@arm.com; Yoder Stuart-B08248; a.r...@virtualopensystems.com; Michal Hocko; Wood Scott-B07421; Sethi Varun-B16395; kvm...@lists.cs.columbia.edu; Rafael J. Wysocki; Guenter Roeck; Dmitry Kasatkin; Tejun Heo; Bjorn Helgaas; Antonios Motakis; t...@virtualopensystems.com; Toshi Kani; Greg KH; linux-kernel@vger.kernel.org; io...@lists.linux-foundation.org; Joe Perches; christoffer.d...@linaro.org Subject: Re: mechanism to allow a driver to bind to any device On Wed, 2014-03-26 at 10:21 -0600, Alex Williamson wrote: On Wed, 2014-03-26 at 23:06 +0800, Alexander Graf wrote: Am 26.03.2014 um 22:40 schrieb Konrad Rzeszutek Wilk konrad.w...@oracle.com: On Wed, Mar 26, 2014 at 01:40:32AM +, Stuart Yoder wrote: Hi Greg, We (Linaro, Freescale, Virtual Open Systems) are trying get an issue closed that has been perculating for a while around creating a mechanism that will allow kernel drivers like vfio can bind to devices of any type. This thread with you: http://www.spinics.net/lists/kvm-arm/msg08370.html ...seems to have died out, so am trying to get your response and will summarize again. Vfio drivers in the kernel (regardless of bus type) need to bind to devices of any type. The driver's function is to simply export hardware resources of any type to user space. There are several approaches that have been proposed: You seem to have missed the one I proposed. 1. new_id -- (current approach) the user explicitly registers each new device type with the vfio driver using the new_id mechanism. Problem: multiple drivers will be resident that handle the same device type...and there is nothing user space hotplug infrastructure can do to help. 2. any id -- the vfio driver could specify a wildcard match of some kind in its ID match table which would allow it to match and bind to any possible device id. However, we don't want the vfio driver grabbing _all_ devices...just the ones we explicitly want to pass to user space. The proposed patch to support this was to create a new flag sysfs_bind_only in struct device_driver. When this flag is set, the driver can only bind to devices via the sysfs bind file. This would allow the wildcard match to work. Patch is here: https://lkml.org/lkml/2013/12/3/253 3. Driver initiated explicit bind -- with this approach the vfio driver would create a private 'bind' sysfs object and the user would echo the requested device into it: echo 0001:03:00.0 /sys/bus/pci/drivers/vfio-pci/vfio_bind In order to make that work, the driver would need to call driver_probe_device() and thus we need this patch: https://lkml.org/lkml/2014/2/8/175 4). Use the 'unbind' (from the original device) and 'bind' to vfio driver. This is approach 2, no? Which I think is what is currently being done. Why is that not sufficient? How would 'bind to vfio driver' look like? The only thing I see in the URL is That works, but it is ugly. There is some mention of race but I don't see how - if you do the 'unbind' on the original driver and then bind the BDF to the VFIO how would you get a race? Typically on PCI, you do a - add wildcard (pci id) match to vfio driver - unbind driver - reprobe - device attaches to vfio driver because it is the least recent match - remove wildcard match from vfio driver If in between you hotplug add a card of the same type, it gets attached to vfio - even though the logical default driver would be the device specific driver. I've mentioned drivers_autoprobe in the past, but I'm not sure we're really factoring it into the discussion. drivers_autoprobe allows us to toggle two points: a) When a new device is added whether we automatically give drivers a try at binding to it b) When a new driver is added whether it gets to try to bind to anything in the system So we do have a mechanism to avoid the race, but the problem is that it becomes the responsibility of userspace to: 1) turn off drivers_autoprobe 2) unbind/new_id/bind/remove_id 3) turn on drivers_autoprobe 4) call drivers_probe for anything added between 1) 3) Is the question about the ugliness of the current solution whether it's unreasonable to ask userspace to do this? What we seem to be asking for above is more like an autoprobe flag per driver where there's some way for this special driver to opt out of auto probing. Option
RE: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
-Original Message- From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] Sent: Thursday, December 19, 2013 2:34 PM To: Wood Scott-B07421 Cc: Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.d...@linaro.org; alex.william...@redhat.com; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395 Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only On Thu, Dec 19, 2013 at 02:22:11PM -0600, Scott Wood wrote: On Wed, 2013-12-18 at 17:07 -0800, Greg Kroah-Hartman wrote: On Tue, Dec 03, 2013 at 12:34:46PM +, Kim Phillips wrote: VFIO supports pass-through of devices to user space - for sake of illustration, say a PCI e1000 device: - the e1000 is first unbound from the PCI e1000 driver via sysfs - the vfio-pci driver is told via new_id that it now handles e1000 devices - the e1000 is explicitly bound to vfio-pci through sysfs However, now we have two drivers in the system that both handle e1000 devices. A hotplug event could then occur and it is ambiguous as to which driver will claim the device. The desired semantics is that vfio- pci is only bound to devices by explicit request in sysfs. This patch makes this possible by introducing a sysfs_bind_only flag in struct device_driver. Why deal with this at all and not just deal with the bind sysfs file instead? That way no driver core logic needs to be changed at all, and your userspace tools know _exactly_ which device is being bound to the new device. Don't mess with the new_id file for stuff like this, as you point out, it's tricky... As discussed before, bind does not bypass the ID checks, and thus it does not work without either new_id or a wildcard match. Ah, forgot about that. Or are you proposing changing bind so that it does bypass the ID checks? Or perhaps a new force_bind file that does? No. But you can use bind/unbind along with the existing new_id file to get what you want today. Yes, but that only works for PCI. There is no such concept for platform drivers. If you just happen to bind a device to a wrong driver for a while, that's not really a problem, right? It's annoying but not the end of the world. I don't like this patch as we are adding lots of special and odd logic to the core, for use by almost no one, which ensures that it will never get tested, and will probably get broken in some subtle way in the future. It certainly will be used by users of vfio-platform. Here is the problem-- the new platform device match_any_dev mechanism in patch 2 of this series is not going to work without sysfs_bind_only. A platform driver that just sets match_any_dev will grab any or all platform devices during normal bus probing. Heck, I can't even ensure that you got it right now, with this tiny patch, how do you know it works given that there are no users of this flag anywhere (hint, you never showed me any...) It has been tested on both vfio-pci and vfio-platform in some limited experiments as far as I know, but that code is not ready for upstream yet. Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only
-Original Message- From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] Sent: Thursday, December 19, 2013 4:32 PM To: Wood Scott-B07421 Cc: Yoder Stuart-B08248; Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777; christoffer.d...@linaro.org; alex.william...@redhat.com; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395 Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only On Thu, Dec 19, 2013 at 04:15:03PM -0600, Scott Wood wrote: On Thu, 2013-12-19 at 13:43 -0800, Greg Kroah-Hartman wrote: On Thu, Dec 19, 2013 at 09:06:21PM +, Stuart Yoder wrote: -Original Message- From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] Sent: Thursday, December 19, 2013 2:34 PM To: Wood Scott-B07421 Cc: Kim Phillips; linux-kernel@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777; Yoder Stuart-B08248; christoffer.d...@linaro.org; alex.william...@redhat.com; a.mota...@virtualopensystems.com; ag...@suse.de; Sethi Varun-B16395 Subject: Re: [REPOST][PATCH 1/2] driver core: Add new device_driver flag to allow binding via sysfs only No. But you can use bind/unbind along with the existing new_id file to get what you want today. Yes, but that only works for PCI. No, not only PCI. There is no such concept for platform drivers. Then fix that. We've already explained why that would be bad. No you haven't, or if you have, my squirrel-brain doesn't remember it... Or make your device not be a platform device, odds are that's the better solution in the end, right? How would that solve anything? We'd just be talking about there not being such a mechanism for the device tree bus instead. Nope, you could add it there, like PCI and other busses have. I don't like this patch as we are adding lots of special and odd logic to the core, for use by almost no one, which ensures that it will never get tested, and will probably get broken in some subtle way in the future. It certainly will be used by users of vfio-platform. Here is the problem-- the new platform device match_any_dev mechanism in patch 2 of this series is not going to work without sysfs_bind_only. A platform driver that just sets match_any_dev will grab any or all platform devices during normal bus probing. No it will not, it will fail in the probe function as it knows to not grab the device, just like any driver for other busses that say it can handle all Intel PCI devices and the like. How will it know not to grab the device? The knowledge of whether the binding was explicitly requested or not does not get passed through to the probe function. Nor should it, as a driver should not know, nor care about this. It's up to the BUS to handle this if it really wants to, and I'm afraid that I really am not convinced that the driver core needs to handle it either. But again, as you don't have anything that could actually use this code that is mergable, it's a totally moot point, sorry. Understand, but what assumption do we develop vfio-plaform with? That a driver core 'sysfs_bind_only' flag is not an option, period? If that is the case we need to go back to square one and invent some new mechanism to bind devices to the vfio-platform driver. I guess it would need to be the platform bus equivalent of new_id. But, then we're left with the potential racy situation where multiple drivers can potentially grab a device and it's ambiguous and non-deterministic at to which driver binds to it. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 03/11] PCI/MSI: Refactor pci_dev_msi_enabled()
On Fri, Jul 25, 2014 at 10:08 PM, Yijing Wang wangyij...@huawei.com wrote: Pci_dev_msi_enabled() is used to check whether device MSI/MSIX enabled. Refactor this function to suuport checking only device MSI or MSIX enabled. Signed-off-by: Yijing Wang wangyij...@huawei.com So this patch refactors things so that checks like this: - if (!dev-msi_enabled) are moved into a function: + if (!pci_dev_msi_enabled(dev, MSI_TYPE)) Can you explain a bit more why this needed. Is it just cleanup? Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
location for new bus driver-- drivers/bus? drivers/misc?
Greg, We (Freescale) have a patch series nearly ready to send out for a new bus driver. It's for a block we call the Freescale 'Management Complex' which manages discoverable hardware objects. The Linux bus driver enumerates these objects, binds discovered objects to drivers just like PCI or any normal bus driver. Question is-- where should this go in the kernel and who would the maintainer be to cc: the RFC? I don't think it warrants a top level driver directory beside pci, usb, etc. Most likely place is drivers/bus or drivers/misc (?). We currently have it in drivers/bus, but as far as I can see there is no maintainer per se for that directory. You and Arnd manage misc so thought I would ask. Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
-Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Wednesday, October 22, 2014 1:33 PM To: Marcel Apfelbaum Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. Perhaps: driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform Finding delimiters that don't conflict may be challenging. I think what you proposed works-- bus-name,bus-dev=driver; Think that will work for PCI, platform, and the new fsl-mc bus we are working on. Also, can we assume that bus-name:dev-name is unique for every bustype? It is for pci, platform? I think that has to be the case. It also seems like there's a question of how long should this override last and how does the user disable it? Isn't that a general question for the driver_overrride mechanism? I'm forgetting if the mechanism in the kernel now has a way to disable it-- e.g. echo /dev/null /sys/pci/devices/.../driver_override ?? So, it would last until explicitly disabled through sysfs. I think with pci-stub.ids= $VENDOR:$DEVICE a user can echo the IDs to the pci-stub/remove_id sysfs entry to cancel the effect. The only option here seems to be a reboot. Do we need a /sys/bus/pci/driver_overrides/{add_name,remove_name} for this interface? Thanks, Thanks, Stuart
RE: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
-Original Message- From: Marcel Apfelbaum [mailto:marce...@redhat.com] Sent: Thursday, October 23, 2014 7:32 AM To: Alex Williamson Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote: [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. The real question here if those bus types/devices would benefit from this feature, and I also must confess that I have no knowledge of the other buses. Can anyone confirm that it does make sense for them? We don't have vfio-platform in use yet, so not much actual real world user experience yet. But, I think it makes sense. Especially, given that we are inventing a kernel parameter I think we should design the syntax so that it can work buses can implement support for this. The driver_override mechanism is not bus specific, so let's not make the kernel parameter bus specific. Perhaps: driver_override=pci,:02:00.0=pci-stub;platform,fakename=vfio-platform Finding delimiters that don't conflict may be challenging. Also, can we assume that bus-name:dev-name is unique for every bustype? It is for pci, platform? For PCI, sure the domain:bus:dev.func is unique, for platform I have no idea, can anyone that knows platform confirm or deny? Yes, dev-name will be unique. All platform devices are under a single platform bus. Stuart
RE: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping.
-Original Message- From: Marcel Apfelbaum [mailto:marce...@redhat.com] Sent: Thursday, October 23, 2014 8:37 AM To: Alex Williamson Cc: linux-...@vger.kernel.org; bhelg...@google.com; linux-kernel@vger.kernel.org; mar...@redhat.com; m...@redhat.com; Yoder Stuart-B08248 Subject: Re: [PATCH v4] PCI: add kernel parameter to override devid-driver mapping. On Thu, 2014-10-23 at 07:11 -0600, Alex Williamson wrote: On Thu, 2014-10-23 at 15:32 +0300, Marcel Apfelbaum wrote: On Wed, 2014-10-22 at 12:32 -0600, Alex Williamson wrote: [cc+ stuart] On Mon, 2014-10-20 at 17:04 +0300, Marcel Apfelbaum wrote: Scanning a lot of devices during boot requires a lot of time. On other scenarios there is a need to bind a driver to a specific slot. Binding devices to pci-stub driver does not work, as it will not differentiate between devices of the same type. Using some start scripts is error prone. The solution leverages driver_override functionality introduced by commit: 782a985d7af26db39e86070d28f987cad21313c0 Author: Alex Williamson alex.william...@redhat.com Date: Tue May 20 08:53:21 2014 -0600 PCI: Introduce new device binding path using pci_dev.driver_override In order to bind PCI slots to specific drivers use: pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar,... Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- v3 - v4: - Addressed Alex Williamson's comments: - Modified the type of driver_override_entry's fields - Used PCI_DEVFN when appropriated - Removed redundant checks - Replaced BUG_ON with pr_err messages - Simpler command line parsing - Addressed Michael S. Tsirkin comments - removed DRIVER_OVERRIDE_NAME_LENGTH limitation v2 - v3: - Corrected subject line v1 - v2: - Addressed Michael S. Tsirkin comments - Removed 32 slots limitation - Better handling of memory allocation failures (preferred BUG_ON over error messages) - Addressed Alex Williamson's comments: - Modified commit message to show parameter usage more clear. - I preferred to re-use parse_args instead of manually using strstr in order to better comply with command line parsing rules. - I didn't use any locking when parsing the command line args (see parse_done usage) assuming that first call will be early in system boot and no race can occur. Please correct me if I am wrong. Notes: - I have further ideas on top of this patch based on your reviews. I thought of: - Use wildcards to specify entire buses/devices, something like: driver[0001:02:*.*]=pci-stub - Use comma to separate several devices: driver[0001:02:03.4,0001:02:04.0,...]=pci-stub - Make domain optional: driver[00:03.0]=pci-stub Comments will be appreciated, Thanks, Marcel Documentation/kernel-parameters.txt | 4 ++ drivers/pci/bus.c | 111 drivers/pci/pci.c | 2 + 3 files changed, 117 insertions(+) The driver_override feature that we're making use of here is also going to be supported by platform devices and potentially more bustypes in the future, so I'm concerned that making a pci specific kernel parameter is too shortsighted. Instead we could hook on to BUS_NOTIFY_ADD_DEVICE for bustypes that support driver_override so we can have a common interface. The real question here if those bus types/devices would benefit from this feature, and I also must confess that I have no knowledge of the other buses. Can anyone confirm that it does make sense for them? Platform devices are adding vfio support, so I expect the next logical question will be how to reserve devices for use by vfio at boot. Well, I'll have to learn more about vfio before saying anything, but my question is if it can be deferred or it has to be part of this patch. If the platform devices do not have a slot like hw address, maybe it can be configured separately. I saw this patch as a PCI patch, and not driver_override expansion; meaning that I only leveraged an existing feature, not trying to extend it. I think other buses may want to use the same mechanism to specify driver bindings, so I think the main thing is to not define a syntax that makes that problematic. Looking at your original syntax, I think it could work for different bus types. Could have something like: platform=driver[]=foo,driver[]=vfio-platform pci=driver[:xx:xx.x]=foo,driver[:xx:xx.x]=bar So no strong opinion on that vs Alex's proposal: driver_override=pci,:02:00.0=pci-stub;platform,=vfio-platform It
RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: Wood Scott-B07421 Sent: Thursday, September 18, 2014 6:29 PM To: Yoder Stuart-B08248 Cc: Phillips Kim-R1AAHA; Alexander Graf; Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs On Thu, 2014-09-18 at 18:13 -0500, Yoder Stuart-B08248 wrote: -Original Message- From: Kim Phillips [mailto:kim.phill...@freescale.com] Sent: Thursday, September 18, 2014 3:23 PM To: Alexander Graf Cc: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org; Yoder Stuart-B08248; Wood Scott-B07421; linuxppc-rele...@linux.freescale.net Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs On Thu, 18 Sep 2014 15:14:03 +0200 Alexander Graf ag...@suse.de wrote: Am 18.09.2014 um 06:17 schrieb German Rivera german.riv...@freescale.com: On 09/15/2014 06:44 PM, Kim Phillips wrote: On Thu, 11 Sep 2014 12:34:21 -0500 J. German Rivera german.riv...@freescale.com wrote: From: J. German Rivera german.riv...@freescale.com APIs to access the Management Complex (MC) hardware module of Freescale LS2 SoCs. This patch includes APIs to check the MC firmware version and to manipulate DPRC objects in the MC. Signed-off-by: J. German Rivera german.riv...@freescale.com Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- drivers/bus/fsl-mc/dpmng.c | 93 + drivers/bus/fsl-mc/dprc.c | 504 +++ drivers/bus/fsl-mc/fsl_dpmng_cmd.h | 83 drivers/bus/fsl-mc/fsl_dprc_cmd.h | 545 + drivers/bus/fsl-mc/fsl_mc_sys.c| 237 +++ include/linux/fsl_dpmng.h | 120 ++ include/linux/fsl_dprc.h | 790 include/linux/fsl_mc_cmd.h | 182 + include/linux/fsl_mc_sys.h | 81 9 files changed, 2635 insertions(+) create mode 100644 drivers/bus/fsl-mc/dpmng.c create mode 100644 drivers/bus/fsl-mc/dprc.c create mode 100644 drivers/bus/fsl-mc/fsl_dpmng_cmd.h create mode 100644 drivers/bus/fsl-mc/fsl_dprc_cmd.h create mode 100644 drivers/bus/fsl-mc/fsl_mc_sys.c create mode 100644 include/linux/fsl_dpmng.h create mode 100644 include/linux/fsl_dprc.h create mode 100644 include/linux/fsl_mc_cmd.h create mode 100644 include/linux/fsl_mc_sys.h the fsl prefix in the filename fsl_dpmng_cmd.h is redundant with its directory name fsl-mc/. Note that I find dashes ('-') in filenames make them easier to type: is there a reason we're using underscores here? This is a convention that we decided early on '-' for directory names and '_' for file names. based on what? We looked at how generally files were named in kernel source. For what it's worth in my 3.16 branch: $ find drivers/ -type f | grep '-' | wc -l 4308 $ find drivers/ -type f | grep '_' | wc -l 6507 ...it seems that there are far more files named with underscores. If Greg or Arnd wants the files renamed, fine, but other than that I see no reason to change this. 60% isn't far more. Regardless of whether we change this (dashes are easier to read, easier to type, and are what English normally uses -- underscores are for when you can't use a dash for technical reasons), let's not start making rules based on weak statistical inferences. Plus, why limit the search to drivers? It's a driver and we looked at the precedent/conventions used by other drivers. E.g. if you looked at arch/ you'd reach the opposite conclusion. I guess if you look at the kernel as a whole it's almost exactly 50/50. So, don't understand how anyone can say it is _clear_ that underscores are better. Stuart
RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: Kim Phillips [mailto:kim.phill...@freescale.com] Sent: Thursday, September 18, 2014 3:23 PM To: Alexander Graf Cc: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org; Yoder Stuart-B08248; Wood Scott-B07421; linuxppc-rele...@linux.freescale.net Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs On Thu, 18 Sep 2014 15:14:03 +0200 Alexander Graf ag...@suse.de wrote: Am 18.09.2014 um 06:17 schrieb German Rivera german.riv...@freescale.com: On 09/15/2014 06:44 PM, Kim Phillips wrote: On Thu, 11 Sep 2014 12:34:21 -0500 J. German Rivera german.riv...@freescale.com wrote: From: J. German Rivera german.riv...@freescale.com APIs to access the Management Complex (MC) hardware module of Freescale LS2 SoCs. This patch includes APIs to check the MC firmware version and to manipulate DPRC objects in the MC. Signed-off-by: J. German Rivera german.riv...@freescale.com Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- drivers/bus/fsl-mc/dpmng.c | 93 + drivers/bus/fsl-mc/dprc.c | 504 +++ drivers/bus/fsl-mc/fsl_dpmng_cmd.h | 83 drivers/bus/fsl-mc/fsl_dprc_cmd.h | 545 + drivers/bus/fsl-mc/fsl_mc_sys.c| 237 +++ include/linux/fsl_dpmng.h | 120 ++ include/linux/fsl_dprc.h | 790 include/linux/fsl_mc_cmd.h | 182 + include/linux/fsl_mc_sys.h | 81 9 files changed, 2635 insertions(+) create mode 100644 drivers/bus/fsl-mc/dpmng.c create mode 100644 drivers/bus/fsl-mc/dprc.c create mode 100644 drivers/bus/fsl-mc/fsl_dpmng_cmd.h create mode 100644 drivers/bus/fsl-mc/fsl_dprc_cmd.h create mode 100644 drivers/bus/fsl-mc/fsl_mc_sys.c create mode 100644 include/linux/fsl_dpmng.h create mode 100644 include/linux/fsl_dprc.h create mode 100644 include/linux/fsl_mc_cmd.h create mode 100644 include/linux/fsl_mc_sys.h the fsl prefix in the filename fsl_dpmng_cmd.h is redundant with its directory name fsl-mc/. Note that I find dashes ('-') in filenames make them easier to type: is there a reason we're using underscores here? This is a convention that we decided early on '-' for directory names and '_' for file names. based on what? We looked at how generally files were named in kernel source. For what it's worth in my 3.16 branch: $ find drivers/ -type f | grep '-' | wc -l 4308 $ find drivers/ -type f | grep '_' | wc -l 6507 ...it seems that there are far more files named with underscores. If Greg or Arnd wants the files renamed, fine, but other than that I see no reason to change this. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
+/** + * @briefDisconnects one endpoint to remove its network link + * + * @param[in] mc_ioPointer to opaque I/O object + * @param[in]dprc_handleHandle to the DPRC object + * @param[in] endpointEndpoint configuration parameters. + * + * @returns'0' on Success; Error code otherwise. + * */ +int dprc_disconnect(struct fsl_mc_io *mc_io, uint16_t dprc_handle, +struct dprc_endpoint *endpoint); + +/*! @} */ this entire file is riddled with non-kernel-doc comment markers: see Documentation/kernel-doc-nano-HOWTO.txt on how to write function and other types of comments in a kernel-doc compliant format. This is because this file is using doxygen comments, as it was developed by another team. Unless someone else has an objection, I will leave the doxygen comments alone and not make any change here. Do you see any other source files in Linux using doxygen comments? Yes. Grep around a bit and you'll see examples of it. I grep'ed for some doxygen tags and found close to 200 source files with them. Mixing different documentation styles can easily become a big mess, because you can't generate external documentation consistently for the whole tree. As German mentioned elsewhere, this file is an interface to a hardware block, was written by another team targetting a wide variety of environments-- u-boot, Linux, user space, other OSes etc. We left the doxygen stuff there because while admitedly not used much, there are other examples of it in the kernel and the documentation seems useful. If it can't go into the kernel as is, we can just delete it. If doxygen is not allowed, it would be nice to see checkpatch updated to complain. Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: Kim Phillips [mailto:kim.phill...@freescale.com] Sent: Monday, September 15, 2014 6:45 PM To: Rivera Jose-B46482 Cc: gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org; Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; ag...@suse.de; linuxppc-rele...@linux.freescale.net Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs On Thu, 11 Sep 2014 12:34:21 -0500 J. German Rivera german.riv...@freescale.com wrote: From: J. German Rivera german.riv...@freescale.com APIs to access the Management Complex (MC) hardware module of Freescale LS2 SoCs. This patch includes APIs to check the MC firmware version and to manipulate DPRC objects in the MC. Signed-off-by: J. German Rivera german.riv...@freescale.com Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- drivers/bus/fsl-mc/dpmng.c | 93 + drivers/bus/fsl-mc/dprc.c | 504 +++ drivers/bus/fsl-mc/fsl_dpmng_cmd.h | 83 drivers/bus/fsl-mc/fsl_dprc_cmd.h | 545 + drivers/bus/fsl-mc/fsl_mc_sys.c| 237 +++ include/linux/fsl_dpmng.h | 120 ++ include/linux/fsl_dprc.h | 790 include/linux/fsl_mc_cmd.h | 182 + include/linux/fsl_mc_sys.h | 81 9 files changed, 2635 insertions(+) create mode 100644 drivers/bus/fsl-mc/dpmng.c create mode 100644 drivers/bus/fsl-mc/dprc.c create mode 100644 drivers/bus/fsl-mc/fsl_dpmng_cmd.h create mode 100644 drivers/bus/fsl-mc/fsl_dprc_cmd.h create mode 100644 drivers/bus/fsl-mc/fsl_mc_sys.c create mode 100644 include/linux/fsl_dpmng.h create mode 100644 include/linux/fsl_dprc.h create mode 100644 include/linux/fsl_mc_cmd.h create mode 100644 include/linux/fsl_mc_sys.h the fsl prefix in the filename fsl_dpmng_cmd.h is redundant with its directory name fsl-mc/. Note that I find dashes ('-') in filenames make them easier to type: is there a reason we're using underscores here? Also, any reason why these and future include files aren't being put in include/linux/fsl/, so as to not pollute the top level include/linux/? That way, we can also remove the fsl- prefix from those filenames, too.. There is nothing in include/linux/fsl except 1 driver from 2 years ago. Why is that? I'd also prefer to see these headers is include/linux/fsl, but given that the directory is essentially unused it seemed that include/linux was more conventional. diff --git a/drivers/bus/fsl-mc/dpmng.c b/drivers/bus/fsl-mc/dpmng.c new file mode 100644 index 000..c6ed27c --- /dev/null +++ b/drivers/bus/fsl-mc/dpmng.c @@ -0,0 +1,93 @@ +/* Copyright 2013-2014 Freescale Semiconductor Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of Freescale Semiconductor nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * + * ALTERNATIVELY, this software may be distributed under the terms of the + * GNU General Public License (GPL) as published by the Free Software + * Foundation, either version 2 of that License or (at your option) any + * later version. + * + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY interesting, normally this text reads: THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ...does that mean we're excluding non-Freescale copyright holders and contributors from this warranty statement? That doesn't seem appropriate for an upstream kernel submission. This is a standard license dual license used by Freescale, wording most likely set by the legal dept. There are many instances of it in the kernel (mostly in device trees). git grep 'THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor' | wc -l 180 Unless it is condition of being accepted into the kernel I want to leave it as is. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: Yoder Stuart-B08248 Sent: Thursday, September 18, 2014 7:19 PM To: 'Alexander Graf'; Rivera Jose-B46482 Cc: Phillips Kim-R1AAHA; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org; Wood Scott-B07421; linuxppc-rele...@linux.freescale.net Subject: RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs +/** + * @briefDisconnects one endpoint to remove its network link + * + * @param[in] mc_ioPointer to opaque I/O object + * @param[in]dprc_handleHandle to the DPRC object + * @param[in] endpointEndpoint configuration parameters. + * + * @returns'0' on Success; Error code otherwise. + * */ +int dprc_disconnect(struct fsl_mc_io *mc_io, uint16_t dprc_handle, +struct dprc_endpoint *endpoint); + +/*! @} */ this entire file is riddled with non-kernel-doc comment markers: see Documentation/kernel-doc-nano-HOWTO.txt on how to write function and other types of comments in a kernel-doc compliant format. This is because this file is using doxygen comments, as it was developed by another team. Unless someone else has an objection, I will leave the doxygen comments alone and not make any change here. Do you see any other source files in Linux using doxygen comments? Yes. Grep around a bit and you'll see examples of it. I grep'ed for some doxygen tags and found close to 200 source files with them. Mixing different documentation styles can easily become a big mess, because you can't generate external documentation consistently for the whole tree. As German mentioned elsewhere, this file is an interface to a hardware block, was written by another team targetting a wide variety of environments-- u-boot, Linux, user space, other OSes etc. We left the doxygen stuff there because while admitedly not used much, there are other examples of it in the kernel and the documentation seems useful. If it can't go into the kernel as is, we can just delete it. ...to be clear, we could just delete the doyxen tags. There is no scenario where I would envision anyone generating documentation from these files in the kernel. The tags are there because of where we grabbed the source from. It certainly would benefit no one by conversion to kerneldoc. However, just leaving the comments and doxygen tags alone as is would be nice. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
+/** + * @briefManagement Complex firmware version information + */ +#define MC_VER_MAJOR 2 +#define MC_VER_MINOR 0 code should be adjusted to run on all *compatible* versions of h/w, not strictly the one set in these defines. This comment is not precise enough be actionable. What exactly you want to be changed here? I think the easy thing to do is to convert the exact version check into a ranged version check: have minimum and maximum versions you support. Or a list of exact versions you support. Or not check for the version at all - or only for the major version and guarantee that the major version indicates backwards compatibility. yes, this was my point: elsewhere I noticed the code denies to run iff those defines are not matched exactly: that code should change to run as Alex describes. As I mentioned in the reply to Alex, I will remove the minor version check. the code should be able to run on all subsequent versions of the h/w, even in the major version case. You're right, in the future if there are future major versions we would want this same driver to function on multiple versions of the hardware. But at this point in time we don't know what future evolutions there will be and we need the check to error out for now. The driver will have to be changed in the future to dynamically deal with different versions. We could add a TODO in the driver to note that. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: Kim Phillips [mailto:kim.phill...@freescale.com] Sent: Friday, September 19, 2014 3:25 PM To: Yoder Stuart-B08248 Cc: Rivera Jose-B46482; Alexander Graf; gre...@linuxfoundation.org; a...@arndb.de; linux- ker...@vger.kernel.org; Wood Scott-B07421; linuxppc-rele...@linux.freescale.net Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs On Fri, 19 Sep 2014 14:06:32 -0500 Yoder Stuart-B08248 stuart.yo...@freescale.com wrote: +/** + * @briefManagement Complex firmware version information + */ +#define MC_VER_MAJOR 2 +#define MC_VER_MINOR 0 code should be adjusted to run on all *compatible* versions of h/w, not strictly the one set in these defines. This comment is not precise enough be actionable. What exactly you want to be changed here? I think the easy thing to do is to convert the exact version check into a ranged version check: have minimum and maximum versions you support. Or a list of exact versions you support. Or not check for the version at all - or only for the major version and guarantee that the major version indicates backwards compatibility. yes, this was my point: elsewhere I noticed the code denies to run iff those defines are not matched exactly: that code should change to run as Alex describes. As I mentioned in the reply to Alex, I will remove the minor version check. the code should be able to run on all subsequent versions of the h/w, even in the major version case. You're right, in the future if there are future major versions we would want this same driver to function on multiple versions of the hardware. But at this point in time we don't know what future evolutions there will be and we need the check to error out for now. why? We have to make the standard assumption that newer versions will be backward compatible, in which case the driver should be left to run. If future changes are backward compatible then only the minor version changes. That is the explicit definition of the meaning of major/minor version for this device. If the major number changes, then it is not backward compatible and will require driver changes. So this error out check is temporary until we reach that future point, where it can be removed and we can hopefully implement dynamic handling of the incompatibility. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, September 19, 2014 3:33 PM To: Phillips Kim-R1AAHA; Yoder Stuart-B08248 Cc: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org; Wood Scott-B07421; linuxppc-rele...@linux.freescale.net Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs On 19.09.14 22:24, Kim Phillips wrote: On Fri, 19 Sep 2014 14:06:32 -0500 Yoder Stuart-B08248 stuart.yo...@freescale.com wrote: +/** + * @briefManagement Complex firmware version information + */ +#define MC_VER_MAJOR 2 +#define MC_VER_MINOR 0 code should be adjusted to run on all *compatible* versions of h/w, not strictly the one set in these defines. This comment is not precise enough be actionable. What exactly you want to be changed here? I think the easy thing to do is to convert the exact version check into a ranged version check: have minimum and maximum versions you support. Or a list of exact versions you support. Or not check for the version at all - or only for the major version and guarantee that the major version indicates backwards compatibility. yes, this was my point: elsewhere I noticed the code denies to run iff those defines are not matched exactly: that code should change to run as Alex describes. As I mentioned in the reply to Alex, I will remove the minor version check. the code should be able to run on all subsequent versions of the h/w, even in the major version case. You're right, in the future if there are future major versions we would want this same driver to function on multiple versions of the hardware. But at this point in time we don't know what future evolutions there will be and we need the check to error out for now. why? We have to make the standard assumption that newer versions will be backward compatible, in which case the driver should be left to run. How much is the interface set in stone? It is set in stone, in that a particular version has an specific set of commands. Can we indicate to the MC that we want version x of the protocol? Then the MC can tell us whether it's compatible or not. No, it's not that clever. For the DPRC object there is a set of commands with a binary interface. The major/minor version number tell us what version of the hw object we are dealing with...what commands are there, what version of the binary interface is there. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
Speaking of which, how does Linux find this new bus? I couldn't find anything that describes the device tree bindings, but maybe I just missed them. There is a separate thread on the ARM mailing list with the device tree and binding (device tree will be in arch/arm64): http://www.spinics.net/lists/arm-kernel/msg354635.html Stuart N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a��� 0��h���i
RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: Kim Phillips [mailto:kim.phill...@freescale.com] Sent: Friday, September 19, 2014 3:58 PM To: Yoder Stuart-B08248 Cc: Alexander Graf; Rivera Jose-B46482; Phillips Kim-R1AAHA; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org; Wood Scott-B07421; linuxppc-rele...@linux.freescale.net Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs On Fri, 19 Sep 2014 13:25:06 -0500 Yoder Stuart-B08248 stuart.yo...@freescale.com wrote: From: Yoder Stuart-B08248 Sent: Thursday, September 18, 2014 7:19 PM +/** + * @briefDisconnects one endpoint to remove its network link + * + * @param[in] mc_ioPointer to opaque I/O object + * @param[in]dprc_handleHandle to the DPRC object + * @param[in] endpointEndpoint configuration parameters. + * + * @returns'0' on Success; Error code otherwise. + * */ +int dprc_disconnect(struct fsl_mc_io *mc_io, uint16_t dprc_handle, +struct dprc_endpoint *endpoint); + +/*! @} */ this entire file is riddled with non-kernel-doc comment markers: see Documentation/kernel-doc-nano-HOWTO.txt on how to write function and other types of comments in a kernel-doc compliant format. This is because this file is using doxygen comments, as it was developed by another team. Unless someone else has an objection, I will leave the doxygen comments alone and not make any change here. Do you see any other source files in Linux using doxygen comments? Yes. Grep around a bit and you'll see examples of it. I grep'ed for some doxygen tags and found close to 200 source files with them. grepping for the one in this patch above - ! @} - returns nothing. Mixing different documentation styles can easily become a big mess, because you can't generate external documentation consistently for the whole tree. As German mentioned elsewhere, this file is an interface to a hardware block, was written by another team targetting a wide variety of environments-- u-boot, Linux, user space, other OSes etc. We left the doxygen stuff there because while admitedly not used much, there are other examples of it in the kernel and the documentation seems useful. If it can't go into the kernel as is, we can just delete it. ...to be clear, we could just delete the doyxen tags. There is no scenario where I would envision anyone generating documentation from these files in the kernel. The tags are there because of where we grabbed the source from. It certainly would benefit no one by conversion to kerneldoc. except kerneldoc users :) However, just leaving the comments and doxygen tags alone as is would be nice. they are incompatible with kerneldoc. Seriously, no one is going to ever generate docs from this obscure bit of code documenting an internal interface within this driver. Makes no sense to convert the comments to kerneldoc. I completely get what you are saying with respect to comments actually documenting kernel interfaces used by different kernel components or uapi interfaces. The doxygen tags are there because this code originated in a different project. That code was intended to be Linux-style compliant and complies with checkpatch --strict. However, we can sanitize the code and remove the tags if they are causing grief. It's too bad though, because we'll have to fork this interface code from its origin. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org] Sent: Monday, September 22, 2014 9:58 AM To: Yoder Stuart-B08248 Cc: Phillips Kim-R1AAHA; Alexander Graf; Rivera Jose-B46482; a...@arndb.de; linux-kernel@vger.kernel.org; Wood Scott-B07421; linuxppc-rele...@linux.freescale.net Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs On Mon, Sep 22, 2014 at 02:42:10PM +, Stuart Yoder wrote: -Original Message- From: Kim Phillips [mailto:kim.phill...@freescale.com] Sent: Friday, September 19, 2014 3:58 PM To: Yoder Stuart-B08248 Cc: Alexander Graf; Rivera Jose-B46482; Phillips Kim-R1AAHA; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org; Wood Scott-B07421; linuxppc-rele...@linux.freescale.net Subject: Re: [PATCH 1/4] drivers/bus: Added Freescale Management Complex APIs On Fri, 19 Sep 2014 13:25:06 -0500 Yoder Stuart-B08248 stuart.yo...@freescale.com wrote: From: Yoder Stuart-B08248 Sent: Thursday, September 18, 2014 7:19 PM +/** + * @briefDisconnects one endpoint to remove its network link + * + * @param[in] mc_ioPointer to opaque I/O object + * @param[in]dprc_handleHandle to the DPRC object + * @param[in] endpointEndpoint configuration parameters. + * + * @returns'0' on Success; Error code otherwise. + * */ +int dprc_disconnect(struct fsl_mc_io *mc_io, uint16_t dprc_handle, +struct dprc_endpoint *endpoint); + +/*! @} */ this entire file is riddled with non-kernel-doc comment markers: see Documentation/kernel-doc-nano-HOWTO.txt on how to write function and other types of comments in a kernel-doc compliant format. This is because this file is using doxygen comments, as it was developed by another team. Unless someone else has an objection, I will leave the doxygen comments alone and not make any change here. Do you see any other source files in Linux using doxygen comments? Yes. Grep around a bit and you'll see examples of it. I grep'ed for some doxygen tags and found close to 200 source files with them. grepping for the one in this patch above - ! @} - returns nothing. Mixing different documentation styles can easily become a big mess, because you can't generate external documentation consistently for the whole tree. As German mentioned elsewhere, this file is an interface to a hardware block, was written by another team targetting a wide variety of environments-- u-boot, Linux, user space, other OSes etc. We left the doxygen stuff there because while admitedly not used much, there are other examples of it in the kernel and the documentation seems useful. If it can't go into the kernel as is, we can just delete it. ...to be clear, we could just delete the doyxen tags. There is no scenario where I would envision anyone generating documentation from these files in the kernel. The tags are there because of where we grabbed the source from. It certainly would benefit no one by conversion to kerneldoc. except kerneldoc users :) However, just leaving the comments and doxygen tags alone as is would be nice. they are incompatible with kerneldoc. Seriously, no one is going to ever generate docs from this obscure bit of code documenting an internal interface within this driver. Makes no sense to convert the comments to kerneldoc. I completely get what you are saying with respect to comments actually documenting kernel interfaces used by different kernel components or uapi interfaces. The doxygen tags are there because this code originated in a different project. That code was intended to be Linux-style compliant and complies with checkpatch --strict. However, we can sanitize the code and remove the tags if they are causing grief. It's too bad though, because we'll have to fork this interface code from its origin. You forked the code when you asked for it be merged into the kernel tree. You shouldn't ever have to rely on the old version again, so please, remove the doxygen mess. Will do. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series
-Original Message- From: Kim Phillips [mailto:kim.phill...@freescale.com] Sent: Monday, September 22, 2014 11:53 AM To: Rivera Jose-B46482 Cc: gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org; Yoder Stuart-B08248; Wood Scott- B07421; ag...@suse.de; linuxppc-rele...@linux.freescale.net Subject: Re: [PATCH 0/3 v2] drivers/bus: Freescale Management Complex bus driver patch series On Fri, 19 Sep 2014 17:49:38 -0500 J. German Rivera german.riv...@freescale.com wrote: CHANGE HISTORY Issues pending resolution not addressed by v2: - What to do with Doxygen comments in patch 1 It's clear they should be removed. - Whether to move or not FSL-specific header files added in include/linux, by this patch series, to another location there wasn't a valid objection against moving them under fsl/ and changing them to use dashes instead of underscores, was there? There was no objection, but here is the observation. The current convention seems to be that under include/linux are 'subsystem' types-- include/linux/mmc include/linux/spi include/linux/raid etc There is no other company that has an include/linux/[company-name] that I can see. Freescale seems to be the only one. And there is only a single driver in there. So it looks like a complete anomaly. Why is that? I guess we could try moving our stuff to incluce/linux/fsl and see if there is any negative feedback on it. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 10/13] irqchip: GICv3: ITS: DT probing and initialization
On Mon, Nov 24, 2014 at 8:35 AM, Marc Zyngier marc.zyng...@arm.com wrote: Add the code that probes the ITS from the device tree, and initialize it. Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- drivers/irqchip/irq-gic-v3-its.c | 169 +++ 1 file changed, 169 insertions(+) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 532c6df..e9d1615 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -1231,3 +1231,172 @@ static const struct irq_domain_ops its_domain_ops = { .alloc = its_irq_domain_alloc, .free = its_irq_domain_free, }; + +static int its_probe(struct device_node *node, struct irq_domain *parent) +{ + struct resource res; + struct its_node *its; + void __iomem *its_base; + u32 val; + u64 baser, tmp; + int err; + + err = of_address_to_resource(node, 0, res); + if (err) { + pr_warn(%s: no regs?\n, node-full_name); + return -ENXIO; + } + + its_base = ioremap(res.start, resource_size(res)); + if (!its_base) { + pr_warn(%s: unable to map registers\n, node-full_name); + return -ENOMEM; + } + + val = readl_relaxed(its_base + GITS_PIDR2) GIC_PIDR2_ARCH_MASK; + if (val != 0x30 val != 0x40) { + pr_warn(%s: no ITS detected, giving up\n, node-full_name); + err = -ENODEV; + goto out_unmap; + } + + pr_info(ITS: %s\n, node-full_name); + + its = kzalloc(sizeof(*its), GFP_KERNEL); + if (!its) { + err = -ENOMEM; + goto out_unmap; + } + + raw_spin_lock_init(its-lock); + INIT_LIST_HEAD(its-entry); + INIT_LIST_HEAD(its-its_device_list); + its-base = its_base; + its-phys_base = res.start; + its-msi_chip.of_node = node; + its-ite_size = ((readl_relaxed(its_base + GITS_TYPER) 4) 0xf) + 1; + + its-cmd_base = kzalloc(ITS_CMD_QUEUE_SZ, GFP_KERNEL); + if (!its-cmd_base) { + err = -ENOMEM; + goto out_free_its; + } + its-cmd_write = its-cmd_base; + + err = its_alloc_tables(its); + if (err) + goto out_free_cmd; + + err = its_alloc_collections(its); + if (err) + goto out_free_tables; + + baser = (virt_to_phys(its-cmd_base)| +GITS_CBASER_WaWb | +GITS_CBASER_InnerShareable | +(ITS_CMD_QUEUE_SZ / SZ_4K - 1) | +GITS_CBASER_VALID); + + writeq_relaxed(baser, its-base + GITS_CBASER); + tmp = readq_relaxed(its-base + GITS_CBASER); + writeq_relaxed(0, its-base + GITS_CWRITER); + writel_relaxed(1, its-base + GITS_CTLR); + + if ((tmp ^ baser) GITS_BASER_SHAREABILITY_MASK) { + pr_info(ITS: using cache flushing for cmd queue\n); + its-flags |= ITS_FLAGS_CMDQ_NEEDS_FLUSHING; + } + + if (of_property_read_bool(its-msi_chip.of_node, msi-controller)) { + its-domain = irq_domain_add_tree(NULL, its_domain_ops, its); + if (!its-domain) { + err = -ENOMEM; + goto out_free_tables; + } + + its-domain-parent = parent; + + its-msi_chip.domain = pci_msi_create_irq_domain(node, + its_pci_msi_domain_info, +its-domain); + if (!its-msi_chip.domain) { + err = -ENOMEM; + goto out_free_domains; + } + + err = of_pci_msi_chip_add(its-msi_chip); + if (err) + goto out_free_domains; + } Hi Marc, We have a requirement to have both PCI and non-PCI buses use the GIC_ITS. Above, you have the hardcoded assumption that this is PCI. How do 2 different bus types share the ITS at the same time. Thanks, Stuart Yoder Freescale -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, November 26, 2014 4:16 AM To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez Nir-RM30794; Schmitt Richard-B43082 Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs On 13.11.14 18:54, J. German Rivera wrote: APIs to access the Management Complex (MC) hardware module of Freescale LS2 SoCs. This patch includes APIs to check the MC firmware version and to manipulate DPRC objects in the MC. Signed-off-by: J. German Rivera german.riv...@freescale.com Signed-off-by: Stuart Yoder stuart.yo...@freescale.com [...] diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c new file mode 100644 index 000..40ae552 --- /dev/null +++ b/drivers/bus/fsl-mc/dprc.c @@ -0,0 +1,933 @@ +/* Copyright 2013-2014 Freescale Semiconductor Inc. +* +* Redistribution and use in source and binary forms, with or without +* modification, are permitted provided that the following conditions are met: +* * Redistributions of source code must retain the above copyright +* notice, this list of conditions and the following disclaimer. +* * Redistributions in binary form must reproduce the above copyright +* notice, this list of conditions and the following disclaimer in the +* documentation and/or other materials provided with the distribution. +* * Neither the name of the above-listed copyright holders nor the +* names of any contributors may be used to endorse or promote products +* derived from this software without specific prior written permission. +* +* +* ALTERNATIVELY, this software may be distributed under the terms of the +* GNU General Public License (GPL) as published by the Free Software +* Foundation, either version 2 of that License or (at your option) any +* later version. +* +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS AS IS +* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE +* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +* POSSIBILITY OF SUCH DAMAGE. +*/ +#include linux/fsl/mc-sys.h +#include linux/fsl/mc-cmd.h +#include linux/fsl/dprc.h +#include dprc-cmd.h + +int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id) This one is definitely a misnomer. It's a command that operates on the MC object, not a DPRC object. Also it doesn't fetch a random container_id, it fetches the root container id. It's not strictly the root container. It fetches the container/DPRC ID associated with the portal you are using. A virtual machine would use it to fetch it's container ID. Please move it and its definition to the files that operate on the MC management interface. Note, the binary interface opcode really is DPRC_CMDID_GET_CONT_ID. We can request that the binary interface naming be changed, but wouldn't it be better to keep the functions separated by opcode type-- having DPRC_CMDID* opcode-based commands be in one file and DPMNG_CMDID* commands in a separate file? We have already submitted a request to move this opcode to the DPMNG group. Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
+int dprc_get_container_id(struct fsl_mc_io *mc_io, int *container_id) This one is definitely a misnomer. It's a command that operates on the MC object, not a DPRC object. Also it doesn't fetch a random container_id, it fetches the root container id. It's not strictly the root container. It fetches the container/DPRC ID associated with the portal you are using. A virtual machine would use it to fetch it's container ID. So does every portal properly react to this call? Yes, they should. Or do only container portals react to it? Every portal is associated with some contianer-- either built into the container/DPRC object, or in a container/DPRC. In fact, what does make the initial portal special? Nothing...it has the same semantics as any other portal. Who reacts to DPMNG_CMDID_foo calls? Every DPRC or only the initial root? All portals. It just means 'what container am I in?' Please move it and its definition to the files that operate on the MC management interface. Note, the binary interface opcode really is DPRC_CMDID_GET_CONT_ID. We can request that the binary interface naming be changed, but wouldn't it be better to keep the functions separated by opcode type-- having DPRC_CMDID* opcode-based commands be in one file and DPMNG_CMDID* commands in a separate file? It really depends on what the semantics are. If this is a call that's only valid on the MC root, then it should belong there. If it's available on every container portal, it should be part of dprc of course. It is valid on any portal, including container portals. For that reason, I do think it may make sense for it to be a DPMNG* command. But, I would say leave the files implementing the opcode groupings alone until the MC architecture is changed. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, November 27, 2014 8:30 AM To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez Nir-RM30794; Schmitt Richard-B43082 Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs On 13.11.14 18:54, J. German Rivera wrote: APIs to access the Management Complex (MC) hardware module of Freescale LS2 SoCs. This patch includes APIs to check the MC firmware version and to manipulate DPRC objects in the MC. Signed-off-by: J. German Rivera german.riv...@freescale.com Signed-off-by: Stuart Yoder stuart.yo...@freescale.com [...] +/* + * Object descriptor, returned from dprc_get_obj() + */ +struct dprc_obj_desc { + /* Type of object: NULL terminated string */ + char type[16]; I don't see where it actually gets NULL terminated - all 16 bytes come directly from the device. While it's probably ok to trust it, I think we'd still be safer off if we just make this a char[17] array to always have our NULL terminating string. That way we're guaranteed we'll never run over our memory boundaries. The device is supposed to guarantee that the string is null terminated...so there will never be valid chars in the 16th character. So, what about just forcing type[15] = '\0'? I think that would be better than making it a char[17]. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, November 27, 2014 10:14 AM To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez Nir-RM30794; Schmitt Richard-B43082 Subject: Re: [PATCH 1/3 v4] drivers/bus: Added Freescale Management Complex APIs On 13.11.14 18:54, J. German Rivera wrote: APIs to access the Management Complex (MC) hardware module of Freescale LS2 SoCs. This patch includes APIs to check the MC firmware version and to manipulate DPRC objects in the MC. Signed-off-by: J. German Rivera german.riv...@freescale.com Signed-off-by: Stuart Yoder stuart.yo...@freescale.com [...] +/** + * Creates an MC I/O object + * + * @dev: device to be associated with the MC I/O object + * @mc_portal_phys_addr: physical address of the MC portal to use + * @mc_portal_size: size in bytes of the MC portal + * @flags: flags for the new MC I/O object + * @new_mc_io: Area to return pointer to newly created MC I/O object + * + * Returns '0' on Success; Error code otherwise. + */ +int __must_check fsl_create_mc_io(struct device *dev, + phys_addr_t mc_portal_phys_addr, + uint32_t mc_portal_size, + uint32_t flags, struct fsl_mc_io **new_mc_io) +{ + struct fsl_mc_io *mc_io; + void __iomem *mc_portal_virt_addr; + struct resource *res; + + mc_io = devm_kzalloc(dev, sizeof(*mc_io), GFP_KERNEL); + if (mc_io == NULL) + return -ENOMEM; + + mc_io-dev = dev; + mc_io-flags = flags; + mc_io-portal_phys_addr = mc_portal_phys_addr; + mc_io-portal_size = mc_portal_size; + res = devm_request_mem_region(dev, + mc_portal_phys_addr, + mc_portal_size, + mc_portal); + if (res == NULL) { + dev_err(dev, + devm_request_mem_region failed for MC portal %#llx\n, + mc_portal_phys_addr); + return -EBUSY; + } + + mc_portal_virt_addr = devm_ioremap_nocache(dev, + mc_portal_phys_addr, + mc_portal_size); While I can't complain about the device itself, I will note that I think it's a pretty bad design decision to expose actual host physical addresses in the protocol. I tend to agree. I'll look into creating a proposed change to the architecture to have the MC communicate a physical offset of some kind. Basically this means that you won't be able to pass a full MC complex into a guest, even if you could virtualize IRQ and DMA access unless you map it at the exact same location as the host's MC complex. Right. But is that really an issue in practice? Could we at least add a ranges property to the MC device description and check whether the physical addresses we get are within that range - if nothing else, at least as sanity check? Then maybe add some calls in the next version that act on that range rather than actual host physical addresses? So you mean something like: fsl_mc: fsl-mc@80c00 { compatible = fsl,qoriq-mc; #stream-id-cells = 2; reg = 0x0008 0x0c00 0 0x40,/* MC portal base */ 0x 0x0834 0 0x4; /* MC control reg */ ranges = 0x8 0x0 0x8 0x0 0x2000; lpi-parent = its; }; The physical addresses returned by the MC fall into a 512MB portal region at 0x8__ in the physical address map. For now map it 1:1, but in the future it could become: ranges = 0x0 0x0 0x8 0x0 0x2000; ...if I can get the hardware architecture changed. Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 09/13] irqchip: GICv3: ITS: MSI support
On Mon, Nov 24, 2014 at 8:35 AM, Marc Zyngier marc.zyng...@arm.com wrote: +/* + * We need a value to serve as a irq-type for LPIs. Choose one that will + * hopefully pique the interest of the reviewer. + */ +#define GIC_IRQ_TYPE_LPI 0xa110c8ed Ok, my interest is piqued. Why this value? Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/8] device core: Introduce per-device MSI domain pointer
On Thu, Jan 8, 2015 at 11:06 AM, Marc Zyngier marc.zyng...@arm.com wrote: As MSI-type features are creeping into non-PCI devices, it is starting to make sense to give our struct device some form of support for this, by allowing a pointer to an MSI irq domain to be set/retrieved. Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- include/linux/device.h | 20 1 file changed, 20 insertions(+) diff --git a/include/linux/device.h b/include/linux/device.h index fb50673..ec4cee5 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -690,6 +690,7 @@ struct acpi_dev_node { * along with subsystem-level and driver-level callbacks. * @pins: For device pin management. * See Documentation/pinctrl.txt for details. + * @msi_domain: The generic MSI domain this device is using. * @numa_node: NUMA node this device is close to. * @dma_mask: Dma mask (if dma'ble device). * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all @@ -750,6 +751,9 @@ struct device { struct dev_pm_info power; struct dev_pm_domain*pm_domain; +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN + struct irq_domain *msi_domain; /* MSI domain device uses */ +#endif This is not a comment on this patch specifically, but a question about other MSI specific fields that might be needed in struct device. Currently the generic MSI domain handling has hardcoded assumptions that devices are PCI-- see the for_each_msi_entry() iterator in msi.h: #define dev_to_msi_list(dev)(to_pci_dev((dev))-msi_list) #define for_each_msi_entry(desc, dev) \ list_for_each_entry((desc), dev_to_msi_list((dev)), list) One approach would be to move the msi_list out of pci_dev and put it in struct device, so all devices can have an msi_list. The other approach would be to keep msi_list in a bus specific device struct, and then dev_to_msi_list() would need to be implemented as a bus specific callback of some kind. The above hardcoded PCI assumption isn't going to work. Wanted to see if there is any advice in which direction to go. Thanks, Stuart Yoder -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/8] device core: Introduce per-device MSI domain pointer
Gerry, So which direction did you take in your patch set-- a) common, generic msi_desc, or b) bus-specific msi_desc like Marc showed (mybus_msi_desc)? Thanks, Stuart On Sun, Jan 18, 2015 at 8:10 PM, Jiang Liu jiang@linux.intel.com wrote: On 2015/1/16 4:35, Stuart Yoder wrote: On Thu, Jan 8, 2015 at 11:06 AM, Marc Zyngier marc.zyng...@arm.com wrote: As MSI-type features are creeping into non-PCI devices, it is starting to make sense to give our struct device some form of support for this, by allowing a pointer to an MSI irq domain to be set/retrieved. Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- include/linux/device.h | 20 1 file changed, 20 insertions(+) diff --git a/include/linux/device.h b/include/linux/device.h index fb50673..ec4cee5 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -690,6 +690,7 @@ struct acpi_dev_node { * along with subsystem-level and driver-level callbacks. * @pins: For device pin management. * See Documentation/pinctrl.txt for details. + * @msi_domain: The generic MSI domain this device is using. * @numa_node: NUMA node this device is close to. * @dma_mask: Dma mask (if dma'ble device). * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all @@ -750,6 +751,9 @@ struct device { struct dev_pm_info power; struct dev_pm_domain*pm_domain; +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN + struct irq_domain *msi_domain; /* MSI domain device uses */ +#endif This is not a comment on this patch specifically, but a question about other MSI specific fields that might be needed in struct device. Currently the generic MSI domain handling has hardcoded assumptions that devices are PCI-- see the for_each_msi_entry() iterator in msi.h: #define dev_to_msi_list(dev)(to_pci_dev((dev))-msi_list) #define for_each_msi_entry(desc, dev) \ list_for_each_entry((desc), dev_to_msi_list((dev)), list) One approach would be to move the msi_list out of pci_dev and put it in struct device, so all devices can have an msi_list. The other approach would be to keep msi_list in a bus specific device struct, and then dev_to_msi_list() would need to be implemented as a bus specific callback of some kind. The above hardcoded PCI assumption isn't going to work. Wanted to see if there is any advice in which direction to go. Hi Stuart, I already have some a patch set to go that direction waiting send out for review:) Thanks! Gerry Thanks, Stuart Yoder -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 0/3 v6] drivers/bus: Freescale Management Complex bus driver patch series
Hi Arnd/Alex, German has posted an example driver for the fsl-mc bus in his RFC [RFC PATCH 1/1] drivers/bus: fsl-mc object allocator driver. In addition I have made available the skeleton for a driver for one of the objects/devices (crypto) that will be discovered on the bus: https://github.com/stuyoder/linux branch: fsl-ms-bus ...it is not functional yet, but shows how a driver registers with the bus, get's probed, performs initialization. Thanks, Stuart -Original Message- From: J. German Rivera [mailto:german.riv...@freescale.com] Sent: Friday, January 16, 2015 7:01 PM To: gre...@linuxfoundation.org; a...@arndb.de; linux-kernel@vger.kernel.org Cc: Yoder Stuart-B08248; Phillips Kim-R1AAHA; Wood Scott-B07421; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Thorpe Geoff-R01361; Sharma Bhupesh-B45370; Erez Nir-RM30794; Schmitt Richard- B43082 Subject: [PATCH 0/3 v6] drivers/bus: Freescale Management Complex bus driver patch series This patch series introduces Linux support for the Freescale Management Complex (fsl-mc) hardware. This patch series is dependent on the patch series ARM64: Add support for FSL's LS2085A SoC (http://thread.gmane.org/gmane.linux.ports.arm.kernel/351829) The fsl-mc is a hardware resource manager that manages specialized hardware objects used in network-oriented packet processing applications. After the fsl-mc block is enabled, pools of hardware resources are available, such as queues, buffer pools, I/O interfaces. These resources are building blocks that can be used to create functional hardware objects such as network interfaces, crypto accelerator instances, or L2 switches. All the fsl-mc managed hardware resources/objects are represented in a physical grouping mechanism called a 'container' or DPRC (data path resource container). From the point of view of an OS, a DPRC functions similar to a plug and play bus. Using fsl-mc commands software can enumerate the contents of the DPRC discovering the hardware objects present and binding them to drivers. Hardware objects can be created and removed dynamically, providing hot pluggability of the hardware objects. Software contexts interact with the fsl-mc by sending commands through a memory mapped hardware interface called an MC portal. Every fsl-mc object type has a command set to manage the objects. Key DPRC commands include: -create/destroy a DPRC -enumerate objects and resource pools in the DPRC, including identifying mappable regions and the number of IRQs an object may have -IRQ configuration -move objects/resources between DPRCs -connecting objects (e.g. connecting a network interface to an L2 switch port) -reset Patch 1 contains a minimal set of low level functions to send an d receive commands to the fsl-mc. It includes support for basic management commands and commands to manipulate DPRC objects. Patch 2 contains a platform device driver that sets up and registers the basic bus infrastructure including support for adding/removing devices, register/unregister of drivers, and bus match support to bind devices to drivers. Patch 3 contains an driver that manages DPRC objects (the container that holds the hardware resources). This driver functions as a bus controller and handles enumeration of the objects in the container and hotplug events. CHANGE HISTORY Changes in v6: - Upgraded MC flibs to version 0.5 - Fixed warnings from latest checkpatch Changes in v5: - Addressed comments from Alex Graf. Details in each patch. Changes in v4: - Addressed changes from ALex Graf. Details in each patch. - Upgraded MC flibs to version 0.5 - Fixed bug related to setting fsl_mc_bus_type.dev_root too late Changes in v3: - Addressed pending issues not resolved in v2: * Removed Doxygen comments as requested by Greg Kroah-Hartman * Moved newly added FSL-specific header files from include/linux to include/linux/fsl * Changed wording of licenses in MC flib source files * Fixed sparse warning about context imbalance in 'mc_send_command' - Addressed additional comments from Kim Phillips, Scott Wood and Timur Tabi. Details in each patch. Issues pending resolution not addressed by v3: - Checkpatch warnings: * WARNING: DT compatible string fsl,qoriq-mc appears un-documented This warning will go away once the prerequisite patch series is accepted upstream. * WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? This warning still happens even after adding MAINTAINERS file entries in the same patch where new files were added Changes in v2: - Added reference to the patch series this patch is dependent on for device tree and device tree bindings changes. (See above). - Removed patch 4 (update to the MAINTAINERS file), as this is done now in patch 1 - Addressed comments from Joe Perches, Kim Phillips, Alex Graf
RE: [PATCH 0/3 v6] drivers/bus: Freescale Management Complex bus driver patch series
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, February 26, 2015 8:33 AM To: Yoder Stuart-B08248; a...@arndb.de Cc: Rivera Jose-B46482; linux-kernel@vger.kernel.org; gre...@linuxfoundation.org Subject: Re: [PATCH 0/3 v6] drivers/bus: Freescale Management Complex bus driver patch series On 27.01.15 15:35, Stuart Yoder wrote: Hi Arnd/Alex, German has posted an example driver for the fsl-mc bus in his RFC [RFC PATCH 1/1] drivers/bus: fsl-mc object allocator driver. In addition I have made available the skeleton for a driver for one of the objects/devices (crypto) that will be discovered on the bus: https://github.com/stuyoder/linux branch: fsl-ms-bus ...it is not functional yet, but shows how a driver registers with the bus, get's probed, performs initialization. Ok, so if I grasp this correctly the idea is that we have a driver attaching to an individual device on the fsl-mc bus. Yes. That driver then goes and allocates / blocks more devices from that bus as it initializes. Yes, there are certain devices/objects on the bus that by themselves are not standalone, functional devices. An example is a buffer pool. Network interface drivers, crypto driver, decompression driver, etc need one or more hardware buffer pools. There is a buffer depletion interrupt associated with the device. The buffer pools itself binds to a resource allocation driver in the kernel, which then can hand out buffer pools as required by other drivers. Is that model always possible? Yes, why would it not be? Which device would a NIC bind to for example? Network interface / Ethernet driver requires some number of buffer pools, plus a management complex portal device (DPMCP) used for sending commands to manage the hardware. I merely want to make sure we're not running ourselves into a bad corner ;). If we are, I would like to understand it. :) Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 0/3 v6] drivers/bus: Freescale Management Complex bus driver patch series
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, February 26, 2015 3:38 PM To: Yoder Stuart-B08248; a...@arndb.de Cc: Rivera Jose-B46482; linux-kernel@vger.kernel.org; gre...@linuxfoundation.org Subject: Re: [PATCH 0/3 v6] drivers/bus: Freescale Management Complex bus driver patch series On 26.02.15 21:32, Stuart Yoder wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, February 26, 2015 8:33 AM To: Yoder Stuart-B08248; a...@arndb.de Cc: Rivera Jose-B46482; linux-kernel@vger.kernel.org; gre...@linuxfoundation.org Subject: Re: [PATCH 0/3 v6] drivers/bus: Freescale Management Complex bus driver patch series On 27.01.15 15:35, Stuart Yoder wrote: Hi Arnd/Alex, German has posted an example driver for the fsl-mc bus in his RFC [RFC PATCH 1/1] drivers/bus: fsl-mc object allocator driver. In addition I have made available the skeleton for a driver for one of the objects/devices (crypto) that will be discovered on the bus: https://github.com/stuyoder/linux branch: fsl-ms-bus ...it is not functional yet, but shows how a driver registers with the bus, get's probed, performs initialization. Ok, so if I grasp this correctly the idea is that we have a driver attaching to an individual device on the fsl-mc bus. Yes. That driver then goes and allocates / blocks more devices from that bus as it initializes. Yes, there are certain devices/objects on the bus that by themselves are not standalone, functional devices. An example is a buffer pool. Network interface drivers, crypto driver, decompression driver, etc need one or more hardware buffer pools. There is a buffer depletion interrupt associated with the device. The buffer pools itself binds to a resource allocation driver in the kernel, which then can hand out buffer pools as required by other drivers. Ok, so there are 2 things on the bus * devices * resources In the general sense, yes. To be picky about terminology we call all these things on the bus objects. Some are more resource-like, in that they are handed out by an allocator to the functional drivers. I don't want to call them 'resources' because that term actually means something slightly different in the hardware architecture that is not actually visible to Linux. Someone really needs to sit down and write some nice ASCII art about all of this and include all the abbreviations in it as well, so that anyone not deeply involved in the architecture has the chance to grasp what this is about. The cover letter for the patch series is a starting point, but yes we need something for ./Documentation. Is that model always possible? Yes, why would it not be? Which device would a NIC bind to for example? Network interface / Ethernet driver requires some number of buffer pools, plus a management complex portal device (DPMCP) used for sending commands to manage the hardware. Ok, so there is always one object that basically owns a particular device. And then there is a cloud of resources that drivers grab as they go. I think I got it by now and the concept makes a lot of sense. I'm not sure whether there's any particular benefit or downside of having resources be devices, but looking at the resource manager code it probably doesn't hurt. They need to be real Linux devices. The reason is that when we bind a DPRC and the objects in it to VFIO, VFIO expects everything to be a device. VFIO exposes 'devices' to user space, and so for example a buffer pool's IRQ needs to be exposed via standard VFIO mechanisms. Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RESEND][ PATCH v3] irqchip/gicv3-its: ITS table size should not be smaller than PSZ
when allocating a device table, if the requested allocation is smaller than the default granule size of the ITS then, we need to round up to the default size Signed-off-by: Minghuan Lian minghuan.l...@freescale.com Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- -v3 changes -updated commit message and added comment -v3 resend-- updated email addresses sent to -would be nice to get this into 4.1 as the kernel will hang booting on some systems drivers/irqchip/irq-gic-v3-its.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 9687f8a..1b7e155 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -828,7 +828,14 @@ static int its_alloc_tables(struct its_node *its) u64 typer = readq_relaxed(its-base + GITS_TYPER); u32 ids = GITS_TYPER_DEVBITS(typer); - order = get_order((1UL ids) * entry_size); + /* +* 'order' was initialized earlier to the default page +* granule of the the ITS. We can't have an allocation +* smaller than that. If the requested allocation +* is smaller, round up to the default page granule. +*/ + order = max(get_order((1UL ids) * entry_size), + order); if (order = MAX_ORDER) { order = MAX_ORDER - 1; pr_warn(%s: Device Table too large, reduce its page order to %u\n, -- 2.3.3 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/3] Docs: dt: Add PCI MSI map bindings
From: Mark Rutland mark.rutl...@arm.com Date: Thu, Jul 23, 2015 at 11:52 AM [cut] diff --git a/Documentation/devicetree/bindings/pci/pci-msi.txt b/Documentation/devicetree/bindings/pci/pci-msi.txt new file mode 100644 index 000..9b3cc81 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/pci-msi.txt @@ -0,0 +1,220 @@ +This document describes the generic device tree binding for describing the +relationship between PCI devices and MSI controllers. + +Each PCI device under a root complex is uniquely identified by its Requester ID +(AKA RID). A Requester ID is a triplet of a Bus number, Device number, and +Function number. + +For the purpose of this document, when treated as a numeric value, a RID is +formatted such that: + +* Bits [15:8] are the Bus number. +* Bits [7:3] are the Device number. +* Bits [2:0] are the Function number. +* Any other bits required for padding must be zero. + +MSIs may be distinguished in part through the use of sideband data accompanying +writes. In the case of PCI devices, this sideband data may be derived from the +Requester ID. A mechanism is required to associate a device with both the MSI +controllers it can address, and the sideband data that will be associated with +its writes to those controllers. + +For generic MSI bindings, see +Documentation/devicetree/bindings/interrupt-controller/msi.txt. + + +PCI root complex + + +Optional properties +--- + +- msi-map: Maps a Requester ID to an MSI controller and associated + msi-specifier data. The property is an arbitrary number of tuples of + (rid-base,msi-controller,msi-base,length), where: + + * rid-base is a single cell describing the first RID matched by the entry. + + * msi-controller is a single phandle to an MSI controller + + * msi-base is an msi-specifier describing the msi-specifier produced for the +first RID matched by the entry. + + * length is a single cell describing how many consecutive RIDs are matched +following the rid-base. + + Any RID r in the interval [rid-base, rid-base + length) is associated with + the listed msi-controller, with the msi-specifier (r - rid-base + msi-base). + +- msi-map-mask: A mask to be applied to each Requester ID prior to being mapped + to an msi-specifier per the msi-map property. Can we extend the msi-map-mask definition to say: A mask value of 0x0 is valid and indicates that no RIDs are _currently_ mapped to any msi-specifier. We have an SoC with a programmable hardware table in the PCI controller that maps requester ID to stream ID, so the overall msi-map (and iommu-map) definition fit into that scheme. But, we would like to be able make the RID-stream-ID mapping decision _lazily_, in Linux, based on actual usage of PCI devices. pcie@360 { compatible = fsl,ls2085a-pcie, snps,dw-pcie; device_type = pci; ... msi-map = 0x0 msi_a 0x7 4, msi-map-mask = 0x0 }; That specifies the there are 4 stream IDs starting at stream ID 0x7, but the requester ID's are not mapped (because the mask is 0x0). This tells the PCI controller driver that there are 4 msi-specifiers (e.g. stream IDs) available and what they are. (same definition would apply to the iommu-map-mask) Thanks, Stuart
RE: [PATCH 2/3] Docs: dt: Add PCI MSI map bindings
-Original Message- From: Mark Rutland [mailto:mark.rutl...@arm.com] Sent: Thursday, August 06, 2015 1:15 PM To: Yoder Stuart-B08248; Marc Zyngier; Will Deacon Cc: devicet...@vger.kernel.org; Lorenzo Pieralisi; a...@arndb.de; linux-kernel@vger.kernel.org; dda...@caviumnetworks.com; io...@lists.linux-foundation.org; tirumalesh.chalama...@caviumnetworks.com; laurent.pinch...@ideasonboard.com; thunder.leiz...@huawei.com; tred...@nvidia.com; linux-arm- ker...@lists.infradead.org; majun...@huawei.com Subject: Re: [PATCH 2/3] Docs: dt: Add PCI MSI map bindings [...] +PCI root complex + + +Optional properties +--- + +- msi-map: Maps a Requester ID to an MSI controller and associated + msi-specifier data. The property is an arbitrary number of tuples of + (rid-base,msi-controller,msi-base,length), where: + + * rid-base is a single cell describing the first RID matched by the entry. + + * msi-controller is a single phandle to an MSI controller + + * msi-base is an msi-specifier describing the msi-specifier produced for the +first RID matched by the entry. + + * length is a single cell describing how many consecutive RIDs are matched +following the rid-base. + + Any RID r in the interval [rid-base, rid-base + length) is associated with + the listed msi-controller, with the msi-specifier (r - rid-base + msi-base). + +- msi-map-mask: A mask to be applied to each Requester ID prior to being mapped + to an msi-specifier per the msi-map property. Can we extend the msi-map-mask definition to say: A mask value of 0x0 is valid and indicates that no RIDs are _currently_ mapped to any msi-specifier. That would break a valid case of the mask being all zeroes. Consider the case that all RIDs get mapped to a single msi-specifier; the obvious way to write that is: msi-map-mask = 0x; msi-map = 0x msi (msi-specifier) 1; In this case all RIDS are always mapped to the single msi-specifier. Does it really break that case? We could have this (your example): msi-map-mask = 0x; msi-map = 0x msi 7 1; // map all RIDs to msi-spec 7 Or, my example: msi-map-mask = 0x; msi-map = 0x msi 7 4; // all RIDs map to any of msi-spec 7,8,9,10 We have an SoC with a programmable hardware table in the PCI controller that maps requester ID to stream ID, so the overall msi-map (and iommu-map) definition fit into that scheme. But, we would like to be able make the RID-stream-ID mapping decision _lazily_, in Linux, based on actual usage of PCI devices. Dynamically programming the mapping is at odds to this binding. I don't see how that can fit. My example above obviously doesn't make sense for a static binding, but can't we allow both a mask value of 0x0 and a length 1 at the same time. Why can the RID-SID mapping not be statically configured prior to entering the OS? The problem for us is the limited number of SIDs available on our SoC. We have an SMMU-500 with 128 SIDs / 64-contexts total. An SR-IOV card could enable VFs dynamically and suddenly there are 64 new RIDs on a PCI bus. There are 4 PCI controllers and firmware doesn't know what might be enabled on which bus. We run out of available SIDs. In that example we want all RIDs mapped to one SID by default, but want the option of setting our dynamic RID-SID table for a situation where say someone assigns a VF to a KVM VM and thus needs an SID to use for SMMU mappings. I think the binding can work for the dynamic case as long as we allow the example I showed above. So, I would propose changing my proposed text to: A mask value of 0x0 is valid and indicates that all RIDS map to the specified msi-specifier(s). If the mask is 0x0 and length 1 it indicates that all RIDs map to any of the msi-specifiers with the actual mapping left unspecified by the msi-map property. Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] staging: fsl-mc: add DPAA2 overview readme
Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- drivers/staging/fsl-mc/README.txt | 364 ++ drivers/staging/fsl-mc/TODO | 4 - 2 files changed, 364 insertions(+), 4 deletions(-) create mode 100644 drivers/staging/fsl-mc/README.txt diff --git a/drivers/staging/fsl-mc/README.txt b/drivers/staging/fsl-mc/README.txt new file mode 100644 index 000..8214102 --- /dev/null +++ b/drivers/staging/fsl-mc/README.txt @@ -0,0 +1,364 @@ +Copyright (C) 2015 Freescale Semiconductor Inc. + +DPAA2 (Data Path Acceleration Architecture Gen2) + + +This document provides an overview of the Freescale DPAA2 architecture +and how it is integrated into the Linux kernel. + +Contents summary + -DPAA2 overview + -Overview of DPAA2 objects + -DPAA2 Linux driver architecture overview +-bus driver +-dprc driver +-allocator +-dpio driver +-Ethernet +-mac + +DPAA2 Overview +-- + +DPAA2 is a hardware architecture designed for high-speeed network +packet processing. DPAA2 consists of sophisticated mechanisms for +processing Ethernet packets, queue management, buffer management, +autonomous L2 switching, virtual Ethernet bridging, and accelerator +(e.g. crypto) sharing. + +A DPAA2 hardware component called the Management Complex (or MC) manages the +DPAA2 hardware resources. The MC provides an object-based abstraction for +software drivers to use the DPAA2 hardware. + +The MC uses DPAA2 hardware resources such as queues, buffer pools, and +network ports to create functional objects/devices such as network +interfaces, an L2 switch, or accelerator instances. + +The MC provides memory-mapped I/O command interfaces (MC portals) +which DPAA2 software drivers use to operate on DPAA2 objects: + + +--+ + | OS | + |DPAA2 drivers | + | || + +-|+ + | + | (create,discover,connect + | config,use,destroy) + | + DPAA2 | + +| mc portal |-+ + | || + | +- - - - - - - - - - - - -V- - -+ | + | | | | + | | Management Complex (MC) | | + | | | | + | +- - - - - - - - - - - - - - - -+ | + | | + | Hardware Hardware | + | Resources Objects| + | - ---| + | -queues -DPRC | + | -buffer pools -DPMCP | + | -Eth MACs/ports -DPIO | + | -network interface-DPNI | + | profiles -DPMAC | + | -queue portals-DPBP | + | -MC portals... | + | ... | + | | + +--+ + +The MC mediates operations such as create, discover, +connect, configuration, and destroy. Fast-path operations +on data, such as packet transmit/receive, are not mediated by +the MC and are done directly using memory mapped regions in +DPIO objects. + +Overview of DPAA2 Objects +- +The section provides a brief overview of some key objects +in the DPAA2 hardware. A simple scenario is described illustrating +the objects involved in creating a network interfaces. + +-DPRC (Datapath Resource Container) + +A DPRC is an container object that holds all the other +types of DPAA2 objects. In the example diagram below there +are 8 objects of 5 types (DPMCP, DPIO, DPBP, DPNI, and DPMAC) +in the container. + ++-+ +| DPRC| +| | +| +---+ +---+ +---+ +---+ +---+ | +| | DPMCP | | DPIO | | DPBP | | DPNI | | DPMAC | | +| +---+ +---+ +---+ +---+---+ +---+---+ | +| | DPMCP | | DPIO | | +| +---+ +---+ | +| | DPMCP | | +| +---+ | +| | ++-+ + +From the point
[PATCH v2] staging: fsl-mc: add DPAA2 overview readme
add README file providing an overview of the DPAA2 architecture and how it is integrated in Linux Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- -v2: added changelog text drivers/staging/fsl-mc/README.txt | 364 ++ drivers/staging/fsl-mc/TODO | 4 - 2 files changed, 364 insertions(+), 4 deletions(-) create mode 100644 drivers/staging/fsl-mc/README.txt diff --git a/drivers/staging/fsl-mc/README.txt b/drivers/staging/fsl-mc/README.txt new file mode 100644 index 000..8214102 --- /dev/null +++ b/drivers/staging/fsl-mc/README.txt @@ -0,0 +1,364 @@ +Copyright (C) 2015 Freescale Semiconductor Inc. + +DPAA2 (Data Path Acceleration Architecture Gen2) + + +This document provides an overview of the Freescale DPAA2 architecture +and how it is integrated into the Linux kernel. + +Contents summary + -DPAA2 overview + -Overview of DPAA2 objects + -DPAA2 Linux driver architecture overview +-bus driver +-dprc driver +-allocator +-dpio driver +-Ethernet +-mac + +DPAA2 Overview +-- + +DPAA2 is a hardware architecture designed for high-speeed network +packet processing. DPAA2 consists of sophisticated mechanisms for +processing Ethernet packets, queue management, buffer management, +autonomous L2 switching, virtual Ethernet bridging, and accelerator +(e.g. crypto) sharing. + +A DPAA2 hardware component called the Management Complex (or MC) manages the +DPAA2 hardware resources. The MC provides an object-based abstraction for +software drivers to use the DPAA2 hardware. + +The MC uses DPAA2 hardware resources such as queues, buffer pools, and +network ports to create functional objects/devices such as network +interfaces, an L2 switch, or accelerator instances. + +The MC provides memory-mapped I/O command interfaces (MC portals) +which DPAA2 software drivers use to operate on DPAA2 objects: + + +--+ + | OS | + |DPAA2 drivers | + | || + +-|+ + | + | (create,discover,connect + | config,use,destroy) + | + DPAA2 | + +| mc portal |-+ + | || + | +- - - - - - - - - - - - -V- - -+ | + | | | | + | | Management Complex (MC) | | + | | | | + | +- - - - - - - - - - - - - - - -+ | + | | + | Hardware Hardware | + | Resources Objects| + | - ---| + | -queues -DPRC | + | -buffer pools -DPMCP | + | -Eth MACs/ports -DPIO | + | -network interface-DPNI | + | profiles -DPMAC | + | -queue portals-DPBP | + | -MC portals... | + | ... | + | | + +--+ + +The MC mediates operations such as create, discover, +connect, configuration, and destroy. Fast-path operations +on data, such as packet transmit/receive, are not mediated by +the MC and are done directly using memory mapped regions in +DPIO objects. + +Overview of DPAA2 Objects +- +The section provides a brief overview of some key objects +in the DPAA2 hardware. A simple scenario is described illustrating +the objects involved in creating a network interfaces. + +-DPRC (Datapath Resource Container) + +A DPRC is an container object that holds all the other +types of DPAA2 objects. In the example diagram below there +are 8 objects of 5 types (DPMCP, DPIO, DPBP, DPNI, and DPMAC) +in the container. + ++-+ +| DPRC| +| | +| +---+ +---+ +---+ +---+ +---+ | +| | DPMCP | | DPIO | | DPBP | | DPNI | | DPMAC | | +| +---+ +---+ +---+ +---+---+ +---+---+ | +| | DPMCP | | DPIO | | +| +---+ +---+ | +| | DPMCP
[PATCH] net/fsl: simplify Kconfig dependency list for fsl networking
make the list of Kconfig dependencies for Freescale networking more general. Simplify to supported architectures: ARM, ARM64, PPC, M68K Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- drivers/net/ethernet/freescale/Kconfig | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig index ff76d4e..70782d7 100644 --- a/drivers/net/ethernet/freescale/Kconfig +++ b/drivers/net/ethernet/freescale/Kconfig @@ -5,9 +5,7 @@ config NET_VENDOR_FREESCALE bool Freescale devices default y - depends on FSL_SOC || QUICC_ENGINE || CPM1 || CPM2 || PPC_MPC512x || \ - M523x || M527x || M5272 || M528x || M520x || M532x || \ - ARCH_MXC || ARCH_MXS || (PPC_MPC52xx PPC_BESTCOMM) + depends on M68K || PPC || ARM || ARM64 ---help--- If you have a network (Ethernet) card belonging to this class, say Y. -- 2.3.3 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/2] Documentation: DT: FTM: add FTM0 be used as alarm timer
On Tue, Aug 11, 2015 at 11:01 PM, Dongsheng Wang dongsheng.w...@freescale.com wrote: From: Wang Dongsheng dongsheng.w...@freescale.com In freescale layerscape platform there is only FTM0 can be used as alarm timer to wake up system. So add FTM0 description for devicetree document. Suggestion: In the Freescale Layerscape platform a flextimer module can be used as an alarm timer to wake up the system. Define a binding for this alarm. Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com --- V3: Include this patch in V3. diff --git a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt index aa8c402..380a0b3d 100644 --- a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt +++ b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt @@ -1,5 +1,7 @@ Freescale FlexTimer Module (FTM) Timer +* Default FTM Timer + Required properties: - compatible : should be fsl,ftm-timer @@ -29,3 +31,26 @@ ftm: ftm@400b8000 { clks VF610_CLK_FTM3_EXT_FIX_EN; big-endian; }; + +* FTM Alarm Timer + The default FTM device contains eight FlexTimer modules. FlexTimer1 is in + on-domain(power is not switched off in deep sleep mode). Other seven FlexTimer + modules(flextimer2/3/4/5/6/7/8) are in off-domain (power is switched off in + deep sleep mode). Suggestion: This binding describes a Flextimer (FTM) instance that can be used as an alarm to wake up a system in deep sleep. The flextimer module with alarm support is in a power domain that remains awake when the SoC is in deep sleep mode. (don't use flextimer1 in the binding...for all you know in the next SoC it will be flextimer7 that has this capability) +Required properties: + +- compatible : should be fsl,ftm-alarm. +- reg : should contain base address and length of FTM timer 0 register. should contain the address and size of the FTM alarm module registers (don't use the number 0) +- interrupts : Should contain FTM 0 interrupt. describes the FTM alarm interrupt +- big-endian: One boolean property, the big endian mode will be in use if it is + present, or the little endian mode will be in use for all the device registers. + +Example: +ftm0: ftm0@29d { + compatible = fsl,ftm-alarm; + reg = 0x0 0x29d 0x0 0x1; + interrupts = GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH; + big-endian; + status = disabled; +}; Drop the number 0 from the names in your example. Thanks, Stuart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2] staging: fsl-mc: add DPAA2 overview readme
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Sunday, August 09, 2015 9:25 AM To: Yoder Stuart-B08248; gre...@linuxfoundation.org; Rivera Jose-B46482; katz Itai-RM05202 Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; a...@arndb.de Subject: Re: [PATCH v2] staging: fsl-mc: add DPAA2 overview readme On 07.08.15 03:09, Stuart Yoder wrote: add README file providing an overview of the DPAA2 architecture and how it is integrated in Linux Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- -v2: added changelog text drivers/staging/fsl-mc/README.txt | 364 ++ drivers/staging/fsl-mc/TODO | 4 - 2 files changed, 364 insertions(+), 4 deletions(-) create mode 100644 drivers/staging/fsl-mc/README.txt diff --git a/drivers/staging/fsl-mc/README.txt b/drivers/staging/fsl-mc/README.txt new file mode 100644 index 000..8214102 --- /dev/null +++ b/drivers/staging/fsl-mc/README.txt @@ -0,0 +1,364 @@ +Copyright (C) 2015 Freescale Semiconductor Inc. + +DPAA2 (Data Path Acceleration Architecture Gen2) + + +This document provides an overview of the Freescale DPAA2 architecture +and how it is integrated into the Linux kernel. + +Contents summary + -DPAA2 overview + -Overview of DPAA2 objects + -DPAA2 Linux driver architecture overview +-bus driver +-dprc driver +-allocator +-dpio driver +-Ethernet +-mac + +DPAA2 Overview +-- + +DPAA2 is a hardware architecture designed for high-speeed network +packet processing. DPAA2 consists of sophisticated mechanisms for +processing Ethernet packets, queue management, buffer management, +autonomous L2 switching, virtual Ethernet bridging, and accelerator +(e.g. crypto) sharing. + +A DPAA2 hardware component called the Management Complex (or MC) manages the +DPAA2 hardware resources. The MC provides an object-based abstraction for +software drivers to use the DPAA2 hardware. + +The MC uses DPAA2 hardware resources such as queues, buffer pools, and +network ports to create functional objects/devices such as network +interfaces, an L2 switch, or accelerator instances. + +The MC provides memory-mapped I/O command interfaces (MC portals) +which DPAA2 software drivers use to operate on DPAA2 objects: + + +--+ + | OS | + |DPAA2 drivers | + | || + +-|+ + | + | (create,discover,connect + | config,use,destroy) + | + DPAA2 | + +| mc portal |-+ + | || + | +- - - - - - - - - - - - -V- - -+ | + | | | | + | | Management Complex (MC) | | + | | | | + | +- - - - - - - - - - - - - - - -+ | + | | + | Hardware Hardware | + | Resources Objects| + | - ---| + | -queues -DPRC | + | -buffer pools -DPMCP | + | -Eth MACs/ports -DPIO | + | -network interface-DPNI | + | profiles -DPMAC | + | -queue portals-DPBP | + | -MC portals... | + | ... | + | | + +--+ + +The MC mediates operations such as create, discover, +connect, configuration, and destroy. Fast-path operations +on data, such as packet transmit/receive, are not mediated by +the MC and are done directly using memory mapped regions in +DPIO objects. + +Overview of DPAA2 Objects +- +The section provides a brief overview of some key objects +in the DPAA2 hardware. A simple scenario is described illustrating +the objects involved in creating a network interfaces. + +-DPRC (Datapath Resource Container) + +A DPRC is an container object that holds all the other +types of DPAA2 objects. In the example diagram below there +are 8 objects of 5 types (DPMCP, DPIO, DPBP, DPNI, and DPMAC) +in the container
RE: [PATCH v2] staging: fsl-mc: add DPAA2 overview readme
-Original Message- From: Tillmann Heidsieck [mailto:theidsi...@leenox.de] Sent: Tuesday, August 11, 2015 6:01 AM To: Yoder Stuart-B08248; gre...@linuxfoundation.org; Rivera Jose-B46482; katz Itai-RM05202 Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; ag...@suse.de; a...@arndb.de Subject: Re: [PATCH v2] staging: fsl-mc: add DPAA2 overview readme Hi Stuart, I am by no means a native speaker, but I have proof-read my fair share of articles and theses, so here are a bunch of suggestions which might or might not help improve you document. On 07.08.2015 03:09, Stuart Yoder wrote: add README file providing an overview of the DPAA2 architecture and how it is integrated in Linux Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- -v2: added changelog text drivers/staging/fsl-mc/README.txt | 364 ++ drivers/staging/fsl-mc/TODO | 4 - 2 files changed, 364 insertions(+), 4 deletions(-) create mode 100644 drivers/staging/fsl-mc/README.txt diff --git a/drivers/staging/fsl-mc/README.txt b/drivers/staging/fsl-mc/README.txt new file mode 100644 index 000..8214102 --- /dev/null +++ b/drivers/staging/fsl-mc/README.txt @@ -0,0 +1,364 @@ +Copyright (C) 2015 Freescale Semiconductor Inc. + +DPAA2 (Data Path Acceleration Architecture Gen2) + + +This document provides an overview of the Freescale DPAA2 architecture +and how it is integrated into the Linux kernel. + +Contents summary + -DPAA2 overview + -Overview of DPAA2 objects + -DPAA2 Linux driver architecture overview +-bus driver +-dprc driver - DPRC driver +-allocator +-dpio driver - DPIO driver +-Ethernet +-mac mac - MAC + +DPAA2 Overview +-- + +DPAA2 is a hardware architecture designed for high-speeed network +packet processing. DPAA2 consists of sophisticated mechanisms for +processing Ethernet packets, queue management, buffer management, +autonomous L2 switching, virtual Ethernet bridging, and accelerator +(e.g. crypto) sharing. + +A DPAA2 hardware component called the Management Complex (or MC) manages the +DPAA2 hardware resources. The MC provides an object-based abstraction for +software drivers to use the DPAA2 hardware. + +The MC uses DPAA2 hardware resources such as queues, buffer pools, and +network ports to create functional objects/devices such as network +interfaces, an L2 switch, or accelerator instances. + +The MC provides memory-mapped I/O command interfaces (MC portals) +which DPAA2 software drivers use to operate on DPAA2 objects: + + +--+ + | OS | + |DPAA2 drivers | + | || + +-|+ + | + | (create,discover,connect + | config,use,destroy) + | + DPAA2 | + +| mc portal |-+ + | || + | +- - - - - - - - - - - - -V- - -+ | + | | | | + | | Management Complex (MC) | | + | | | | + | +- - - - - - - - - - - - - - - -+ | + | | + | Hardware Hardware | + | Resources Objects| + | - ---| + | -queues -DPRC | + | -buffer pools -DPMCP | + | -Eth MACs/ports -DPIO | + | -network interface-DPNI | + | profiles -DPMAC | + | -queue portals-DPBP | + | -MC portals... | + | ... | + | | + +--+ + +The MC mediates operations such as create, discover, +connect, configuration, and destroy. Fast-path operations +on data, such as packet transmit/receive, are not mediated by +the MC and are done directly using memory mapped regions in +DPIO objects. + +Overview of DPAA2 Objects +- +The section provides a brief overview of some key objects +in the DPAA2 hardware. A simple scenario is described illustrating +the objects involved in creating a network interfaces. + +-DPRC (Datapath Resource Container
[PATCH] staging: fsl-mc: update TODO list
update TODO list to provide more detail on remaining work Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- drivers/staging/fsl-mc/TODO | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/staging/fsl-mc/TODO b/drivers/staging/fsl-mc/TODO index d78288b..c29516b 100644 --- a/drivers/staging/fsl-mc/TODO +++ b/drivers/staging/fsl-mc/TODO @@ -6,8 +6,30 @@ and if so add support for this. * Add at least one device driver for a DPAA2 object (child device of the - fsl-mc bus). + fsl-mc bus). Most likely candidate for this is adding DPAA2 Ethernet + driver support, which depends on drivers for several objects: DPNI, + DPIO, DPMAC. Other pre-requisites include: + + * interrupt support. for meaningful driver support we need + interrupts, and thus need message interrupt support by the bus + driver. + -Note: this has dependencies on generic MSI support work + in process upstream, see [1] and [2]. + + * Management Complex (MC) command serialization. locking mechanisms + are needed by drivers to serialize commands sent to the MC, including + from atomic context. + + * MC firmware uprev. The MC firmware upon which the fsl-mc + bus driver and DPAA2 object drivers are based is continuing + to evolve, so minor updates are needed to keep in sync with binary + interface changes to the MC. + +* Cleanup Please send any patches to Greg Kroah-Hartman gre...@linuxfoundation.org, german.riv...@freescale.com, de...@driverdev.osuosl.org, linux-kernel@vger.kernel.org + +[1] https://lkml.org/lkml/2015/7/9/93 +[2] https://lkml.org/lkml/2015/7/7/712 -- 2.3.3 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/5] staging: fsl-mc: Management Complex restool driver
> -Original Message- > From: Lijun Pan [mailto:lijun@freescale.com] > Sent: Sunday, October 25, 2015 5:41 PM > To: gre...@linuxfoundation.org; a...@arndb.de; de...@driverdev.osuosl.org; > linux-kernel@vger.kernel.org > Cc: Yoder Stuart-B08248; katz Itai-RM05202; Rivera Jose-B46482; Li > Yang-Leo-R58472; Wood Scott-B07421; > ag...@suse.de; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Sharma > Bhupesh-B45370; Erez Nir-RM30794; > Schmitt Richard-B43082; dan.carpen...@oracle.com; Pan Lijun-B44306 > Subject: [PATCH 5/5] staging: fsl-mc: Management Complex restool driver > > The kernel support for the restool (a user space resource management > tool) is a driver for the /dev/dprc.N device file. > Its purpose is to provide an ioctl interface, > which the restool uses to interact with the MC bus driver Name of the user space tool should be updated to be current: s/restool/ls-restool/ > and with the MC firmware. > We allocate a dpmcp at driver initialization, > and keep that dpmcp until driver exit. > We use that dpmcp by default. > If that dpmcp is in use, we create another portal at run time > and destroy the newly created portal after use. > The ioctl RESTOOL_SEND_MC_COMMAND sends user space command to fsl-mc > bus and utilizes the fsl-mc bus to communicate with MC firmware. > The ioctl RESTOOL_DPRC_SYNC request the mc-bus launch > objects scan under root dprc. > In order to support multiple root dprc, we utilize the bus notify > mechanism to scan fsl_mc_bus_type for the newly added root dprc. > After discovering the root dprc, it creates a miscdevice > /dev/dprc.N to associate with this root dprc. > > Signed-off-by: Lijun Pan> --- > drivers/staging/fsl-mc/bus/Kconfig | 7 +- > drivers/staging/fsl-mc/bus/Makefile | 3 + > drivers/staging/fsl-mc/bus/mc-ioctl.h | 24 ++ > drivers/staging/fsl-mc/bus/mc-restool.c | 488 > > 4 files changed, 521 insertions(+), 1 deletion(-) > create mode 100644 drivers/staging/fsl-mc/bus/mc-ioctl.h > create mode 100644 drivers/staging/fsl-mc/bus/mc-restool.c > > diff --git a/drivers/staging/fsl-mc/bus/Kconfig > b/drivers/staging/fsl-mc/bus/Kconfig > index 0d779d9..39c6ef9 100644 > --- a/drivers/staging/fsl-mc/bus/Kconfig > +++ b/drivers/staging/fsl-mc/bus/Kconfig > @@ -21,4 +21,9 @@ config FSL_MC_BUS > Only enable this option when building the kernel for > Freescale QorQIQ LS2 SoCs. > > - > +config FSL_MC_RESTOOL > +tristate "Freescale Management Complex (MC) restool driver" s/restool/ls-restool/ Thanks, Stuart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] arm64: dts: Added syscon-reboot node for FSL's LS2085A SoC
> -Original Message- > From: J. German Rivera [mailto:german.riv...@freescale.com] > Sent: Friday, October 23, 2015 8:31 PM > To: robh...@kernel.org; mark.rutl...@arm.com; devicet...@vger.kernel.org; > linux-arm- > ker...@lists.infradead.org; linux-kernel@vger.kernel.org > Cc: Sharma Bhupesh-B45370; Yoder Stuart-B08248; Li Yang-Leo-R58472; Rivera > Jose-B46482 > Subject: [PATCH] arm64: dts: Added syscon-reboot node for FSL's LS2085A SoC > > Added sys-reboot node to the FSL's LS2085A SoC DT to leverage > the ARM-generic reboot mechanism for this SoC. This mechanism > is enabled through CONFIG_POWER_RESET_SYSCON. > > Signed-off-by: J. German Rivera> --- > arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi | 12 > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi > b/arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi > index e281ceb..6f82163 100644 > --- a/arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi > +++ b/arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi > @@ -131,6 +131,18 @@ > interrupts = <1 9 0x4>; > }; > > + rst_ccsr: rstccsr@1E6 { > + compatible = "syscon"; > + reg = <0x0 0x1E6 0x0 0x1>; > + }; > + > + reboot@65024000 { > + compatible ="syscon-reboot"; > + regmap = <_ccsr>; > + offset = <0x0>; > + mask = <0x2>; > + }; > + Drop the unit address "65024000", since you have no "reg" property. The ePAPR explicitly says: "If the node has no reg property, the @ and unit-address must be omitted and the node-name alone differentiates the node from other nodes at the same level in the tree. Also, other examples in the kernel just use plain "reboot" as the name: arch/arm/boot/dts/hisi-x5hd2.dtsi arch/arm/boot/dts/vfxxx.dtsi arch/mips/boot/dts/brcm/bcm6328.dtsi Also, Bhupesh's LS2080A DTS updates were just accepte4d, and the device tree name is no longer fsl-ls2085a.dtsi. You need to rebase on top of those changes. Thanks, Stuart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] arm64: dts: Added syscon-reboot node for FSL's LS2085A SoC
I think that comment "Currently supported enable-method is psci v0.2" is a statement of intent, not what is available currently. And the only plan I am aware of is PSCI with UEFI based firmware. U-boot is a key firmware platform for us and has no PSCI implementation available. So, we need this. The device tree simply describes the hardware that is there and that's what this patch exposes. If down the road all firmware provides a PSCI based reset interface then we will naturally use that. But we need something in the meantime to let us reboot the system. Thanks, Stuart > -Original Message- > From: Mark Rutland [mailto:mark.rutl...@arm.com] > Sent: Tuesday, October 27, 2015 11:35 AM > To: Rivera Jose-B46482 > Cc: robh...@kernel.org; devicet...@vger.kernel.org; > linux-arm-ker...@lists.infradead.org; linux- > ker...@vger.kernel.org; Sharma Bhupesh-B45370; Yoder Stuart-B08248; Li > Yang-Leo-R58472 > Subject: Re: [PATCH] arm64: dts: Added syscon-reboot node for FSL's LS2085A > SoC > > On Fri, Oct 23, 2015 at 08:31:20PM -0500, J. German Rivera wrote: > > Added sys-reboot node to the FSL's LS2085A SoC DT to leverage > > the ARM-generic reboot mechanism for this SoC. This mechanism > > is enabled through CONFIG_POWER_RESET_SYSCON. > > Per the comments in arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi, the > platform has PSCI 0.2+, and therefore already has system reset > functionality. > > Given that, why is this necessary? > > Thanks, > Mark. > > > Signed-off-by: J. German Rivera> > --- > > arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi | 12 > > 1 file changed, 12 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi > > b/arch/arm64/boot/dts/freescale/fsl- > ls2085a.dtsi > > index e281ceb..6f82163 100644 > > --- a/arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi > > +++ b/arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi > > @@ -131,6 +131,18 @@ > > interrupts = <1 9 0x4>; > > }; > > > > + rst_ccsr: rstccsr@1E6 { > > + compatible = "syscon"; > > + reg = <0x0 0x1E6 0x0 0x1>; > > + }; > > + > > + reboot@65024000 { > > + compatible ="syscon-reboot"; > > + regmap = <_ccsr>; > > + offset = <0x0>; > > + mask = <0x2>; > > + }; > > + > > timer { > > compatible = "arm,armv8-timer"; > > interrupts = <1 13 0x8>, /* Physical Secure PPI, active-low */ > > -- > > 2.3.3 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/5] staging: fsl-mc: Management Complex restool driver
> -Original Message- > From: Lijun Pan [mailto:lijun@freescale.com] > Sent: Sunday, October 25, 2015 5:41 PM > To: gre...@linuxfoundation.org; a...@arndb.de; de...@driverdev.osuosl.org; > linux-kernel@vger.kernel.org > Cc: Yoder Stuart-B08248; katz Itai-RM05202; Rivera Jose-B46482; Li > Yang-Leo-R58472; Wood Scott-B07421; > ag...@suse.de; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Sharma > Bhupesh-B45370; Erez Nir-RM30794; > Schmitt Richard-B43082; dan.carpen...@oracle.com; Pan Lijun-B44306 > Subject: [PATCH 5/5] staging: fsl-mc: Management Complex restool driver > > The kernel support for the restool (a user space resource management > tool) is a driver for the /dev/dprc.N device file. > Its purpose is to provide an ioctl interface, > which the restool uses to interact with the MC bus driver > and with the MC firmware. > We allocate a dpmcp at driver initialization, > and keep that dpmcp until driver exit. > We use that dpmcp by default. > If that dpmcp is in use, we create another portal at run time > and destroy the newly created portal after use. > The ioctl RESTOOL_SEND_MC_COMMAND sends user space command to fsl-mc > bus and utilizes the fsl-mc bus to communicate with MC firmware. > The ioctl RESTOOL_DPRC_SYNC request the mc-bus launch > objects scan under root dprc. > In order to support multiple root dprc, we utilize the bus notify > mechanism to scan fsl_mc_bus_type for the newly added root dprc. > After discovering the root dprc, it creates a miscdevice > /dev/dprc.N to associate with this root dprc. > > Signed-off-by: Lijun Pan> --- > drivers/staging/fsl-mc/bus/Kconfig | 7 +- > drivers/staging/fsl-mc/bus/Makefile | 3 + > drivers/staging/fsl-mc/bus/mc-ioctl.h | 24 ++ > drivers/staging/fsl-mc/bus/mc-restool.c | 488 > > 4 files changed, 521 insertions(+), 1 deletion(-) > create mode 100644 drivers/staging/fsl-mc/bus/mc-ioctl.h > create mode 100644 drivers/staging/fsl-mc/bus/mc-restool.c > > diff --git a/drivers/staging/fsl-mc/bus/Kconfig > b/drivers/staging/fsl-mc/bus/Kconfig > index 0d779d9..39c6ef9 100644 > --- a/drivers/staging/fsl-mc/bus/Kconfig > +++ b/drivers/staging/fsl-mc/bus/Kconfig > @@ -21,4 +21,9 @@ config FSL_MC_BUS > Only enable this option when building the kernel for > Freescale QorQIQ LS2 SoCs. > > - > +config FSL_MC_RESTOOL > +tristate "Freescale Management Complex (MC) restool driver" > +depends on FSL_MC_BUS > +help > + Driver that provides kernel support for the Freescale Management > + Complex resource manager user-space tool. > diff --git a/drivers/staging/fsl-mc/bus/Makefile > b/drivers/staging/fsl-mc/bus/Makefile > index 25433a9..28b5fc0 100644 > --- a/drivers/staging/fsl-mc/bus/Makefile > +++ b/drivers/staging/fsl-mc/bus/Makefile > @@ -15,3 +15,6 @@ mc-bus-driver-objs := mc-bus.o \ > mc-allocator.o \ > dpmcp.o \ > dpbp.o > + > +# MC restool kernel support > +obj-$(CONFIG_FSL_MC_RESTOOL) += mc-restool.o > diff --git a/drivers/staging/fsl-mc/bus/mc-ioctl.h > b/drivers/staging/fsl-mc/bus/mc-ioctl.h > new file mode 100644 > index 000..e52f907 > --- /dev/null > +++ b/drivers/staging/fsl-mc/bus/mc-ioctl.h > @@ -0,0 +1,24 @@ > +/* > + * Freescale Management Complex (MC) ioclt interface > + * > + * Copyright (C) 2014 Freescale Semiconductor, Inc. > + * Author: Lijun Pan > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > +#ifndef _FSL_MC_IOCTL_H_ > +#define _FSL_MC_IOCTL_H_ > + > +#include > + > +#define RESTOOL_IOCTL_TYPE 'R' > + > +#define RESTOOL_DPRC_SYNC \ > + _IO(RESTOOL_IOCTL_TYPE, 0x2) We no longer need a sync ioctl, as a the /sys/bus/fsl-mc/rescan mechanism in the previous patch replaces this. > +#define RESTOOL_SEND_MC_COMMAND \ > + _IOWR(RESTOOL_IOCTL_TYPE, 0x4, struct mc_command) > + > +#endif /* _FSL_MC_IOCTL_H_ */ > diff --git a/drivers/staging/fsl-mc/bus/mc-restool.c > b/drivers/staging/fsl-mc/bus/mc-restool.c > new file mode 100644 > index 000..a219172 > --- /dev/null > +++ b/drivers/staging/fsl-mc/bus/mc-restool.c > @@ -0,0 +1,488 @@ > +/* > + * Freescale Management Complex (MC) restool driver > + * > + * Copyright (C) 2014 Freescale Semiconductor, Inc. > + * Author: Lijun Pan > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#include "../include/mc-private.h" > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "mc-ioctl.h" > +#include "../include/mc-sys.h" > +#include
RE: [PATCH v4] arm64: dts: Added syscon-reboot node for FSL's LS2085A SoC
> -Original Message- > From: J. German Rivera [mailto:german.riv...@freescale.com] > Sent: Friday, October 30, 2015 3:05 PM > To: robh...@kernel.org; mark.rutl...@arm.com; devicet...@vger.kernel.org; > linux-arm- > ker...@lists.infradead.org; linux-kernel@vger.kernel.org > Cc: Sharma Bhupesh-B45370; Yoder Stuart-B08248; Li Yang-Leo-R58472; Rivera > Jose-B46482 > Subject: [PATCH v4] arm64: dts: Added syscon-reboot node for FSL's LS2085A SoC > > Added sys-reboot node to the FSL's LS2085A SoC DT to leverage > the ARM-generic reboot mechanism for this SoC. This mechanism > is enabled through CONFIG_POWER_RESET_SYSCON. German, one thing I noticed with this patch-- with the name change to ls2080a, we should update the patch subject and commit message to reflect that. Thanks, Stuart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v2] arm64: dts: Added syscon-reboot node for FSL's LS2085A SoC
> -Original Message- > From: Arnd Bergmann [mailto:a...@arndb.de] > Sent: Thursday, October 29, 2015 4:49 PM > To: linux-arm-ker...@lists.infradead.org > Cc: Rivera Jose-B46482; robh...@kernel.org; mark.rutl...@arm.com; > devicet...@vger.kernel.org; linux- > ker...@vger.kernel.org; Sharma Bhupesh-B45370; Li Yang-Leo-R58472; Yoder > Stuart-B08248 > Subject: Re: [PATCH v2] arm64: dts: Added syscon-reboot node for FSL's > LS2085A SoC > > On Wednesday 28 October 2015 16:09:44 J. German Rivera wrote: > > + rst_ccsr: rstccsr@1E6 { > > + compatible = "syscon"; > > + reg = <0x0 0x1E6 0x0 0x1>; > > + }; > > + > > > > What does 'rstccsr' stand for? Is this by chance a reset controller? It is a region of some miscellaneous registers, some related to reset. > If so, we probably want a real driver for it rather than just a > syscon. There is not other stuff in this region that to be exposed, and so we really don't need a real driver. My suggestion is to perhaps make that more explicit in the proposed device tree node, by making the reg size "4" and naming it as per the register name rstcr: rstcr@1E6 { compatible = "syscon"; reg = <0x0 0x1E6 0x0 0x4>; }; The intent is really just to expose a single reset register and use syscon-reboot until PSCI is available. Thanks, Stuart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] arm64: dts: Added syscon-reboot node for FSL's LS2085A SoC
> -Original Message- > From: Arnd Bergmann [mailto:a...@arndb.de] > Sent: Friday, October 30, 2015 8:36 AM > To: linux-arm-ker...@lists.infradead.org > Cc: Yoder Stuart-B08248; Mark Rutland; devicet...@vger.kernel.org; Sharma > Bhupesh-B45370; Rivera Jose-B46482; > linux-kernel@vger.kernel.org; robh...@kernel.org; Li Yang-Leo-R58472 > Subject: Re: [PATCH] arm64: dts: Added syscon-reboot node for FSL's LS2085A > SoC > > On Tuesday 27 October 2015 18:25:04 Stuart Yoder wrote: > > I think that comment "Currently supported enable-method is psci v0.2" is a > > statement of > > intent, not what is available currently. And the only plan I am aware of > > is PSCI > > with UEFI based firmware. > > > > U-boot is a key firmware platform for us and has no PSCI implementation > > available. > > I believe U-boot has supported PSCI since 2013. Maybe you just need to update > to a newer version? We are using a very recent u-boot. U-boot for ARMv8, which is what this SoC is based on, does not have PSCI as far as I can tell. We (Freescale) plan to eventually have an EL3 firmware component that could remain resident for u-boot based systems, but it is a roadmap thing and I don't know when we're going to get it. So, for now, most straightforward thing is to get reboot functionality is to expose the reset register via syscon-reboot in the DT...no new driver needed. Once a PSCI component is available we'll move to it. Thanks, Stuart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3] arm64: dts: Added syscon-reboot node for FSL's LS2085A SoC
> -Original Message- > From: J. German Rivera [mailto:german.riv...@freescale.com] > Sent: Friday, October 30, 2015 12:08 PM > To: robh...@kernel.org; mark.rutl...@arm.com; devicet...@vger.kernel.org; > linux-arm- > ker...@lists.infradead.org; linux-kernel@vger.kernel.org > Cc: Sharma Bhupesh-B45370; Yoder Stuart-B08248; Li Yang-Leo-R58472; Rivera > Jose-B46482 > Subject: [PATCH v3] arm64: dts: Added syscon-reboot node for FSL's LS2085A SoC > > Added sys-reboot node to the FSL's LS2085A SoC DT to leverage > the ARM-generic reboot mechanism for this SoC. This mechanism > is enabled through CONFIG_POWER_RESET_SYSCON. > > Signed-off-by: J. German Rivera <german.riv...@freescale.com> > --- > CHANGE HISTORY: > > Changes in v2: > - Addressed comment form Stuart Yoder: > * Removed "@" from reboot node > > Changes in v3: > - Addressed comment form Stuart Yoder: > * Expose only the reset register > > arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi | 12 > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi > b/arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi > index e281ceb..5e3d894 100644 > --- a/arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi > +++ b/arch/arm64/boot/dts/freescale/fsl-ls2085a.dtsi > @@ -131,6 +131,18 @@ > interrupts = <1 9 0x4>; > }; > > + rstcr: rstcr@1E6 { > + compatible = "syscon"; > + reg = <0x0 0x1E6 0x0 0x4>; > + }; > + > + reboot { > + compatible ="syscon-reboot"; > + regmap = <>; > + offset = <0x0>; > + mask = <0x2>; > + }; > + > timer { > compatible = "arm,armv8-timer"; > interrupts = <1 13 0x8>, /* Physical Secure PPI, active-low */ German, the other thing I just noticed that I had mentioned on v1 of the patch was that the device tree has been renamed, so fsl-ls2085a.dtsi no longer exists. You need to rebase this on top of: git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git branch: for-next Thanks, Stuart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/3] Docs: dt: Add PCI MSI map bindings
> -Original Message- > From: Mark Rutland [mailto:mark.rutl...@arm.com] > Sent: Monday, September 07, 2015 1:05 PM > To: David Daney > Cc: Marc Zyngier; tirumalesh.chalama...@caviumnetworks.com; Richter, Robert; > Chintakuntla, Radha; > devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; > io...@lists.linux-foundation.org; linux-arm- > ker...@lists.infradead.org; Will Deacon; Robin Murphy; Lorenzo Pieralisi; > a...@arndb.de; tred...@nvidia.com; > majun...@huawei.com; thunder.leiz...@huawei.com; > laurent.pinch...@ideasonboard.com; Yoder Stuart-B08248 > Subject: Re: [PATCH 2/3] Docs: dt: Add PCI MSI map bindings > > On Fri, Sep 04, 2015 at 11:33:35PM +0100, David Daney wrote: > > Hi Mark, > > Hi David, > > > I now have a prototype implementation for irq-gic-v3-its.c that is using > > this binding on Cavium's ThunderX platform. > > > > Q: Have you guys had any more thoughts on this that might require > > changing the binding? > > Having discussed this with Stuart and others at Linux Plumbers, I think > that the binding is sufficient, and unlikely to change greatly unless > there is a strong objection (Stuart, please correct me if I am wrong). > > Per Marc's comments there are probably some edge cases and/or wording > details to sort out, but I think the common/simple case is sorted. I'll > send a v2 once those have been settled. I think the binding as-is, is sufficient for the static description of RID to SID. I think the binding can be extended in a backwards compatible way to support dynamic RID->SID mappings if we need that in the future. The case that we (Freescale) are concerned with are where someone plugs an SRIOV card into an SoC and enables, say 64 VFs. It's like hot-plugging 64 new PCI devices at once. If all those devices that show up belong to the host they belong to one 'isolation context' and could share the same streamID, same SMMU mappings, and the same ITT in the GIC ITS. So a static mapping could work. But, as soon as I want to assign VF#5 to a virtual machine I need a new RID->SID mapping for VF#5. To require firmware to do that mapping ahead of time is a _real_ pain. I think longer term we need some mechanism to allow lazy, dynamic RID->SID mappings by Linux. We can start with the static approach, but we need to keep in the back of our minds that there may be cases in the near future where a static mapping is too inflexible. Thanks, Stuart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] Docs: dt: update binding for Freescale Management Complex
Update the binding for the Freescale Management Complex to include definition of ranges, msi-parent, and dpmac subnodes. Stuart Yoder (3): Docs: dt: fsl-mc: update binding to include msi-parent Docs: dt: fsl-mc update binding to include definition of ranges Docs: dt: fsl-mc: update binding to define dpmac subnodes .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 81 +- 1 file changed, 80 insertions(+), 1 deletion(-) -- 2.3.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] Docs: dt: fsl-mc update binding to include definition of ranges
Define a ranges property to specify the mapping between the MC address space and the system address space. Signed-off-by: Stuart Yoder <stuart.yo...@freescale.com> --- .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 30 +- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt index 6aac955..848975f 100644 --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt @@ -35,6 +35,26 @@ Required properties: Definition: Must be present and point to the MSI controller node handling message interrupts for the MC. +- ranges +Value type: +Definition: A standard property. Defines the mapping between the child +MC address space and the parent system address space. + +The MC address space is defined by 3 components: + + +Valid values for region type are + 0x0 - MC portals + 0x1 - QBMAN portals + +- #address-cells +Value type: +Definition: Must be 3. (see definition in 'ranges' property) + +- #size-cells +Value type: +Definition: Must be 1. + Example: fsl_mc: fsl-mc@80c00 { @@ -42,5 +62,13 @@ Example: reg = <0x0008 0x0c00 0 0x40>,/* MC portal base */ <0x 0x0834 0 0x4>; /* MC control reg */ msi-parent = <>; -}; +#address-cells = <3>; +#size-cells = <1>; +/* + * Region type 0x0 - MC portals + * Region type 0x1 - QBMAN portals + */ +ranges = <0x0 0x0 0x0 0x8 0x0c00 0x400 + 0x1 0x0 0x0 0x8 0x1800 0x800>; +}; -- 2.3.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] Docs: dt: fsl-mc: update binding to include msi-parent
The Freescale Management Complex and all associated objects use message interrupts, and thus an msi-parent is required. Signed-off-by: Stuart Yoder <stuart.yo...@freescale.com> --- Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt index c7a26ca..6aac955 100644 --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt @@ -30,11 +30,17 @@ Required properties: region may not be present in some scenarios, such as in the device tree presented to a virtual machine. +- msi-parent +Value type: +Definition: Must be present and point to the MSI controller node +handling message interrupts for the MC. + Example: fsl_mc: fsl-mc@80c00 { compatible = "fsl,qoriq-mc"; reg = <0x0008 0x0c00 0 0x40>,/* MC portal base */ <0x 0x0834 0 0x4>; /* MC control reg */ +msi-parent = <>; }; -- 2.3.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] Docs: dt: fsl-mc: update binding to define dpmac subnodes
The fsl-mc node may optionally have dpmac sub-nodes that describe the relationship between the Ethernet MACs which belong to the MC and the Ethernet PHYs on the system board. Signed-off-by: Stuart Yoder <stuart.yo...@freescale.com> --- .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 45 ++ 1 file changed, 45 insertions(+) diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt index 848975f..6611a7c 100644 --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt @@ -55,6 +55,40 @@ Required properties: Value type: Definition: Must be 1. +Sub-nodes: + +The fsl-mc node may optionally have dpmac sub-nodes that describe +the relationship between the Ethernet MACs which belong to the MC +and the Ethernet PHYs on the system board. + +The dpmac nodes must be under a node named "dpmacs" which contains +the following properties: + +- #address-cells + Value type: + Definition: Must be present if dpmac sub-nodes are defined and must + have a value of 1. + +- #size-cells + Value type: + Definition: Must be present if dpmac sub-nodes are defined and must + have a value of 0. + +These nodes must have the following properties: + +- compatible + Value type: + Definition: Must be "fsl,qoriq-mc-dpmac". + +- reg + Value type: + Definition: Specifies the id of the dpmac. + +- phy-handle + Value type: + Definition: Specifies the phandle to the PHY device node associated + with the this dpmac. + Example: fsl_mc: fsl-mc@80c00 { @@ -71,4 +105,15 @@ Example: */ ranges = <0x0 0x0 0x0 0x8 0x0c00 0x400 0x1 0x0 0x0 0x8 0x1800 0x800>; + +dpmacs { +#address-cells = <1>; +#size-cells = <0>; + +dpmac@1 { +compatible = "fsl,qoriq-mc-dpmac"; +reg = <1>; +phy-handle = <_phy0>; +} +} }; -- 2.3.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 08/11] staging: fsl-mc: dprc: add missing irq free
add missing free of the Linux irq when tearing down interrupts Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dprc-driver.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c index 1a6bcc4..96ee1b7 100644 --- a/drivers/staging/fsl-mc/bus/dprc-driver.c +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c @@ -760,7 +760,12 @@ error_cleanup_msi_domain: */ static void dprc_teardown_irq(struct fsl_mc_device *mc_dev) { + struct fsl_mc_device_irq *irq = mc_dev->irqs[0]; + (void)disable_dprc_irq(mc_dev); + + devm_free_irq(_dev->dev, irq->msi_desc->irq, _dev->dev); + fsl_mc_free_irqs(mc_dev); } -- 1.9.0
[PATCH 11/11] staging: fsl-mc: convert mc command build/parse to use C structs
From: Ioana Radulescu <ruxandra.radule...@nxp.com> The layer abstracting the building of commands and extracting responses is currently based on macros that shift and mask the command fields and requires exposing offset/size values as macro parameters and makes the code harder to read. For clarity and maintainability, instead use an implementation based on mapping the MC command definitions to C structures. These structures contain the hardware command fields (which are naturally-aligned) and individual fields are little-endian ordering (the byte ordering of the hardware). As such, there is no need to perform the conversion between core and hardware (LE) endianness in mc_send_command(), but instead each individual field in a command will be converted separately if needed by the function building the command or extracting the response. This patch does not introduce functional changes, both the hardware ABIs and the APIs exposed for the DPAA2 objects remain the same. Signed-off-by: Ioana Radulescu <ruxandra.radule...@nxp.com> Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dpbp.c | 132 -- drivers/staging/fsl-mc/bus/dpmcp-cmd.h| 86 +++- drivers/staging/fsl-mc/bus/dpmcp.c| 89 ++-- drivers/staging/fsl-mc/bus/dpmng-cmd.h| 12 +- drivers/staging/fsl-mc/bus/dpmng.c| 14 +- drivers/staging/fsl-mc/bus/dprc-cmd.h | 379 +++- drivers/staging/fsl-mc/bus/dprc.c | 715 ++ drivers/staging/fsl-mc/bus/mc-sys.c | 46 +- drivers/staging/fsl-mc/include/dpbp-cmd.h | 125 +- drivers/staging/fsl-mc/include/mc-cmd.h | 91 ++-- 10 files changed, 1056 insertions(+), 633 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dpbp.c b/drivers/staging/fsl-mc/bus/dpbp.c index c31fe1b..fe271fb 100644 --- a/drivers/staging/fsl-mc/bus/dpbp.c +++ b/drivers/staging/fsl-mc/bus/dpbp.c @@ -1,4 +1,4 @@ -/* Copyright 2013-2014 Freescale Semiconductor Inc. +/* Copyright 2013-2016 Freescale Semiconductor Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: @@ -57,12 +57,14 @@ int dpbp_open(struct fsl_mc_io *mc_io, u16 *token) { struct mc_command cmd = { 0 }; + struct dpbp_cmd_open *cmd_params; int err; /* prepare command */ cmd.header = mc_encode_cmd_header(DPBP_CMDID_OPEN, cmd_flags, 0); - cmd.params[0] |= mc_enc(0, 32, dpbp_id); + cmd_params = (struct dpbp_cmd_open *)cmd.params; + cmd_params->dpbp_id = cpu_to_le32(dpbp_id); /* send command to mc*/ err = mc_send_command(mc_io, ); @@ -70,7 +72,7 @@ int dpbp_open(struct fsl_mc_io *mc_io, return err; /* retrieve response parameters */ - *token = MC_CMD_HDR_READ_TOKEN(cmd.header); + *token = mc_cmd_hdr_read_token(); return err; } @@ -143,7 +145,7 @@ int dpbp_create(struct fsl_mc_io *mc_io, return err; /* retrieve response parameters */ - *token = MC_CMD_HDR_READ_TOKEN(cmd.header); + *token = mc_cmd_hdr_read_token(); return 0; } @@ -231,6 +233,7 @@ int dpbp_is_enabled(struct fsl_mc_io *mc_io, int *en) { struct mc_command cmd = { 0 }; + struct dpbp_rsp_is_enabled *rsp_params; int err; /* prepare command */ cmd.header = mc_encode_cmd_header(DPBP_CMDID_IS_ENABLED, cmd_flags, @@ -242,7 +245,8 @@ int dpbp_is_enabled(struct fsl_mc_io *mc_io, return err; /* retrieve response parameters */ - *en = (int)mc_dec(cmd.params[0], 0, 1); + rsp_params = (struct dpbp_rsp_is_enabled *)cmd.params; + *en = rsp_params->enabled & DPBP_ENABLE; return 0; } @@ -286,14 +290,16 @@ int dpbp_set_irq(struct fsl_mc_io *mc_io, struct dpbp_irq_cfg *irq_cfg) { struct mc_command cmd = { 0 }; + struct dpbp_cmd_set_irq *cmd_params; /* prepare command */ cmd.header = mc_encode_cmd_header(DPBP_CMDID_SET_IRQ, cmd_flags, token); - cmd.params[0] |= mc_enc(0, 8, irq_index); - cmd.params[0] |= mc_enc(32, 32, irq_cfg->val); - cmd.params[1] |= mc_enc(0, 64, irq_cfg->addr); - cmd.params[2] |= mc_enc(0, 32, irq_cfg->irq_num); + cmd_params = (struct dpbp_cmd_set_irq *)cmd.params; + cmd_params->irq_index = irq_index; + cmd_params->irq_val = cpu_to_le32(irq_cfg->val); + cmd_params->irq_addr = cpu_to_le64(irq_cfg->addr); + cmd_params->irq_num = cpu_to_le32(irq_cfg->irq_num); /* send command to mc*/ return mc_send_command(mc_io, ); @@ -319,12 +325,15 @@ int dpbp_get_irq(struct fsl_mc_io *mc_io, struct dpbp_irq_cfg *irq_cfg
[PATCH 00/11] staging: fsl-mc: module loading support, fixes, and cleanup
This patch series does some cleanup and further sets the stage for additional fsl-mc device drivers. -Patches 1-4 add missing fsl-mc support for modalias and udev-based module loading of drivers. -Patch 5 exports a function some drivers rely on. -Patch 6 makes a needed helper function visible to the dprc driver. -Patch 7 fixes a bug where an asymmetry existed in how mc_io structs were destroyed. -Patch 8 fixes a bug where an irq free was missing in the dprc driver. -Patch 9 fixes a bug where there was an ordering issue with how resources were freed. -Patch 10 fixes a bug with how hwirq numbers were determined, which prevented more than one dprc from being used by the kernel -Patch 11 is a cleanup patch to improve the readability/maintainability of the functions that build/parse MC commands. It uses structs instead of the previous shift/mask macros. This sets the precedence we want other new drivers to follow. Bharat Bhushan (1): staging: fsl-mc: fix asymmetry in destroy of mc_io Ioana Radulescu (1): staging: fsl-mc: convert mc command build/parse to use C structs Stuart Yoder (9): staging: fsl-mc: add support for the modalias sysfs attribute staging: fsl-mc: implement uevent callback and set the modalias staging: fsl-mc: clean up the device id struct staging: fsl-mc: add support for device table matching staging: fsl-mc: export mc_get_version staging: fsl-mc: make fsl_mc_is_root_dprc() global staging: fsl-mc: dprc: add missing irq free staging: fsl-mc: dprc: fix ordering problem freeing resources in remove of dprc staging: fsl-mc: properly set hwirq in msi set_desc drivers/staging/fsl-mc/bus/dpbp.c | 132 -- drivers/staging/fsl-mc/bus/dpmcp-cmd.h| 86 +++- drivers/staging/fsl-mc/bus/dpmcp.c| 89 ++-- drivers/staging/fsl-mc/bus/dpmng-cmd.h| 12 +- drivers/staging/fsl-mc/bus/dpmng.c| 15 +- drivers/staging/fsl-mc/bus/dprc-cmd.h | 379 +++- drivers/staging/fsl-mc/bus/dprc-driver.c | 20 +- drivers/staging/fsl-mc/bus/dprc.c | 715 ++ drivers/staging/fsl-mc/bus/mc-allocator.c | 2 +- drivers/staging/fsl-mc/bus/mc-bus.c | 71 ++- drivers/staging/fsl-mc/bus/mc-msi.c | 17 +- drivers/staging/fsl-mc/bus/mc-sys.c | 46 +- drivers/staging/fsl-mc/include/dpbp-cmd.h | 125 +- drivers/staging/fsl-mc/include/mc-cmd.h | 91 ++-- drivers/staging/fsl-mc/include/mc.h | 21 +- include/linux/mod_devicetable.h | 16 + scripts/mod/devicetable-offsets.c | 4 + scripts/mod/file2alias.c | 12 + 18 files changed, 1175 insertions(+), 678 deletions(-) -- 1.9.0
[PATCH 10/11] staging: fsl-mc: properly set hwirq in msi set_desc
For an MSI domain the hwirq is an arbitrary but unique id to identify an interrupt. Previously the hwirq was set to the MSI index of the interrupt, but that only works if there is one DPRC. Additional DPRCs require an expanded namespace. Use both the ICID (which is unique per DPRC) and the MSI index to compose a hwirq value. Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/mc-msi.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/staging/fsl-mc/bus/mc-msi.c b/drivers/staging/fsl-mc/bus/mc-msi.c index e202b2b..c7be156 100644 --- a/drivers/staging/fsl-mc/bus/mc-msi.c +++ b/drivers/staging/fsl-mc/bus/mc-msi.c @@ -20,11 +20,26 @@ #include "../include/mc-sys.h" #include "dprc-cmd.h" +/* + * Generate a unique ID identifying the interrupt (only used within the MSI + * irqdomain. Combine the icid with the interrupt index. + */ +static irq_hw_number_t fsl_mc_domain_calc_hwirq(struct fsl_mc_device *dev, + struct msi_desc *desc) +{ + /* +* Make the base hwirq value for ICID*1 so it is readable +* as a decimal value in /proc/interrupts. +*/ + return (irq_hw_number_t)(desc->fsl_mc.msi_index + (dev->icid * 1)); +} + static void fsl_mc_msi_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc) { arg->desc = desc; - arg->hwirq = (irq_hw_number_t)desc->fsl_mc.msi_index; + arg->hwirq = fsl_mc_domain_calc_hwirq(to_fsl_mc_device(desc->dev), + desc); } static void fsl_mc_msi_update_dom_ops(struct msi_domain_info *info) -- 1.9.0
[PATCH 03/11] staging: fsl-mc: clean up the device id struct
-rename the struct used for fsl-mc device ids to be more consistent with other busses -remove the now obsolete and unused version fields Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dprc-driver.c | 2 +- drivers/staging/fsl-mc/bus/mc-allocator.c | 2 +- drivers/staging/fsl-mc/bus/mc-bus.c | 2 +- drivers/staging/fsl-mc/include/mc.h | 10 +++--- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c index 7fc4717..f865d18 100644 --- a/drivers/staging/fsl-mc/bus/dprc-driver.c +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c @@ -805,7 +805,7 @@ static int dprc_remove(struct fsl_mc_device *mc_dev) return 0; } -static const struct fsl_mc_device_match_id match_id_table[] = { +static const struct fsl_mc_device_id match_id_table[] = { { .vendor = FSL_MC_VENDOR_FREESCALE, .obj_type = "dprc"}, diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c b/drivers/staging/fsl-mc/bus/mc-allocator.c index fb08f22..e59d850 100644 --- a/drivers/staging/fsl-mc/bus/mc-allocator.c +++ b/drivers/staging/fsl-mc/bus/mc-allocator.c @@ -717,7 +717,7 @@ static int fsl_mc_allocator_remove(struct fsl_mc_device *mc_dev) return 0; } -static const struct fsl_mc_device_match_id match_id_table[] = { +static const struct fsl_mc_device_id match_id_table[] = { { .vendor = FSL_MC_VENDOR_FREESCALE, .obj_type = "dpbp", diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c index cf92a1c..e975adc 100644 --- a/drivers/staging/fsl-mc/bus/mc-bus.c +++ b/drivers/staging/fsl-mc/bus/mc-bus.c @@ -36,7 +36,7 @@ static bool fsl_mc_is_root_dprc(struct device *dev); */ static int fsl_mc_bus_match(struct device *dev, struct device_driver *drv) { - const struct fsl_mc_device_match_id *id; + const struct fsl_mc_device_id *id; struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev); struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(drv); bool found = false; diff --git a/drivers/staging/fsl-mc/include/mc.h b/drivers/staging/fsl-mc/include/mc.h index ac7c1ce..bc0d45c 100644 --- a/drivers/staging/fsl-mc/include/mc.h +++ b/drivers/staging/fsl-mc/include/mc.h @@ -39,7 +39,7 @@ struct fsl_mc_bus; */ struct fsl_mc_driver { struct device_driver driver; - const struct fsl_mc_device_match_id *match_id_table; + const struct fsl_mc_device_id *match_id_table; int (*probe)(struct fsl_mc_device *dev); int (*remove)(struct fsl_mc_device *dev); void (*shutdown)(struct fsl_mc_device *dev); @@ -51,20 +51,16 @@ struct fsl_mc_driver { container_of(_drv, struct fsl_mc_driver, driver) /** - * struct fsl_mc_device_match_id - MC object device Id entry for driver matching + * struct fsl_mc_device_id - MC object device Id entry for driver matching * @vendor: vendor ID * @obj_type: MC object type - * @ver_major: MC object version major number - * @ver_minor: MC object version minor number * * Type of entries in the "device Id" table for MC object devices supported by * a MC object device driver. The last entry of the table has vendor set to 0x0 */ -struct fsl_mc_device_match_id { +struct fsl_mc_device_id { u16 vendor; const char obj_type[16]; - u32 ver_major; - u32 ver_minor; }; /** -- 1.9.0
[PATCH 01/11] staging: fsl-mc: add support for the modalias sysfs attribute
In order to support uevent based module loading implement modalias support for the fsl-mc bus driver. Aliases are based on vendor and object/device id and are of the form "fsl-mc:vNdN". Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/mc-bus.c | 25 + 1 file changed, 25 insertions(+) diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c index 4053643..d8776dd 100644 --- a/drivers/staging/fsl-mc/bus/mc-bus.c +++ b/drivers/staging/fsl-mc/bus/mc-bus.c @@ -82,10 +82,35 @@ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env) return 0; } +static ssize_t modalias_show(struct device *dev, struct device_attribute *attr, +char *buf) +{ + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev); + + return sprintf(buf, "fsl-mc:v%08Xd%s\n", mc_dev->obj_desc.vendor, + mc_dev->obj_desc.type); +} +static DEVICE_ATTR_RO(modalias); + +static struct attribute *fsl_mc_dev_attrs[] = { + _attr_modalias.attr, + NULL, +}; + +static const struct attribute_group fsl_mc_dev_group = { + .attrs = fsl_mc_dev_attrs, +}; + +static const struct attribute_group *fsl_mc_dev_groups[] = { + _mc_dev_group, + NULL, +}; + struct bus_type fsl_mc_bus_type = { .name = "fsl-mc", .match = fsl_mc_bus_match, .uevent = fsl_mc_bus_uevent, + .dev_groups = fsl_mc_dev_groups, }; EXPORT_SYMBOL_GPL(fsl_mc_bus_type); -- 1.9.0
[PATCH 04/11] staging: fsl-mc: add support for device table matching
Move the definition of fsl_mc_device_id to its proper location in mod_devicetable.h, and add fsl-mc bus support to devicetable-offsets.c and file2alias.c to enable device table matching. With this patch udev based module loading of fsl-mc drivers is supported. Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/include/mc.h | 13 - include/linux/mod_devicetable.h | 16 scripts/mod/devicetable-offsets.c | 4 scripts/mod/file2alias.c| 12 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/drivers/staging/fsl-mc/include/mc.h b/drivers/staging/fsl-mc/include/mc.h index bc0d45c..a9a9d23 100644 --- a/drivers/staging/fsl-mc/include/mc.h +++ b/drivers/staging/fsl-mc/include/mc.h @@ -51,19 +51,6 @@ struct fsl_mc_driver { container_of(_drv, struct fsl_mc_driver, driver) /** - * struct fsl_mc_device_id - MC object device Id entry for driver matching - * @vendor: vendor ID - * @obj_type: MC object type - * - * Type of entries in the "device Id" table for MC object devices supported by - * a MC object device driver. The last entry of the table has vendor set to 0x0 - */ -struct fsl_mc_device_id { - u16 vendor; - const char obj_type[16]; -}; - -/** * enum fsl_mc_pool_type - Types of allocatable MC bus resources * * Entries in these enum are used as indices in the array of resource diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index 6e4c645..ed84c07 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -657,4 +657,20 @@ struct ulpi_device_id { kernel_ulong_t driver_data; }; +/** + * struct fsl_mc_device_id - MC object device identifier + * @vendor: vendor ID + * @obj_type: MC object type + * @ver_major: MC object version major number + * @ver_minor: MC object version minor number + * + * Type of entries in the "device Id" table for MC object devices supported by + * a MC object device driver. The last entry of the table has vendor set to 0x0 + */ +struct fsl_mc_device_id { + __u16 vendor; + const char obj_type[16]; +}; + + #endif /* LINUX_MOD_DEVICETABLE_H */ diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c index 840b973..e4d90e5 100644 --- a/scripts/mod/devicetable-offsets.c +++ b/scripts/mod/devicetable-offsets.c @@ -202,5 +202,9 @@ int main(void) DEVID_FIELD(hda_device_id, rev_id); DEVID_FIELD(hda_device_id, api_version); + DEVID(fsl_mc_device_id); + DEVID_FIELD(fsl_mc_device_id, vendor); + DEVID_FIELD(fsl_mc_device_id, obj_type); + return 0; } diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index a915507..b3f88a3 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -1289,6 +1289,18 @@ static int do_hda_entry(const char *filename, void *symval, char *alias) } ADD_TO_DEVTABLE("hdaudio", hda_device_id, do_hda_entry); +/* Looks like: fsl-mc:vNdN */ +static int do_fsl_mc_entry(const char *filename, void *symval, + char *alias) +{ + DEF_FIELD(symval, fsl_mc_device_id, vendor); + DEF_FIELD_ADDR(symval, fsl_mc_device_id, obj_type); + + sprintf(alias, "fsl-mc:v%08Xd%s", vendor, *obj_type); + return 1; +} +ADD_TO_DEVTABLE("fslmc", fsl_mc_device_id, do_fsl_mc_entry); + /* Does namelen bytes of name exactly match the symbol? */ static bool sym_is(const char *name, unsigned namelen, const char *symbol) { -- 1.9.0
[PATCH 09/11] staging: fsl-mc: dprc: fix ordering problem freeing resources in remove of dprc
When unbinding a dprc from the dprc driver the cleanup of the resource pools must happen after irq pool cleanup is done. Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dprc-driver.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c index 96ee1b7..d2a71f1 100644 --- a/drivers/staging/fsl-mc/bus/dprc-driver.c +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c @@ -796,16 +796,18 @@ static int dprc_remove(struct fsl_mc_device *mc_dev) dprc_teardown_irq(mc_dev); device_for_each_child(_dev->dev, NULL, __fsl_mc_device_remove); - dprc_cleanup_all_resource_pools(mc_dev); - error = dprc_close(mc_dev->mc_io, 0, mc_dev->mc_handle); - if (error < 0) - dev_err(_dev->dev, "dprc_close() failed: %d\n", error); if (dev_get_msi_domain(_dev->dev)) { fsl_mc_cleanup_irq_pool(mc_bus); dev_set_msi_domain(_dev->dev, NULL); } + dprc_cleanup_all_resource_pools(mc_dev); + + error = dprc_close(mc_dev->mc_io, 0, mc_dev->mc_handle); + if (error < 0) + dev_err(_dev->dev, "dprc_close() failed: %d\n", error); + if (!fsl_mc_is_root_dprc(_dev->dev)) { fsl_destroy_mc_io(mc_dev->mc_io); mc_dev->mc_io = NULL; -- 1.9.0
[PATCH 07/11] staging: fsl-mc: fix asymmetry in destroy of mc_io
From: Bharat Bhushan <bharat.bhus...@nxp.com> An mc_io represents a mapped MC portal. Previously, an mc_io was created for the root dprc in fsl_mc_bus_probe() and for child dprcs in dprc_probe(). But the free of that data structure happened in the general bus remove callback. This asymmetry resulted in some bugs due to unwanted destroys of mc_io object in some scenarios (e.g. vfio). Fix this bug by making things symmetric-- mc_io created in fsl_mc_bus_probe() is freed in fsl_mc_bus_remove(). The mc_io created in dprc_probe() is freed in dprc_remove(). Signed-off-by: Bharat Bhushan <bharat.bhus...@nxp.com> [Stuart: added check for root dprc and reworded commit message] Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dprc-driver.c | 5 + drivers/staging/fsl-mc/bus/mc-bus.c | 8 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c index f865d18..1a6bcc4 100644 --- a/drivers/staging/fsl-mc/bus/dprc-driver.c +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c @@ -801,6 +801,11 @@ static int dprc_remove(struct fsl_mc_device *mc_dev) dev_set_msi_domain(_dev->dev, NULL); } + if (!fsl_mc_is_root_dprc(_dev->dev)) { + fsl_destroy_mc_io(mc_dev->mc_io); + mc_dev->mc_io = NULL; + } + dev_info(_dev->dev, "DPRC device unbound from driver"); return 0; } diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c index a49186e..db3afdb 100644 --- a/drivers/staging/fsl-mc/bus/mc-bus.c +++ b/drivers/staging/fsl-mc/bus/mc-bus.c @@ -579,10 +579,6 @@ void fsl_mc_device_remove(struct fsl_mc_device *mc_dev) if (strcmp(mc_dev->obj_desc.type, "dprc") == 0) { mc_bus = to_fsl_mc_bus(mc_dev); - if (mc_dev->mc_io) { - fsl_destroy_mc_io(mc_dev->mc_io); - mc_dev->mc_io = NULL; - } if (fsl_mc_is_root_dprc(_dev->dev)) { if (atomic_read(_dprc_count) > 0) @@ -810,6 +806,10 @@ static int fsl_mc_bus_remove(struct platform_device *pdev) return -EINVAL; fsl_mc_device_remove(mc->root_mc_bus_dev); + + fsl_destroy_mc_io(mc->root_mc_bus_dev->mc_io); + mc->root_mc_bus_dev->mc_io = NULL; + dev_info(>dev, "Root MC bus device removed"); return 0; } -- 1.9.0
[PATCH 06/11] staging: fsl-mc: make fsl_mc_is_root_dprc() global
make fsl_mc_is_root_dprc() global so that the dprc driver can use it Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/mc-bus.c | 28 +--- drivers/staging/fsl-mc/include/mc.h | 2 ++ 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c index e975adc..a49186e 100644 --- a/drivers/staging/fsl-mc/bus/mc-bus.c +++ b/drivers/staging/fsl-mc/bus/mc-bus.c @@ -24,8 +24,6 @@ static struct kmem_cache *mc_dev_cache; -static bool fsl_mc_is_root_dprc(struct device *dev); - /** * fsl_mc_bus_match - device to driver matching callback * @dev: the MC object device structure to match against @@ -247,19 +245,6 @@ static void fsl_mc_get_root_dprc(struct device *dev, } } -/** - * fsl_mc_is_root_dprc - function to check if a given device is a root dprc - */ -static bool fsl_mc_is_root_dprc(struct device *dev) -{ - struct device *root_dprc_dev; - - fsl_mc_get_root_dprc(dev, _dprc_dev); - if (!root_dprc_dev) - return false; - return dev == root_dprc_dev; -} - static int get_dprc_attr(struct fsl_mc_io *mc_io, int container_id, struct dprc_attributes *attr) { @@ -424,6 +409,19 @@ error_cleanup_regions: } /** + * fsl_mc_is_root_dprc - function to check if a given device is a root dprc + */ +bool fsl_mc_is_root_dprc(struct device *dev) +{ + struct device *root_dprc_dev; + + fsl_mc_get_root_dprc(dev, _dprc_dev); + if (!root_dprc_dev) + return false; + return dev == root_dprc_dev; +} + +/** * Add a newly discovered MC object device to be visible in Linux */ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc, diff --git a/drivers/staging/fsl-mc/include/mc.h b/drivers/staging/fsl-mc/include/mc.h index a9a9d23..853cbf3 100644 --- a/drivers/staging/fsl-mc/include/mc.h +++ b/drivers/staging/fsl-mc/include/mc.h @@ -207,6 +207,8 @@ int __must_check fsl_mc_allocate_irqs(struct fsl_mc_device *mc_dev); void fsl_mc_free_irqs(struct fsl_mc_device *mc_dev); +bool fsl_mc_is_root_dprc(struct device *dev); + extern struct bus_type fsl_mc_bus_type; #endif /* _FSL_MC_H_ */ -- 1.9.0
[PATCH 02/11] staging: fsl-mc: implement uevent callback and set the modalias
Replace placeholder code in the uevent callback to properly set the MODALIAS env variable. Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/mc-bus.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c index d8776dd..cf92a1c 100644 --- a/drivers/staging/fsl-mc/bus/mc-bus.c +++ b/drivers/staging/fsl-mc/bus/mc-bus.c @@ -78,7 +78,13 @@ out: */ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env) { - pr_debug("%s invoked\n", __func__); + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev); + + if (add_uevent_var(env, "MODALIAS=fsl-mc:v%08Xd%s", + mc_dev->obj_desc.vendor, + mc_dev->obj_desc.type)) + return -ENOMEM; + return 0; } -- 1.9.0
[PATCH 05/11] staging: fsl-mc: export mc_get_version
some drivers (built as modules) rely on mc_get_version() Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dpmng.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/fsl-mc/bus/dpmng.c b/drivers/staging/fsl-mc/bus/dpmng.c index f633fcd..a31fa9b 100644 --- a/drivers/staging/fsl-mc/bus/dpmng.c +++ b/drivers/staging/fsl-mc/bus/dpmng.c @@ -67,6 +67,7 @@ int mc_get_version(struct fsl_mc_io *mc_io, return 0; } +EXPORT_SYMBOL(mc_get_version); /** * dpmng_get_container_id() - Get container ID associated with a given portal. -- 1.9.0
RE: [PATCH] of/irq: Fix msi-map calculation for nonzero rid-base
> -Original Message- > From: Mark Rutland [mailto:mark.rutl...@arm.com] > Sent: Tuesday, February 09, 2016 10:08 AM > To: Stuart Yoder <stuart.yo...@nxp.com> > Cc: Marc Zyngier <marc.zyng...@arm.com>; Robin Murphy <robin.mur...@arm.com>; > robh...@kernel.org; frowand.l...@gmail.com; grant.lik...@linaro.org; > devicet...@vger.kernel.org; david.da...@cavium.com; linux-arm- > ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > sta...@vger.kernel.org > Subject: Re: [PATCH] of/irq: Fix msi-map calculation for nonzero rid-base > > On Tue, Feb 09, 2016 at 03:56:55PM +, Stuart Yoder wrote: > > > > > -Original Message- > > > From: Marc Zyngier [mailto:marc.zyng...@arm.com] > > > Sent: Tuesday, February 09, 2016 6:06 AM > > > To: Robin Murphy <robin.mur...@arm.com>; robh...@kernel.org; > > > frowand.l...@gmail.com; > > > grant.lik...@linaro.org; devicet...@vger.kernel.org > > > Cc: mark.rutl...@arm.com; david.da...@cavium.com; Stuart Yoder > <stuart.yo...@nxp.com>; > > > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > > > sta...@vger.kernel.org > > > Subject: Re: [PATCH] of/irq: Fix msi-map calculation for nonzero rid-base > > > > > > Hi Robin, > > > > > > On 09/02/16 11:04, Robin Murphy wrote: > > > > The existing msi-map code is fine for shifting the entire RID space > > > > upwards, but attempting finer-grained remapping reveals a bug. It turns > > > > out that we are mistakenly treating the msi-base part as an offset, not > > > > as a new base to remap onto, so things get squiffy when rid-base is > > > > nonzero. Fix this, and at the same time add a sanity check against > > > > having msi-map-mask clash with a nonzero rid-base, as that's another > > > > thing one can easily get wrong. > > > > > > > > CC: <sta...@vger.kernel.org> > > > > Signed-off-by: Robin Murphy <robin.mur...@arm.com> > > > > > > Looks like Stuart and you both found the same bug at the same time: > > > https://lkml.org/lkml/2016/2/8/1066 > > > > > > but yours seem more correct to me (the rid_base masking in Stuart's > > > version seems odd). > > > > > > > --- > > > > drivers/of/irq.c | 9 - > > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/of/irq.c b/drivers/of/irq.c > > > > index 7ee21ae..e7bfc17 100644 > > > > --- a/drivers/of/irq.c > > > > +++ b/drivers/of/irq.c > > > > @@ -635,6 +635,13 @@ static u32 __of_msi_map_rid(struct device *dev, > > > > struct > > > device_node **np, > > > > msi_base = be32_to_cpup(msi_map + 2); > > > > rid_len = be32_to_cpup(msi_map + 3); > > > > > > > > + if (rid_base & ~map_mask) { > > > > + dev_err(parent_dev, > > > > + "Invalid msi-map translation - > > > > msi-map-mask (0x%x) ignores > rid- > > > base (0x%x)\n", > > > > + map_mask, rid_base); > > > > + return rid_out; > > > > + } > > > > + > > > > msi_controller_node = of_find_node_by_phandle(phandle); > > > > > > > > matched = (masked_rid >= rid_base && > > > > @@ -654,7 +661,7 @@ static u32 __of_msi_map_rid(struct device *dev, > > > > struct > device_node > > > **np, > > > > if (!matched) > > > > return rid_out; > > > > > > > > - rid_out = masked_rid + msi_base; > > > > + rid_out = masked_rid - rid_base + msi_base; > > > > dev_dbg(dev, > > > > "msi-map at: %s, using mask %08x, rid-base: %08x, > > > > msi-base: %08x, > length: > > > %08x, rid: %08x -> %08x\n", > > > > dev_name(parent_dev), map_mask, rid_base, msi_base, > > > > > > > > This computation: masked_rid - rid_base > > > > ...doesn't seem right to me. We are taking a rid that > > has been already masked and subtracting a rid base that has > > not been masked. > > The binding only mentions that the input RID is masked, not the base, so > that seems correct to me. > > >
RE: [PATCH] of/irq: Fix msi-map calculation for nonzero rid-base
> -Original Message- > From: Marc Zyngier [mailto:marc.zyng...@arm.com] > Sent: Tuesday, February 09, 2016 6:06 AM > To: Robin Murphy <robin.mur...@arm.com>; robh...@kernel.org; > frowand.l...@gmail.com; > grant.lik...@linaro.org; devicet...@vger.kernel.org > Cc: mark.rutl...@arm.com; david.da...@cavium.com; Stuart Yoder > <stuart.yo...@nxp.com>; > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; > sta...@vger.kernel.org > Subject: Re: [PATCH] of/irq: Fix msi-map calculation for nonzero rid-base > > Hi Robin, > > On 09/02/16 11:04, Robin Murphy wrote: > > The existing msi-map code is fine for shifting the entire RID space > > upwards, but attempting finer-grained remapping reveals a bug. It turns > > out that we are mistakenly treating the msi-base part as an offset, not > > as a new base to remap onto, so things get squiffy when rid-base is > > nonzero. Fix this, and at the same time add a sanity check against > > having msi-map-mask clash with a nonzero rid-base, as that's another > > thing one can easily get wrong. > > > > CC: <sta...@vger.kernel.org> > > Signed-off-by: Robin Murphy <robin.mur...@arm.com> > > Looks like Stuart and you both found the same bug at the same time: > https://lkml.org/lkml/2016/2/8/1066 > > but yours seem more correct to me (the rid_base masking in Stuart's > version seems odd). > > > --- > > drivers/of/irq.c | 9 - > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/of/irq.c b/drivers/of/irq.c > > index 7ee21ae..e7bfc17 100644 > > --- a/drivers/of/irq.c > > +++ b/drivers/of/irq.c > > @@ -635,6 +635,13 @@ static u32 __of_msi_map_rid(struct device *dev, struct > device_node **np, > > msi_base = be32_to_cpup(msi_map + 2); > > rid_len = be32_to_cpup(msi_map + 3); > > > > + if (rid_base & ~map_mask) { > > + dev_err(parent_dev, > > + "Invalid msi-map translation - msi-map-mask > > (0x%x) ignores rid- > base (0x%x)\n", > > + map_mask, rid_base); > > + return rid_out; > > + } > > + > > msi_controller_node = of_find_node_by_phandle(phandle); > > > > matched = (masked_rid >= rid_base && > > @@ -654,7 +661,7 @@ static u32 __of_msi_map_rid(struct device *dev, struct > > device_node > **np, > > if (!matched) > > return rid_out; > > > > - rid_out = masked_rid + msi_base; > > + rid_out = masked_rid - rid_base + msi_base; > > dev_dbg(dev, > > "msi-map at: %s, using mask %08x, rid-base: %08x, msi-base: > > %08x, length: > %08x, rid: %08x -> %08x\n", > > dev_name(parent_dev), map_mask, rid_base, msi_base, > > This computation: masked_rid - rid_base ...doesn't seem right to me. We are taking a rid that has been already masked and subtracting a rid base that has not been masked. I don't see how you can combine masked and unmasked values in the same calculation. Say I have this msi mapping: msi-map = <0x0100 0x11 0x1>; msi-map-mask = <0xff>; masked_rid = 0x0 rid_base = 0x0100 msi_base = 0x11 masked_rid - rid_base is 0x0 - 0x0100...which does not give the msi index/offset we want. Correct final answer should be 0x11. In my patch I masked the rid_base so it can be subtracted from the masked_rid. masked_rid_base = 0x00 msi_base + (masked_rid - masked_rid_base) = 0x11 Stuart
RE: [PATCH v3 7/8] staging: fsl-mc: update TODO and README for restool driver
> -Original Message- > From: Lijun Pan [mailto:lijun@freescale.com] > Sent: Monday, February 08, 2016 5:40 PM > To: gre...@linuxfoundation.org; a...@arndb.de; de...@driverdev.osuosl.org; > linux- > ker...@vger.kernel.org > Cc: stuart.yo...@freescale.com; itai.k...@freescale.com; > german.riv...@freescale.com; > le...@freescale.com; scottw...@freescale.com; ag...@suse.de; > bhamc...@freescale.com; > r89...@freescale.com; bhupesh.sha...@freescale.com; nir.e...@freescale.com; > richard.schm...@freescale.com; dan.carpen...@oracle.com; > lijun.pan2...@gmail.com; Lijun > Pan> Subject: [PATCH v3 7/8] staging: fsl-mc: update TODO and README for restool > driver > > Add more introduction of restool driver and state why > restool driver is needed in helping moving fsl-mc bus > out of staging tree. > > Signed-off-by: Lijun Pan > --- > drivers/staging/fsl-mc/README.txt | 11 ++- > drivers/staging/fsl-mc/TODO | 18 -- > 2 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/fsl-mc/README.txt > b/drivers/staging/fsl-mc/README.txt > index 8214102..e9ec507 100644 > --- a/drivers/staging/fsl-mc/README.txt > +++ b/drivers/staging/fsl-mc/README.txt > @@ -130,7 +130,16 @@ the objects involved in creating a network interfaces. > via a config file passed to the MC when firmware starts > it. There is also a Linux user space tool called "restool" > that can be used to create/destroy containers and objects > -dynamically. > +dynamically. The kernel side restool driver communicates with > +user space restool via ioctl. Restool relies on allocator driver > +to allocate dpmcp resources, enumerates fsl-mc bus to find root dprc > +objects of interest. When the user space restool program sends a request > +to restool driver to create a dp* objects in MC firmware, an interrupt > +will be triggered by MC firmware and the dprc driver's interrupt handler > +shall process the interrupt (synchronizing the objects in MC firmware and > +objects in Linux kernel). Though small, restool driver helps verify all > +the functionality of fsl-mc bus, dprc driver, allocator driver, > +and MC flib API. > > -DPAA2 Objects for an Ethernet Network Interface > > diff --git a/drivers/staging/fsl-mc/TODO b/drivers/staging/fsl-mc/TODO > index 5065821..4892eb6 100644 > --- a/drivers/staging/fsl-mc/TODO > +++ b/drivers/staging/fsl-mc/TODO > @@ -5,10 +5,24 @@ >fsl-mc bus out of staging. > > * Decide if multiple root fsl-mc buses will be supported per Linux instance, > - and if so add support for this. > + and if so add support for this. No matter fsl-mc bus support multiple root > + dprc or not, restool driver is designed to support multiple root if fsl-mc > + bus is ready some day later. If there is only one root dprc, restool driver > + works fine. > + > +* Add at least one driver utilizing fsl-mc bus. Restool driver is a very > + small and simple driver, which interacts with fsl-mc bus, dprc driver, > + allocator driver. Restool relies on allocator driver to allocate > + dpmcp resources, enumerates fsl-mc bus to find root dprc objects of > interest. > + When the user space restool program sends a request to restool driver to > + create a dp* objects in MC firmware, an interrupt will be triggered by > + MC firmware and the dprc driver's interrupt handler shall process the > + interrupt. Though small, restool driver helps verify all the functionality > + of fsl-mc bus, dprc driver, allocator driver, and MC flib API. Restool > + driver helps in moving fsl-mc bus out of staging branch. I don't agree with expanding the TODO list. The discussion with Alex Graf and Greg last year left us with the target of getting a object driver into staging that uses the fsl-mc bus in a normal way, which is what the current TODO list reflects. Now that MSI support is in we are closer to being able to do that. The restool driver is a special case that I think can wait. Stuart
RE: [PATCH] of/irq: Fix msi-map calculation for nonzero rid-base
> -Original Message- > From: Robin Murphy [mailto:robin.mur...@arm.com] > Sent: Tuesday, February 09, 2016 5:05 AM > To: robh...@kernel.org; frowand.l...@gmail.com; grant.lik...@linaro.org; > devicet...@vger.kernel.org > Cc: marc.zyng...@arm.com; mark.rutl...@arm.com; david.da...@cavium.com; > Stuart Yoder > <stuart.yo...@nxp.com>; linux-arm-ker...@lists.infradead.org; linux- > ker...@vger.kernel.org; sta...@vger.kernel.org > Subject: [PATCH] of/irq: Fix msi-map calculation for nonzero rid-base > > The existing msi-map code is fine for shifting the entire RID space > upwards, but attempting finer-grained remapping reveals a bug. It turns > out that we are mistakenly treating the msi-base part as an offset, not > as a new base to remap onto, so things get squiffy when rid-base is > nonzero. Fix this, and at the same time add a sanity check against > having msi-map-mask clash with a nonzero rid-base, as that's another > thing one can easily get wrong. > > CC: <sta...@vger.kernel.org> > Signed-off-by: Robin Murphy <robin.mur...@arm.com> > --- > drivers/of/irq.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/of/irq.c b/drivers/of/irq.c > index 7ee21ae..e7bfc17 100644 > --- a/drivers/of/irq.c > +++ b/drivers/of/irq.c > @@ -635,6 +635,13 @@ static u32 __of_msi_map_rid(struct device *dev, struct > device_node > **np, > msi_base = be32_to_cpup(msi_map + 2); > rid_len = be32_to_cpup(msi_map + 3); > > + if (rid_base & ~map_mask) { > + dev_err(parent_dev, > + "Invalid msi-map translation - msi-map-mask > (0x%x) ignores rid- > base (0x%x)\n", > + map_mask, rid_base); > + return rid_out; > + } > + > msi_controller_node = of_find_node_by_phandle(phandle); > > matched = (masked_rid >= rid_base && > @@ -654,7 +661,7 @@ static u32 __of_msi_map_rid(struct device *dev, struct > device_node > **np, > if (!matched) > return rid_out; > > - rid_out = masked_rid + msi_base; > + rid_out = masked_rid - rid_base + msi_base; > dev_dbg(dev, > "msi-map at: %s, using mask %08x, rid-base: %08x, msi-base: > %08x, length: > %08x, rid: %08x -> %08x\n", > dev_name(parent_dev), map_mask, rid_base, msi_base, > -- > 2.7.0.25.gfc10eb5.dirty Tested-by: Stuart Yoder <stuart.yo...@nxp.com>
[PATCH] of/irq: fix bug in computing output requester-id for an msi-map
From: Stuart Yoder <stuart.yo...@nxp.com> The binding for msi-map specifies that the output requester id should be computed as: (r - rid-base + msi-base) ...update the code accordingly. Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/of/irq.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 706e3ff..3f5e5fd 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -589,6 +589,7 @@ static u32 __of_msi_map_rid(struct device *dev, struct device_node **np, struct device_node *msi_controller_node; struct device_node *msi_np = *np; u32 map_mask, masked_rid, rid_base, msi_base, rid_len, phandle; + u32 masked_rid_base; int msi_map_len; bool matched; u32 rid_out = rid_in; @@ -654,7 +655,8 @@ static u32 __of_msi_map_rid(struct device *dev, struct device_node **np, if (!matched) return rid_out; - rid_out = masked_rid + msi_base; + masked_rid_base = map_mask & rid_base; + rid_out = msi_base + (masked_rid - masked_rid_base); dev_dbg(dev, "msi-map at: %s, using mask %08x, rid-base: %08x, msi-base: %08x, length: %08x, rid: %08x -> %08x\n", dev_name(parent_dev), map_mask, rid_base, msi_base, -- 1.7.9.5
RE: [PATCH 13/14] staging: fsl-mc: return -EINVAL for all fsl_mc_portal_allocate() failures
> -Original Message- > From: Stuart Yoder <stuart.yo...@nxp.com> > Date: Mon, Apr 11, 2016 at 11:56 AM > Subject: [PATCH 13/14] staging: fsl-mc: return -EINVAL for all > fsl_mc_portal_allocate() failures > To: gre...@linuxfoundation.org, german.riv...@nxp.com > Cc: de...@driverdev.osuosl.org, linux-kernel@vger.kernel.org, > ag...@suse.de, a...@arndb.de, leoyang...@nxp.com, Horia Geantă > <horia.gea...@nxp.com>, Stuart Yoder <stuart.yo...@nxp.com> > > > From: Horia Geantă <horia.gea...@nxp.com> > > There are some error paths that allow for a NULL new_mc_io and err = 0 > return code. Return -EINVAL instead. > > Signed-off-by: Horia Geantă <horia.gea...@nxp.com> > Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> > --- > drivers/staging/fsl-mc/bus/mc-allocator.c |1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c > b/drivers/staging/fsl-mc/bus/mc-allocator.c > index 4676ba1..7ee71e7 100644 > --- a/drivers/staging/fsl-mc/bus/mc-allocator.c > +++ b/drivers/staging/fsl-mc/bus/mc-allocator.c > @@ -306,6 +306,7 @@ int __must_check fsl_mc_portal_allocate(struct > fsl_mc_device *mc_dev, > if (error < 0) > return error; > > + error = -EINVAL; > dpmcp_dev = resource->data; > if (WARN_ON(!dpmcp_dev)) > goto error_cleanup_resource; > -- > 1.7.9.5 I had smtp mail server weirdness that led me to belive that the freescale.com email addresses in this patch were causing the patch to not show up on the mailing list, so I resent [PATCH 13/14] with the email address changed to nxp.com. Then both copies of [PATCH 13/14] showed up. So please ignore the duplicated email for this patch. Both copies are identical and either either freescale.com or nxp.com are ok for the email address. nxp.com version would be preferred I guess. Thanks, Stuart
[PATCH 02/14] staging: fsl-mc: DPAA2 overview readme update
From: Stuart Yoder <stuart.yo...@nxp.com> incorporated feedback from review comments, other misc cleanup/tweaks Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/README.txt | 138 + 1 file changed, 80 insertions(+), 58 deletions(-) diff --git a/drivers/staging/fsl-mc/README.txt b/drivers/staging/fsl-mc/README.txt index 8214102..179536a 100644 --- a/drivers/staging/fsl-mc/README.txt +++ b/drivers/staging/fsl-mc/README.txt @@ -11,11 +11,11 @@ Contents summary -Overview of DPAA2 objects -DPAA2 Linux driver architecture overview -bus driver --dprc driver +-DPRC driver -allocator --dpio driver +-DPIO driver -Ethernet --mac +-MAC DPAA2 Overview -- @@ -37,6 +37,9 @@ interfaces, an L2 switch, or accelerator instances. The MC provides memory-mapped I/O command interfaces (MC portals) which DPAA2 software drivers use to operate on DPAA2 objects: +The diagram below shows an overview of the DPAA2 resource management +architecture: + +--+ | OS | |DPAA2 drivers | @@ -77,13 +80,13 @@ DPIO objects. Overview of DPAA2 Objects - -The section provides a brief overview of some key objects -in the DPAA2 hardware. A simple scenario is described illustrating -the objects involved in creating a network interfaces. +The section provides a brief overview of some key DPAA2 objects. +A simple scenario is described illustrating the objects involved +in creating a network interfaces. -DPRC (Datapath Resource Container) -A DPRC is an container object that holds all the other +A DPRC is a container object that holds all the other types of DPAA2 objects. In the example diagram below there are 8 objects of 5 types (DPMCP, DPIO, DPBP, DPNI, and DPMAC) in the container. @@ -101,23 +104,23 @@ the objects involved in creating a network interfaces. | | +-+ -From the point of view of an OS, a DPRC is bus-like. Like -a plug-and-play bus, such as PCI, DPRC commands can be used to -enumerate the contents of the DPRC, discover the hardware -objects present (including mappable regions and interrupts). +From the point of view of an OS, a DPRC behaves similar to a plug and +play bus, like PCI. DPRC commands can be used to enumerate the contents +of the DPRC, discover the hardware objects present (including mappable +regions and interrupts). - dprc.1 (bus) + DPRC.1 (bus) | +--++---+---+---+ || | | | -dpmcp.1 dpio.1 dpbp.1 dpni.1 dpmac.1 -dpmcp.2 dpio.2 -dpmcp.3 +DPMCP.1 DPIO.1 DPBP.1 DPNI.1 DPMAC.1 +DPMCP.2 DPIO.2 +DPMCP.3 Hardware objects can be created and destroyed dynamically, providing the ability to hot plug/unplug objects in and out of the DPRC. -A DPRC has a mappable mmio region (an MC portal) that can be used +A DPRC has a mappable MMIO region (an MC portal) that can be used to send MC commands. It has an interrupt for status events (like hotplug). @@ -137,10 +140,11 @@ the objects involved in creating a network interfaces. A typical Ethernet NIC is monolithic-- the NIC device contains TX/RX queuing mechanisms, configuration mechanisms, buffer management, physical ports, and interrupts. DPAA2 uses a more granular approach -utilizing multiple hardware objects. Each object has specialized -functions, and are used together by software to provide Ethernet network -interface functionality. This approach provides efficient use of finite -hardware resources, flexibility, and performance advantages. +utilizing multiple hardware objects. Each object provides specialized +functions. Groups of these objects are used by software to provide +Ethernet network interface functionality. This approach provides +efficient use of finite hardware resources, flexibility, and +performance advantages. The diagram below shows the objects needed for a simple network interface configuration on a system with 2 CPUs. @@ -168,46 +172,52 @@ the objects involved in creating a network interfaces. Below the objects are described. For each object a brief description is provided along with a summary of the kinds of operations the object -supports and a summary of key resources of the object (mmio regions -and irqs). +supports and a summary of key resources of the object (MMIO regions +and IRQs). -DPMAC (Datapath Ethernet MAC): represents an Ethernet MAC, a hardware device t
[PATCH 05/14] staging: fsl-mc: update dprc binary interface to v5.1
From: Stuart Yoder <stuart.yo...@nxp.com> The meaning of the "status" parameter in dprc_get_irq_status has changed, and this patch updates the flib and caller of the API. Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dprc-cmd.h|4 ++-- drivers/staging/fsl-mc/bus/dprc-driver.c |1 + drivers/staging/fsl-mc/bus/dprc.c| 26 +- drivers/staging/fsl-mc/bus/mc-msi.c |2 +- drivers/staging/fsl-mc/include/dprc.h| 19 --- 5 files changed, 29 insertions(+), 23 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dprc-cmd.h b/drivers/staging/fsl-mc/bus/dprc-cmd.h index 6552c20..d0198f5 100644 --- a/drivers/staging/fsl-mc/bus/dprc-cmd.h +++ b/drivers/staging/fsl-mc/bus/dprc-cmd.h @@ -41,8 +41,8 @@ #define _FSL_DPRC_CMD_H /* DPRC Version */ -#define DPRC_VER_MAJOR 4 -#define DPRC_VER_MINOR 0 +#define DPRC_VER_MAJOR 5 +#define DPRC_VER_MINOR 1 /* Command IDs */ #define DPRC_CMDID_CLOSE 0x800 diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c index 31488a7..4334f3c 100644 --- a/drivers/staging/fsl-mc/bus/dprc-driver.c +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c @@ -423,6 +423,7 @@ static irqreturn_t dprc_irq0_handler_thread(int irq_num, void *arg) if (WARN_ON(!msi_desc || msi_desc->irq != (u32)irq_num)) goto out; + status = 0; error = dprc_get_irq_status(mc_io, 0, mc_dev->mc_handle, 0, ); if (error < 0) { diff --git a/drivers/staging/fsl-mc/bus/dprc.c b/drivers/staging/fsl-mc/bus/dprc.c index 381b9a9..a2c4737 100644 --- a/drivers/staging/fsl-mc/bus/dprc.c +++ b/drivers/staging/fsl-mc/bus/dprc.c @@ -265,7 +265,7 @@ int dprc_get_irq(struct fsl_mc_io *mc_io, /* retrieve response parameters */ irq_cfg->val = mc_dec(cmd.params[0], 0, 32); irq_cfg->paddr = mc_dec(cmd.params[1], 0, 64); - irq_cfg->user_irq_id = mc_dec(cmd.params[2], 0, 32); + irq_cfg->irq_num = mc_dec(cmd.params[2], 0, 32); *type = mc_dec(cmd.params[2], 32, 32); return 0; @@ -296,7 +296,7 @@ int dprc_set_irq(struct fsl_mc_io *mc_io, cmd.params[0] |= mc_enc(32, 8, irq_index); cmd.params[0] |= mc_enc(0, 32, irq_cfg->val); cmd.params[1] |= mc_enc(0, 64, irq_cfg->paddr); - cmd.params[2] |= mc_enc(0, 32, irq_cfg->user_irq_id); + cmd.params[2] |= mc_enc(0, 32, irq_cfg->irq_num); /* send command to mc*/ return mc_send_command(mc_io, ); @@ -466,6 +466,7 @@ int dprc_get_irq_status(struct fsl_mc_io *mc_io, /* prepare command */ cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_IRQ_STATUS, cmd_flags, token); + cmd.params[0] |= mc_enc(0, 32, *status); cmd.params[0] |= mc_enc(32, 8, irq_index); /* send command to mc*/ @@ -948,6 +949,7 @@ int dprc_get_obj(struct fsl_mc_io *mc_io, obj_desc->state = mc_dec(cmd.params[1], 32, 32); obj_desc->ver_major = mc_dec(cmd.params[2], 0, 16); obj_desc->ver_minor = mc_dec(cmd.params[2], 16, 16); + obj_desc->flags = mc_dec(cmd.params[2], 32, 16); obj_desc->type[0] = mc_dec(cmd.params[3], 0, 8); obj_desc->type[1] = mc_dec(cmd.params[3], 8, 8); obj_desc->type[2] = mc_dec(cmd.params[3], 16, 8); @@ -1042,6 +1044,7 @@ int dprc_get_obj_desc(struct fsl_mc_io *mc_io, obj_desc->state = (u32)mc_dec(cmd.params[1], 32, 32); obj_desc->ver_major = (u16)mc_dec(cmd.params[2], 0, 16); obj_desc->ver_minor = (u16)mc_dec(cmd.params[2], 16, 16); + obj_desc->flags = mc_dec(cmd.params[2], 32, 16); obj_desc->type[0] = (char)mc_dec(cmd.params[3], 0, 8); obj_desc->type[1] = (char)mc_dec(cmd.params[3], 8, 8); obj_desc->type[2] = (char)mc_dec(cmd.params[3], 16, 8); @@ -1108,7 +,7 @@ int dprc_set_obj_irq(struct fsl_mc_io *mc_io, cmd.params[0] |= mc_enc(32, 8, irq_index); cmd.params[0] |= mc_enc(0, 32, irq_cfg->val); cmd.params[1] |= mc_enc(0, 64, irq_cfg->paddr); - cmd.params[2] |= mc_enc(0, 32, irq_cfg->user_irq_id); + cmd.params[2] |= mc_enc(0, 32, irq_cfg->irq_num); cmd.params[2] |= mc_enc(32, 32, obj_id); cmd.params[3] |= mc_enc(0, 8, obj_type[0]); cmd.params[3] |= mc_enc(8, 8, obj_type[1]); @@ -1189,7 +1192,7 @@ int dprc_get_obj_irq(struct fsl_mc_io *mc_io, /* retrieve response parameters */ irq_cfg->val = (u32)mc_dec(cmd.params[0], 0, 32); irq_cfg->paddr = (u64)mc_dec(cmd.params[1], 0, 64); - irq_cfg->user_irq_id = (int)mc_dec(cmd.params[2], 0, 32); + irq_cfg->irq_num = (int)
[PATCH 13/14] staging: fsl-mc: return -EINVAL for all fsl_mc_portal_allocate() failures
From: Horia Geantă <horia.gea...@freescale.com> There are some error paths that allow for a NULL new_mc_io and err = 0 return code. Return -EINVAL instead. Signed-off-by: Horia Geantă <horia.gea...@freescale.com> Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/mc-allocator.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c b/drivers/staging/fsl-mc/bus/mc-allocator.c index 4676ba1..7ee71e7 100644 --- a/drivers/staging/fsl-mc/bus/mc-allocator.c +++ b/drivers/staging/fsl-mc/bus/mc-allocator.c @@ -306,6 +306,7 @@ int __must_check fsl_mc_portal_allocate(struct fsl_mc_device *mc_dev, if (error < 0) return error; + error = -EINVAL; dpmcp_dev = resource->data; if (WARN_ON(!dpmcp_dev)) goto error_cleanup_resource; -- 1.7.9.5
[PATCH 11/14] staging: fsl-mc: add quirk handling for dpseci objects < 4.0
From: Horia Geanta <horia.gea...@nxp.com> dpseci objects < 4.0 are not coherent-- in spite of the fact that the MC reports them to be coherent in certain versions. Add a special case to set the no shareability flag for dpseci objects < 4.0. Signed-off-by: Horia Geanta <horia.gea...@nxp.com> (Stuart: reworded commit message, updated comment in patch) Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dprc-driver.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c index 53c6e98..7fc4717 100644 --- a/drivers/staging/fsl-mc/bus/dprc-driver.c +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c @@ -312,6 +312,15 @@ int dprc_scan_objects(struct fsl_mc_device *mc_bus_dev, continue; } + /* +* add a quirk for all versions of dpsec < 4.0...none +* are coherent regardless of what the MC reports. +*/ + if ((strcmp(obj_desc->type, "dpseci") == 0) && + (obj_desc->ver_major < 4)) + obj_desc->flags |= + DPRC_OBJ_FLAG_NO_MEM_SHAREABILITY; + irq_count += obj_desc->irq_count; dev_dbg(_bus_dev->dev, "Discovered object: type %s, id %d\n", -- 1.7.9.5
[PATCH 04/14] staging: fsl-mc: update dpbp binary interface to v2.2
From: Stuart Yoder <stuart.yo...@nxp.com> Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dpbp.c | 77 - drivers/staging/fsl-mc/include/dpbp-cmd.h |4 +- drivers/staging/fsl-mc/include/dpbp.h | 51 ++- 3 files changed, 127 insertions(+), 5 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dpbp.c b/drivers/staging/fsl-mc/bus/dpbp.c index 2d97173..c31fe1b 100644 --- a/drivers/staging/fsl-mc/bus/dpbp.c +++ b/drivers/staging/fsl-mc/bus/dpbp.c @@ -293,7 +293,7 @@ int dpbp_set_irq(struct fsl_mc_io *mc_io, cmd.params[0] |= mc_enc(0, 8, irq_index); cmd.params[0] |= mc_enc(32, 32, irq_cfg->val); cmd.params[1] |= mc_enc(0, 64, irq_cfg->addr); - cmd.params[2] |= mc_enc(0, 32, irq_cfg->user_irq_id); + cmd.params[2] |= mc_enc(0, 32, irq_cfg->irq_num); /* send command to mc*/ return mc_send_command(mc_io, ); @@ -334,7 +334,7 @@ int dpbp_get_irq(struct fsl_mc_io *mc_io, /* retrieve response parameters */ irq_cfg->val = (u32)mc_dec(cmd.params[0], 0, 32); irq_cfg->addr = (u64)mc_dec(cmd.params[1], 0, 64); - irq_cfg->user_irq_id = (int)mc_dec(cmd.params[2], 0, 32); + irq_cfg->irq_num = (int)mc_dec(cmd.params[2], 0, 32); *type = (int)mc_dec(cmd.params[2], 32, 32); return 0; } @@ -502,6 +502,7 @@ int dpbp_get_irq_status(struct fsl_mc_io *mc_io, /* prepare command */ cmd.header = mc_encode_cmd_header(DPBP_CMDID_GET_IRQ_STATUS, cmd_flags, token); + cmd.params[0] |= mc_enc(0, 32, *status); cmd.params[0] |= mc_enc(32, 8, irq_index); /* send command to mc*/ @@ -580,3 +581,75 @@ int dpbp_get_attributes(struct fsl_mc_io *mc_io, return 0; } EXPORT_SYMBOL(dpbp_get_attributes); + +/** + * dpbp_set_notifications() - Set notifications towards software + * @mc_io: Pointer to MC portal's I/O object + * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_' + * @token: Token of DPBP object + * @cfg: notifications configuration + * + * Return: '0' on Success; Error code otherwise. + */ +int dpbp_set_notifications(struct fsl_mc_io *mc_io, + u32 cmd_flags, + u16 token, + struct dpbp_notification_cfg *cfg) +{ + struct mc_command cmd = { 0 }; + + /* prepare command */ + cmd.header = mc_encode_cmd_header(DPBP_CMDID_SET_NOTIFICATIONS, + cmd_flags, + token); + + cmd.params[0] |= mc_enc(0, 32, cfg->depletion_entry); + cmd.params[0] |= mc_enc(32, 32, cfg->depletion_exit); + cmd.params[1] |= mc_enc(0, 32, cfg->surplus_entry); + cmd.params[1] |= mc_enc(32, 32, cfg->surplus_exit); + cmd.params[2] |= mc_enc(0, 16, cfg->options); + cmd.params[3] |= mc_enc(0, 64, cfg->message_ctx); + cmd.params[4] |= mc_enc(0, 64, cfg->message_iova); + + /* send command to mc*/ + return mc_send_command(mc_io, ); +} + +/** + * dpbp_get_notifications() - Get the notifications configuration + * @mc_io: Pointer to MC portal's I/O object + * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_' + * @token: Token of DPBP object + * @cfg: notifications configuration + * + * Return: '0' on Success; Error code otherwise. + */ +int dpbp_get_notifications(struct fsl_mc_io *mc_io, + u32 cmd_flags, + u16 token, + struct dpbp_notification_cfg *cfg) +{ + struct mc_command cmd = { 0 }; + int err; + + /* prepare command */ + cmd.header = mc_encode_cmd_header(DPBP_CMDID_GET_NOTIFICATIONS, + cmd_flags, + token); + + /* send command to mc*/ + err = mc_send_command(mc_io, ); + if (err) + return err; + + /* retrieve response parameters */ + cfg->depletion_entry = (u32)mc_dec(cmd.params[0], 0, 32); + cfg->depletion_exit = (u32)mc_dec(cmd.params[0], 32, 32); + cfg->surplus_entry = (u32)mc_dec(cmd.params[1], 0, 32); + cfg->surplus_exit = (u32)mc_dec(cmd.params[1], 32, 32); + cfg->options = (u16)mc_dec(cmd.params[2], 0, 16); + cfg->message_ctx = (u64)mc_dec(cmd.params[3], 0, 64); + cfg->message_iova = (u64)mc_dec(cmd.params[4], 0, 64); + + return 0; +} diff --git a/drivers/staging/fsl-mc/include/dpbp-cmd.h b/drivers/staging/fsl-mc/include/dpbp-cmd.h index efa9bf3..c57b454 100644 --- a/drivers/staging/fsl-mc/include/dpbp-cmd.h +++ b/drivers/staging/fsl-mc/include/dpbp-cmd.h @@ -34,7 +34,7 @@ /* DPBP Version */ #define DPBP_VER_MAJOR 2 -#define DPBP_VER_MINOR
[PATCH 06/14] staging: fsl-mc: don't use object versions to make binding decisions
From: Itai Katz <itai.k...@nxp.com> Up until now if the object version expected by a driver (in the API header file) did not match the actual object version in the MC hardware the bus driver refused to bind the object to the driver or printed out WARN_ON dumps. This patch removes those checks, and the responsibility of object version checking should now be done in the object drivers themselves. If the actual version discovered is not supported, the driver's probe function should fail. Drivers should use version checks to support new features and provide backwards compatibility if at all possible. This patch also removes the checks that caused bus driver probing to fail if the overall MC version discovered did not match the firmware version from the API header...this was too strict. The overall MC version is informational like a release number, and continues to be printed in the boot log. Signed-off-by: Itai Katz <itai.k...@nxp.com> (Stuart: reworded commit log) Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dprc-driver.c |4 +-- drivers/staging/fsl-mc/bus/mc-allocator.c |6 - drivers/staging/fsl-mc/bus/mc-bus.c | 38 + 3 files changed, 2 insertions(+), 46 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c index 4334f3c..2d88c10 100644 --- a/drivers/staging/fsl-mc/bus/dprc-driver.c +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c @@ -780,9 +780,7 @@ static int dprc_remove(struct fsl_mc_device *mc_dev) static const struct fsl_mc_device_match_id match_id_table[] = { { .vendor = FSL_MC_VENDOR_FREESCALE, -.obj_type = "dprc", -.ver_major = DPRC_VER_MAJOR, -.ver_minor = DPRC_VER_MINOR}, +.obj_type = "dprc"}, {.vendor = 0x0}, }; diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c b/drivers/staging/fsl-mc/bus/mc-allocator.c index 86f8543..52b16f7 100644 --- a/drivers/staging/fsl-mc/bus/mc-allocator.c +++ b/drivers/staging/fsl-mc/bus/mc-allocator.c @@ -722,20 +722,14 @@ static const struct fsl_mc_device_match_id match_id_table[] = { { .vendor = FSL_MC_VENDOR_FREESCALE, .obj_type = "dpbp", -.ver_major = DPBP_VER_MAJOR, -.ver_minor = DPBP_VER_MINOR }, { .vendor = FSL_MC_VENDOR_FREESCALE, .obj_type = "dpmcp", -.ver_major = DPMCP_VER_MAJOR, -.ver_minor = DPMCP_VER_MINOR }, { .vendor = FSL_MC_VENDOR_FREESCALE, .obj_type = "dpcon", -.ver_major = DPCON_VER_MAJOR, -.ver_minor = DPCON_VER_MINOR }, {.vendor = 0x0}, }; diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c index b594556..981e4c2 100644 --- a/drivers/staging/fsl-mc/bus/mc-bus.c +++ b/drivers/staging/fsl-mc/bus/mc-bus.c @@ -40,8 +40,6 @@ static int fsl_mc_bus_match(struct device *dev, struct device_driver *drv) struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev); struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(drv); bool found = false; - bool major_version_mismatch = false; - bool minor_version_mismatch = false; if (WARN_ON(!fsl_mc_bus_exists())) goto out; @@ -64,32 +62,12 @@ static int fsl_mc_bus_match(struct device *dev, struct device_driver *drv) for (id = mc_drv->match_id_table; id->vendor != 0x0; id++) { if (id->vendor == mc_dev->obj_desc.vendor && strcmp(id->obj_type, mc_dev->obj_desc.type) == 0) { - if (id->ver_major == mc_dev->obj_desc.ver_major) { - found = true; - if (id->ver_minor != mc_dev->obj_desc.ver_minor) - minor_version_mismatch = true; - } else { - major_version_mismatch = true; - } + found = true; break; } } - if (major_version_mismatch) { - dev_warn(dev, -"Major version mismatch: driver version %u.%u, MC object version %u.%u\n", -id->ver_major, id->ver_minor, -mc_dev->obj_desc.ver_major, -mc_dev->obj_desc.ver_minor); - } else if (minor_version_mismatch) { - dev_warn(dev, -"Minor version mismatch: driver version %u.%u, MC object version %u.%u\n", -id->ver_major, id->ver_minor, -mc_dev->obj_desc.ver_major, -mc_dev->obj_desc.ver_minor); - } - out: dev_dbg(dev, "%smatched
[PATCH 08/14] staging: fsl-mc: set cacheable flag for added devices if applicable
From: Itai Katz <itai.k...@nxp.com> Some DPAA2 devices have mmio regions that should be mapped as cacheable by drivers. Set IORESOURCE_CACHEABLE in the region's flags if applicable. Signed-off-by: Itai Katz <itai.k...@nxp.com> [Stuart: update subject and commit message] Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/mc-bus.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c index 8bf76d7..2075597 100644 --- a/drivers/staging/fsl-mc/bus/mc-bus.c +++ b/drivers/staging/fsl-mc/bus/mc-bus.c @@ -354,6 +354,8 @@ static int fsl_mc_device_get_mmio_regions(struct fsl_mc_device *mc_dev, regions[i].end = regions[i].start + region_desc.size - 1; regions[i].name = "fsl-mc object MMIO region"; regions[i].flags = IORESOURCE_IO; + if (region_desc.flags & DPRC_REGION_CACHEABLE) + regions[i].flags |= IORESOURCE_CACHEABLE; } mc_dev->regions = regions; -- 1.7.9.5
[PATCH 09/14] staging: fsl-mc: get version of root dprc from MC hardware
From: Itai Katz <itai.k...@nxp.com> The root dprc is discovered as a platform device in the device tree. The version of that dprc was previously set using hardcoded values from the API header in the kernel). This patch removes the use of the hardcoded version numbers and instead reads the actual dprc version from the hardware. Signed-off-by: Itai Katz <itai.k...@nxp.com> (Stuart: resolved merge conflict, updated commit subject/log) Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/mc-bus.c | 45 --- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c index 2075597..b4c3c5e 100644 --- a/drivers/staging/fsl-mc/bus/mc-bus.c +++ b/drivers/staging/fsl-mc/bus/mc-bus.c @@ -229,11 +229,10 @@ static bool fsl_mc_is_root_dprc(struct device *dev) return dev == root_dprc_dev; } -static int get_dprc_icid(struct fsl_mc_io *mc_io, -int container_id, u16 *icid) +static int get_dprc_attr(struct fsl_mc_io *mc_io, +int container_id, struct dprc_attributes *attr) { u16 dprc_handle; - struct dprc_attributes attr; int error; error = dprc_open(mc_io, 0, container_id, _handle); @@ -242,15 +241,14 @@ static int get_dprc_icid(struct fsl_mc_io *mc_io, return error; } - memset(, 0, sizeof(attr)); - error = dprc_get_attributes(mc_io, 0, dprc_handle, ); + memset(attr, 0, sizeof(struct dprc_attributes)); + error = dprc_get_attributes(mc_io, 0, dprc_handle, attr); if (error < 0) { dev_err(mc_io->dev, "dprc_get_attributes() failed: %d\n", error); goto common_cleanup; } - *icid = attr.icid; error = 0; common_cleanup: @@ -258,6 +256,34 @@ common_cleanup: return error; } +static int get_dprc_icid(struct fsl_mc_io *mc_io, +int container_id, u16 *icid) +{ + struct dprc_attributes attr; + int error; + + error = get_dprc_attr(mc_io, container_id, ); + if (error == 0) + *icid = attr.icid; + + return error; +} + +static int get_dprc_version(struct fsl_mc_io *mc_io, + int container_id, u16 *major, u16 *minor) +{ + struct dprc_attributes attr; + int error; + + error = get_dprc_attr(mc_io, container_id, ); + if (error == 0) { + *major = attr.version.major; + *minor = attr.version.minor; + } + + return error; +} + static int translate_mc_addr(struct fsl_mc_device *mc_dev, enum dprc_region_type mc_region_type, u64 mc_offset, phys_addr_t *phys_addr) @@ -719,11 +745,14 @@ static int fsl_mc_bus_probe(struct platform_device *pdev) goto error_cleanup_mc_io; } + error = get_dprc_version(mc_io, container_id, +_desc.ver_major, _desc.ver_minor); + if (error < 0) + goto error_cleanup_mc_io; + obj_desc.vendor = FSL_MC_VENDOR_FREESCALE; strcpy(obj_desc.type, "dprc"); obj_desc.id = container_id; - obj_desc.ver_major = DPRC_VER_MAJOR; - obj_desc.ver_minor = DPRC_VER_MINOR; obj_desc.irq_count = 1; obj_desc.region_count = 0; -- 1.7.9.5
[PATCH 12/14] staging: fsl-mc: add dpmcp version check
From: Itai Katz <itai.k...@nxp.com> The dpmcp driver supports dpmcp version 3.0 and above. This patch adds the code to check the version. Signed-off-by: Itai Katz <itai.k...@nxp.com> Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dpmcp-cmd.h|6 +++--- drivers/staging/fsl-mc/bus/mc-allocator.c | 11 +++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dpmcp-cmd.h b/drivers/staging/fsl-mc/bus/dpmcp-cmd.h index c6f4ec0..c9b52dd 100644 --- a/drivers/staging/fsl-mc/bus/dpmcp-cmd.h +++ b/drivers/staging/fsl-mc/bus/dpmcp-cmd.h @@ -32,9 +32,9 @@ #ifndef _FSL_DPMCP_CMD_H #define _FSL_DPMCP_CMD_H -/* DPMCP Version */ -#define DPMCP_VER_MAJOR3 -#define DPMCP_VER_MINOR0 +/* Minimal supported DPMCP Version */ +#define DPMCP_MIN_VER_MAJOR3 +#define DPMCP_MIN_VER_MINOR0 /* Command IDs */ #define DPMCP_CMDID_CLOSE 0x800 diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c b/drivers/staging/fsl-mc/bus/mc-allocator.c index 52b16f7..4676ba1 100644 --- a/drivers/staging/fsl-mc/bus/mc-allocator.c +++ b/drivers/staging/fsl-mc/bus/mc-allocator.c @@ -310,6 +310,17 @@ int __must_check fsl_mc_portal_allocate(struct fsl_mc_device *mc_dev, if (WARN_ON(!dpmcp_dev)) goto error_cleanup_resource; + if (dpmcp_dev->obj_desc.ver_major < DPMCP_MIN_VER_MAJOR || + (dpmcp_dev->obj_desc.ver_major == DPMCP_MIN_VER_MAJOR && +dpmcp_dev->obj_desc.ver_minor < DPMCP_MIN_VER_MINOR)) { + dev_err(_dev->dev, + "ERROR: Version %d.%d of DPMCP not supported.\n", + dpmcp_dev->obj_desc.ver_major, + dpmcp_dev->obj_desc.ver_minor); + error = -ENOTSUPP; + goto error_cleanup_resource; + } + if (WARN_ON(dpmcp_dev->obj_desc.region_count == 0)) goto error_cleanup_resource; -- 1.7.9.5
[PATCH 10/14] staging: fsl-mc: add dprc version check
From: Itai Katz <itai.k...@nxp.com> The dprc driver supports dprc version 5.0 and above. This patch adds the code to check the version. Signed-off-by: Itai Katz <itai.k...@nxp.com> (Stuart: resolved merge conflicts, split dpseci quirk into separate patch) Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dprc-cmd.h |6 +++--- drivers/staging/fsl-mc/bus/dprc-driver.c| 19 +++ drivers/staging/fsl-mc/bus/mc-bus.c |1 + drivers/staging/fsl-mc/include/mc-private.h |2 ++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dprc-cmd.h b/drivers/staging/fsl-mc/bus/dprc-cmd.h index d0198f5..9b854fa 100644 --- a/drivers/staging/fsl-mc/bus/dprc-cmd.h +++ b/drivers/staging/fsl-mc/bus/dprc-cmd.h @@ -40,9 +40,9 @@ #ifndef _FSL_DPRC_CMD_H #define _FSL_DPRC_CMD_H -/* DPRC Version */ -#define DPRC_VER_MAJOR 5 -#define DPRC_VER_MINOR 1 +/* Minimal supported DPRC Version */ +#define DPRC_MIN_VER_MAJOR 5 +#define DPRC_MIN_VER_MINOR 0 /* Command IDs */ #define DPRC_CMDID_CLOSE 0x800 diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c index 2d88c10..53c6e98 100644 --- a/drivers/staging/fsl-mc/bus/dprc-driver.c +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c @@ -693,6 +693,25 @@ static int dprc_probe(struct fsl_mc_device *mc_dev) goto error_cleanup_msi_domain; } + error = dprc_get_attributes(mc_dev->mc_io, 0, mc_dev->mc_handle, + _bus->dprc_attr); + if (error < 0) { + dev_err(_dev->dev, "dprc_get_attributes() failed: %d\n", + error); + goto error_cleanup_open; + } + + if (mc_bus->dprc_attr.version.major < DPRC_MIN_VER_MAJOR || + (mc_bus->dprc_attr.version.major == DPRC_MIN_VER_MAJOR && + mc_bus->dprc_attr.version.minor < DPRC_MIN_VER_MINOR)) { + dev_err(_dev->dev, + "ERROR: DPRC version %d.%d not supported\n", + mc_bus->dprc_attr.version.major, + mc_bus->dprc_attr.version.minor); + error = -ENOTSUPP; + goto error_cleanup_open; + } + mutex_init(_bus->scan_mutex); /* diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c index b4c3c5e..4053643 100644 --- a/drivers/staging/fsl-mc/bus/mc-bus.c +++ b/drivers/staging/fsl-mc/bus/mc-bus.c @@ -745,6 +745,7 @@ static int fsl_mc_bus_probe(struct platform_device *pdev) goto error_cleanup_mc_io; } + memset(_desc, 0, sizeof(struct dprc_obj_desc)); error = get_dprc_version(mc_io, container_id, _desc.ver_major, _desc.ver_minor); if (error < 0) diff --git a/drivers/staging/fsl-mc/include/mc-private.h b/drivers/staging/fsl-mc/include/mc-private.h index ee5f1d2..cab1ae9 100644 --- a/drivers/staging/fsl-mc/include/mc-private.h +++ b/drivers/staging/fsl-mc/include/mc-private.h @@ -94,12 +94,14 @@ struct fsl_mc_resource_pool { * from the physical DPRC. * @irq_resources: Pointer to array of IRQ objects for the IRQ pool * @scan_mutex: Serializes bus scanning + * @dprc_attr: DPRC attributes */ struct fsl_mc_bus { struct fsl_mc_device mc_dev; struct fsl_mc_resource_pool resource_pools[FSL_MC_NUM_POOL_TYPES]; struct fsl_mc_device_irq *irq_resources; struct mutex scan_mutex;/* serializes bus scanning */ + struct dprc_attributes dprc_attr; }; #define to_fsl_mc_bus(_mc_dev) \ -- 1.7.9.5
[PATCH 03/14] staging: fsl-mc: update dpmcp binary interface to v3.0
From: Stuart Yoder <stuart.yo...@nxp.com> Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/dpmcp-cmd.h |5 ++--- drivers/staging/fsl-mc/bus/dpmcp.c | 35 ++-- drivers/staging/fsl-mc/bus/dpmcp.h | 10 ++--- 3 files changed, 6 insertions(+), 44 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dpmcp-cmd.h b/drivers/staging/fsl-mc/bus/dpmcp-cmd.h index a87e9f8..c6f4ec0 100644 --- a/drivers/staging/fsl-mc/bus/dpmcp-cmd.h +++ b/drivers/staging/fsl-mc/bus/dpmcp-cmd.h @@ -33,8 +33,8 @@ #define _FSL_DPMCP_CMD_H /* DPMCP Version */ -#define DPMCP_VER_MAJOR2 -#define DPMCP_VER_MINOR1 +#define DPMCP_VER_MAJOR3 +#define DPMCP_VER_MINOR0 /* Command IDs */ #define DPMCP_CMDID_CLOSE 0x800 @@ -52,6 +52,5 @@ #define DPMCP_CMDID_SET_IRQ_MASK 0x014 #define DPMCP_CMDID_GET_IRQ_MASK 0x015 #define DPMCP_CMDID_GET_IRQ_STATUS 0x016 -#define DPMCP_CMDID_CLEAR_IRQ_STATUS 0x017 #endif /* _FSL_DPMCP_CMD_H */ diff --git a/drivers/staging/fsl-mc/bus/dpmcp.c b/drivers/staging/fsl-mc/bus/dpmcp.c index b0248f5..fd6dd4e 100644 --- a/drivers/staging/fsl-mc/bus/dpmcp.c +++ b/drivers/staging/fsl-mc/bus/dpmcp.c @@ -213,7 +213,7 @@ int dpmcp_set_irq(struct fsl_mc_io *mc_io, cmd.params[0] |= mc_enc(0, 8, irq_index); cmd.params[0] |= mc_enc(32, 32, irq_cfg->val); cmd.params[1] |= mc_enc(0, 64, irq_cfg->paddr); - cmd.params[2] |= mc_enc(0, 32, irq_cfg->user_irq_id); + cmd.params[2] |= mc_enc(0, 32, irq_cfg->irq_num); /* send command to mc*/ return mc_send_command(mc_io, ); @@ -254,7 +254,7 @@ int dpmcp_get_irq(struct fsl_mc_io *mc_io, /* retrieve response parameters */ irq_cfg->val = (u32)mc_dec(cmd.params[0], 0, 32); irq_cfg->paddr = (u64)mc_dec(cmd.params[1], 0, 64); - irq_cfg->user_irq_id = (int)mc_dec(cmd.params[2], 0, 32); + irq_cfg->irq_num = (int)mc_dec(cmd.params[2], 0, 32); *type = (int)mc_dec(cmd.params[2], 32, 32); return 0; } @@ -435,37 +435,6 @@ int dpmcp_get_irq_status(struct fsl_mc_io *mc_io, } /** - * dpmcp_clear_irq_status() - Clear a pending interrupt's status - * - * @mc_io: Pointer to MC portal's I/O object - * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_' - * @token: Token of DPMCP object - * @irq_index: The interrupt index to configure - * @status:Bits to clear (W1C) - one bit per cause: - * 0 = don't change - * 1 = clear status bit - * - * Return: '0' on Success; Error code otherwise. - */ -int dpmcp_clear_irq_status(struct fsl_mc_io *mc_io, - u32 cmd_flags, - u16 token, - u8 irq_index, - u32 status) -{ - struct mc_command cmd = { 0 }; - - /* prepare command */ - cmd.header = mc_encode_cmd_header(DPMCP_CMDID_CLEAR_IRQ_STATUS, - cmd_flags, token); - cmd.params[0] |= mc_enc(0, 32, status); - cmd.params[0] |= mc_enc(32, 8, irq_index); - - /* send command to mc*/ - return mc_send_command(mc_io, ); -} - -/** * dpmcp_get_attributes - Retrieve DPMCP attributes. * * @mc_io: Pointer to MC portal's I/O object diff --git a/drivers/staging/fsl-mc/bus/dpmcp.h b/drivers/staging/fsl-mc/bus/dpmcp.h index 6df351f..fe79d4d 100644 --- a/drivers/staging/fsl-mc/bus/dpmcp.h +++ b/drivers/staging/fsl-mc/bus/dpmcp.h @@ -82,12 +82,12 @@ int dpmcp_reset(struct fsl_mc_io *mc_io, * struct dpmcp_irq_cfg - IRQ configuration * @paddr: Address that must be written to signal a message-based interrupt * @val: Value to write into irq_addr address - * @user_irq_id: A user defined number associated with this IRQ + * @irq_num: A user defined number associated with this IRQ */ struct dpmcp_irq_cfg { uint64_t paddr; uint32_t val; -intuser_irq_id; +intirq_num; }; int dpmcp_set_irq(struct fsl_mc_io *mc_io, @@ -133,12 +133,6 @@ int dpmcp_get_irq_status(struct fsl_mc_io *mc_io, uint8_t irq_index, uint32_t*status); -int dpmcp_clear_irq_status(struct fsl_mc_io*mc_io, - uint32_t cmd_flags, - uint16_t token, - uint8_t irq_index, - uint32_t status); - /** * struct dpmcp_attr - Structure representing DPMCP attributes * @id:DPMCP object ID -- 1.7.9.5
[PATCH 13/14] staging: fsl-mc: return -EINVAL for all fsl_mc_portal_allocate() failures
From: Horia Geantă <horia.gea...@nxp.com> There are some error paths that allow for a NULL new_mc_io and err = 0 return code. Return -EINVAL instead. Signed-off-by: Horia Geantă <horia.gea...@nxp.com> Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/mc-allocator.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/fsl-mc/bus/mc-allocator.c b/drivers/staging/fsl-mc/bus/mc-allocator.c index 4676ba1..7ee71e7 100644 --- a/drivers/staging/fsl-mc/bus/mc-allocator.c +++ b/drivers/staging/fsl-mc/bus/mc-allocator.c @@ -306,6 +306,7 @@ int __must_check fsl_mc_portal_allocate(struct fsl_mc_device *mc_dev, if (error < 0) return error; + error = -EINVAL; dpmcp_dev = resource->data; if (WARN_ON(!dpmcp_dev)) goto error_cleanup_resource; -- 1.7.9.5
[PATCH 07/14] staging: fsl-mc: set up coherent dma ops for added devices
From: Stuart Yoder <stuart.yo...@nxp.com> Unless discovered devices have the no shareability flag set, set up coherent dma ops for them. Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com> --- drivers/staging/fsl-mc/bus/mc-bus.c |4 1 file changed, 4 insertions(+) diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-mc/bus/mc-bus.c index 981e4c2..8bf76d7 100644 --- a/drivers/staging/fsl-mc/bus/mc-bus.c +++ b/drivers/staging/fsl-mc/bus/mc-bus.c @@ -469,6 +469,10 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc, goto error_cleanup_dev; } + /* Objects are coherent, unless 'no shareability' flag set. */ + if (!(obj_desc->flags & DPRC_OBJ_FLAG_NO_MEM_SHAREABILITY)) + arch_setup_dma_ops(_dev->dev, 0, 0, NULL, true); + /* * The device-specific probe callback will get invoked by device_add() */ -- 1.7.9.5
[PATCH 00/14] staging: fsl-mc: misc updates
From: Stuart Yoder <stuart.yo...@nxp.com> This patch series makes further progress towards completing the fsl-mc TODO list. -patch 1 removes three items from the TODO file that were previously completed-- multiple root dprc support, MSI support, and command serialization -patch 2 makes some way overdue updates to README.txt from review comments on the mailing list last fall -patches 3-5 update the binary interface for several objects to sync with latest MC firmware (todo item: "MC firmware uprev") -patches 6-13 are cleanup items -change in how object versions are used in binding decisions -setting coherent dma_ops for devices if necessary -setting cacheable flag for object regions if applicable -getting the version of the root dprc from hardware, not based on what was in the .h file -dprc driver now refuses to probe if dprc version is not minimum expected -added quirk for bug in coherency flag for dpseci objects -allocator driver refuses to bind to dpmcp objects if version is not minimum expected -patch 14 adds a second maintainer for the driver Horia Geanta (1): staging: fsl-mc: add quirk handling for dpseci objects < 4.0 Horia Geantă (1): staging: fsl-mc: return -EINVAL for all fsl_mc_portal_allocate() failures Itai Katz (5): staging: fsl-mc: don't use object versions to make binding decisions staging: fsl-mc: set cacheable flag for added devices if applicable staging: fsl-mc: get version of root dprc from MC hardware staging: fsl-mc: add dprc version check staging: fsl-mc: add dpmcp version check Stuart Yoder (7): staging: fsl-mc: TODO updates staging: fsl-mc: DPAA2 overview readme update staging: fsl-mc: update dpmcp binary interface to v3.0 staging: fsl-mc: update dpbp binary interface to v2.2 staging: fsl-mc: update dprc binary interface to v5.1 staging: fsl-mc: set up coherent dma ops for added devices MAINTAINERS: fsl-mc: Add second maintainer MAINTAINERS |1 + drivers/staging/fsl-mc/README.txt | 138 --- drivers/staging/fsl-mc/TODO | 13 --- drivers/staging/fsl-mc/bus/dpbp.c | 77 ++- drivers/staging/fsl-mc/bus/dpmcp-cmd.h |7 +- drivers/staging/fsl-mc/bus/dpmcp.c | 35 +-- drivers/staging/fsl-mc/bus/dpmcp.h | 10 +- drivers/staging/fsl-mc/bus/dprc-cmd.h |6 +- drivers/staging/fsl-mc/bus/dprc-driver.c| 33 ++- drivers/staging/fsl-mc/bus/dprc.c | 26 ++--- drivers/staging/fsl-mc/bus/mc-allocator.c | 18 ++-- drivers/staging/fsl-mc/bus/mc-bus.c | 93 +- drivers/staging/fsl-mc/bus/mc-msi.c |2 +- drivers/staging/fsl-mc/include/dpbp-cmd.h |4 +- drivers/staging/fsl-mc/include/dpbp.h | 51 +- drivers/staging/fsl-mc/include/dprc.h | 19 ++-- drivers/staging/fsl-mc/include/mc-private.h |2 + 17 files changed, 335 insertions(+), 200 deletions(-) -- 1.7.9.5