* Re: [PATCH net-next 1/2] dts: Add bindings for multicast hash bins and perfect filter entries
From: Giuseppe CAVALLARO @ 2014-02-04 9:58 UTC (permalink / raw)
To: Vince Bridgers, devicetree, netdev
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
dinguyen, rayagond
In-Reply-To: <1391199356-27226-2-git-send-email-vbridgers2013@gmail.com>
Hello Vince
On 1/31/2014 9:15 PM, Vince Bridgers wrote:
> This change adds bindings for the number of multicast hash bins and perfect
> filter entries supported by the Synopsys EMAC. The Synopsys EMAC core is
> configurable at device creation time, and can be configured for a different
> number of multicast hash bins and a different number of perfect filter entries.
> The device does not provide a way to query these parameters, therefore
> parameters are required. The Altera Cyclone V SOC has support for 256
> multicast hash bins and 128 perfect filter entries, and is different than
> what's currently provided in the stmmac driver.
>
thanks a lot for these patches, I have just added some comment for the
patch #2
peppe
> Signed-off-by: Vince Bridgers <vbridgers2013@gmail.com>
> ---
> Documentation/devicetree/bindings/net/stmmac.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
> index 9d92d42..dbf7498 100644
> --- a/Documentation/devicetree/bindings/net/stmmac.txt
> +++ b/Documentation/devicetree/bindings/net/stmmac.txt
> @@ -34,6 +34,10 @@ Optional properties:
> reset phandle is given
> - max-frame-size: Maximum Transfer Unit (IEEE defined MTU), rather
> than the maximum frame size.
> +- snps,multicast-filter-bins: Number of multicast filter hash bins
> + supported by this device instance
> +- snps,perfect-filter-entries: Number of perfect filter entries supported
> + by this device instance
>
> Examples:
>
> @@ -46,4 +50,6 @@ Examples:
> mac-address = [000000000000]; /* Filled in by U-Boot */
> max-frame-size = <3800>;
> phy-mode = "gmii";
> + snps,multicast-filter-bins = <256>;
> + snps,perfect-filter-entries = <128>;
> };
>
^ permalink raw reply
* Re: [PATCH net-next 2/2] stmmac: add extended set multicast filter & devicetree options
From: Giuseppe CAVALLARO @ 2014-02-04 9:57 UTC (permalink / raw)
To: Vince Bridgers, devicetree, netdev
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
dinguyen, rayagond
In-Reply-To: <1391199356-27226-3-git-send-email-vbridgers2013@gmail.com>
On 1/31/2014 9:15 PM, Vince Bridgers wrote:
> The synopsys EMAC can be configured for different numbers of multicast hash
> bins and perfect filter entries at device creation time and there's no way
> to query this configuration information at runtime. As a result, a devicetree
> parameter is required in order for the driver to program these filters
> correctly for a particular device instance. This patch extends the current
> driver by providing a different multicast filter programming function if
> different than the currently supported 64 multicast hash bins and 32
> perfect unicast addresses. This patch is required to correct multicast
> filtering for the Altera Cyclone V SOC.
>
> Signed-off-by: Vince Bridgers <vbridgers2013@gmail.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/common.h | 42 +++--
> drivers/net/ethernet/stmicro/stmmac/dwmac1000.h | 5 +-
> .../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 161 +++++++++++++++++---
> .../net/ethernet/stmicro/stmmac/dwmac100_core.c | 29 ++--
> .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 6 +-
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 36 ++---
> .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 51 +++++++
> include/linux/stmmac.h | 2 +
> 8 files changed, 261 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 7834a39..ca07afe 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -294,6 +294,8 @@ struct dma_features {
>
> #define JUMBO_LEN 9000
>
> +#define GMAC_MAX_PERFECT_ADDRESSES 32
> +
> struct stmmac_desc_ops {
> /* DMA RX descriptor ring initialization */
> void (*init_rx_desc) (struct dma_desc *p, int disable_rx_ic, int mode,
> @@ -368,34 +370,37 @@ struct stmmac_dma_ops {
> void (*rx_watchdog) (void __iomem *ioaddr, u32 riwt);
> };
>
> +struct mac_device_info;
> +
> struct stmmac_ops {
> /* MAC core initialization */
> - void (*core_init) (void __iomem *ioaddr, int mtu);
> + void (*core_init)(struct mac_device_info *hw, int mtu);
> /* Enable and verify that the IPC module is supported */
> - int (*rx_ipc) (void __iomem *ioaddr);
> + int (*rx_ipc)(struct mac_device_info *hw);
> /* Dump MAC registers */
> - void (*dump_regs) (void __iomem *ioaddr);
> + void (*dump_regs)(struct mac_device_info *hw);
> /* Handle extra events on specific interrupts hw dependent */
> - int (*host_irq_status) (void __iomem *ioaddr,
> + int (*host_irq_status)(struct mac_device_info *hw,
> struct stmmac_extra_stats *x);
> /* Multicast filter setting */
> - void (*set_filter) (struct net_device *dev, int id);
> + void (*set_filter)(struct mac_device_info *hw,
> + struct net_device *dev);
> /* Flow control setting */
> - void (*flow_ctrl) (void __iomem *ioaddr, unsigned int duplex,
> + void (*flow_ctrl)(struct mac_device_info *hw, unsigned int duplex,
> unsigned int fc, unsigned int pause_time);
> /* Set power management mode (e.g. magic frame) */
> - void (*pmt) (void __iomem *ioaddr, unsigned long mode);
> + void (*pmt)(struct mac_device_info *hw, unsigned long mode);
> /* Set/Get Unicast MAC addresses */
> - void (*set_umac_addr) (void __iomem *ioaddr, unsigned char *addr,
> + void (*set_umac_addr)(struct mac_device_info *hw, unsigned char *addr,
> unsigned int reg_n);
> - void (*get_umac_addr) (void __iomem *ioaddr, unsigned char *addr,
> + void (*get_umac_addr)(struct mac_device_info *hw, unsigned char *addr,
> unsigned int reg_n);
> - void (*set_eee_mode) (void __iomem *ioaddr);
> - void (*reset_eee_mode) (void __iomem *ioaddr);
> - void (*set_eee_timer) (void __iomem *ioaddr, int ls, int tw);
> - void (*set_eee_pls) (void __iomem *ioaddr, int link);
> - void (*ctrl_ane) (void __iomem *ioaddr, bool restart);
> - void (*get_adv) (void __iomem *ioaddr, struct rgmii_adv *adv);
> + void (*set_eee_mode)(struct mac_device_info *hw);
> + void (*reset_eee_mode)(struct mac_device_info *hw);
> + void (*set_eee_timer)(struct mac_device_info *hw, int ls, int tw);
> + void (*set_eee_pls)(struct mac_device_info *hw, int link);
> + void (*ctrl_ane)(struct mac_device_info *hw, bool restart);
> + void (*get_adv)(struct mac_device_info *hw, struct rgmii_adv *adv);
> };
>
> struct stmmac_hwtimestamp {
> @@ -447,9 +452,14 @@ struct mac_device_info {
> struct mii_regs mii; /* MII register Addresses */
> struct mac_link link;
> unsigned int synopsys_uid;
> + void __iomem *pcsr; /* vpointer to device CSRs */
> + int multicast_filter_bins;
> + int unicast_filter_entries;
> + int mcast_bits_log2;
> };
>
> -struct mac_device_info *dwmac1000_setup(void __iomem *ioaddr);
> +struct mac_device_info *dwmac1000_setup(void __iomem *ioaddr,
> + int mcbins, int perfect_uc_entries);
> struct mac_device_info *dwmac100_setup(void __iomem *ioaddr);
>
> void stmmac_set_mac_addr(void __iomem *ioaddr, u8 addr[6],
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
> index f37d90f..40b8533 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
> @@ -87,7 +87,6 @@ enum power_event {
> (reg * 8))
> #define GMAC_ADDR_LOW(reg) (((reg > 15) ? 0x00000804 : 0x00000044) + \
> (reg * 8))
> -#define GMAC_MAX_PERFECT_ADDRESSES 32
>
> /* PCS registers (AN/TBI/SGMII/RGMII) offset */
> #define GMAC_AN_CTRL 0x000000c0 /* AN control */
> @@ -130,6 +129,8 @@ enum power_event {
> #define GMAC_CONTROL_2K 0x08000000 /* IEEE 802.3as 2K packets */
> #define GMAC_CONTROL_TC 0x01000000 /* Transmit Conf. in RGMII/SGMII */
> #define GMAC_CONTROL_WD 0x00800000 /* Disable Watchdog on receive */
> +
> +/* GMAC Configuration defines */
> #define GMAC_CONTROL_JD 0x00400000 /* Jabber disable */
> #define GMAC_CONTROL_BE 0x00200000 /* Frame Burst Enable */
> #define GMAC_CONTROL_JE 0x00100000 /* Jumbo frame */
> @@ -262,5 +263,7 @@ enum rtc_control {
> #define GMAC_MMC_TX_INTR 0x108
> #define GMAC_MMC_RX_CSUM_OFFLOAD 0x208
>
> +#define GMAC_EXTHASH_BASE 0x500
> +
> extern const struct stmmac_dma_ops dwmac1000_dma_ops;
> #endif /* __DWMAC1000_H__ */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> index b3e148e..44db9fb 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> @@ -26,14 +26,15 @@
> Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> *******************************************************************************/
>
> +#include <linux/io.h>
> #include <linux/crc32.h>
> -#include <linux/slab.h>
> #include <linux/ethtool.h>
> -#include <asm/io.h>
> +#include <linux/slab.h>
> #include "dwmac1000.h"
>
> -static void dwmac1000_core_init(void __iomem *ioaddr, int mtu)
> +static void dwmac1000_core_init(struct mac_device_info *hw, int mtu)
> {
> + void __iomem *ioaddr = hw->pcsr;
> u32 value = readl(ioaddr + GMAC_CONTROL);
> value |= GMAC_CORE_INIT;
> if (mtu > 1500)
> @@ -52,8 +53,9 @@ static void dwmac1000_core_init(void __iomem *ioaddr, int mtu)
> #endif
> }
>
> -static int dwmac1000_rx_ipc_enable(void __iomem *ioaddr)
> +static int dwmac1000_rx_ipc_enable(struct mac_device_info *hw)
> {
> + void __iomem *ioaddr = hw->pcsr;
> u32 value = readl(ioaddr + GMAC_CONTROL);
>
> value |= GMAC_CONTROL_IPC;
> @@ -64,8 +66,9 @@ static int dwmac1000_rx_ipc_enable(void __iomem *ioaddr)
> return !!(value & GMAC_CONTROL_IPC);
> }
>
> -static void dwmac1000_dump_regs(void __iomem *ioaddr)
> +static void dwmac1000_dump_regs(struct mac_device_info *hw)
> {
> + void __iomem *ioaddr = hw->pcsr;
> int i;
> pr_info("\tDWMAC1000 regs (base addr = 0x%p)\n", ioaddr);
>
> @@ -76,21 +79,113 @@ static void dwmac1000_dump_regs(void __iomem *ioaddr)
> }
> }
>
> -static void dwmac1000_set_umac_addr(void __iomem *ioaddr, unsigned char *addr,
> +static void dwmac1000_set_umac_addr(struct mac_device_info *hw,
> + unsigned char *addr,
> unsigned int reg_n)
> {
> - stmmac_set_mac_addr(ioaddr, addr, GMAC_ADDR_HIGH(reg_n),
> + stmmac_set_mac_addr(hw->pcsr, addr, GMAC_ADDR_HIGH(reg_n),
> GMAC_ADDR_LOW(reg_n));
> }
>
> -static void dwmac1000_get_umac_addr(void __iomem *ioaddr, unsigned char *addr,
> +static void dwmac1000_get_umac_addr(struct mac_device_info *hw,
> + unsigned char *addr,
> unsigned int reg_n)
> {
> - stmmac_get_mac_addr(ioaddr, addr, GMAC_ADDR_HIGH(reg_n),
> + stmmac_get_mac_addr(hw->pcsr, addr, GMAC_ADDR_HIGH(reg_n),
> GMAC_ADDR_LOW(reg_n));
> }
>
> -static void dwmac1000_set_filter(struct net_device *dev, int id)
> +static void dwmac1000_set_extmchash(void __iomem *ioaddr, u32 *mcfilterbits,
> + int numhashregs)
> +{
> + int regs;
> + for (regs = 0; regs < numhashregs; regs++)
> + writel(mcfilterbits[regs],
> + ioaddr + GMAC_EXTHASH_BASE + regs * 4);
> +}
> +
> +static void dwmac1000_set_filterex(struct mac_device_info *hw,
> + struct net_device *dev)
> +{
> + void __iomem *ioaddr = (void __iomem *)dev->base_addr;
> + unsigned int value = 0;
> + unsigned int perfect_addr_number;
> + u32 mc_filter[8];
> +
> + pr_debug("%s: # mcasts %d, # unicast %d\n", __func__,
> + netdev_mc_count(dev), netdev_uc_count(dev));
> +
> + if (dev->flags & IFF_PROMISC) {
> + value = GMAC_FRAME_FILTER_PR;
> + } else if (dev->flags & IFF_ALLMULTI) {
> + value = GMAC_FRAME_FILTER_PM; /* pass all multi */
> + } else if (!netdev_mc_empty(dev)) {
> + struct netdev_hw_addr *ha;
> +
> + memset(mc_filter, 0, sizeof(mc_filter));
> +
> + /* Hash filter for multicast */
> + value = GMAC_FRAME_FILTER_HMC;
> +
> + netdev_for_each_mc_addr(ha, dev) {
> + /* The upper n bits of the calculated CRC are used to
> + * index the contents of the hash table depending
> + * on the particular core's multicast hash size
> + * configured through Synopsys Core Consultant
> + */
> + int bit_nr = bitrev32(~crc32_le(~0, ha->addr,
> + ETH_ALEN)) >>
> + (32 - hw->mcast_bits_log2);
> +
> + /* The most significant bit determines the register to
> + * use (H/L) while the other 5 bits determine the bit
> + * within the register.
> + */
> + mc_filter[bit_nr >> 5] |= 1 << (bit_nr & 31);
> + }
> + if (hw->mcast_bits_log2 == 6) {
> + writel(mc_filter[0], ioaddr + GMAC_HASH_LOW);
> + writel(mc_filter[1], ioaddr + GMAC_HASH_HIGH);
> + } else if (hw->mcast_bits_log2 == 7) {
> + dwmac1000_set_extmchash(ioaddr, mc_filter, 4);
> + } else if (hw->mcast_bits_log2 == 8) {
> + dwmac1000_set_extmchash(ioaddr, mc_filter, 8);
> + } else {
> + pr_debug("STMMAC: err in setting mulitcast filter\n");
> + }
> + }
> +
> + /* Extra 16 regs are available in cores newer than the 3.40. */
> + if (hw->synopsys_uid > DWMAC_CORE_3_40)
> + perfect_addr_number = hw->unicast_filter_entries;
> + else
> + perfect_addr_number = hw->unicast_filter_entries / 2;
can you check if this is safe enough? I mean if we pass a setting that
could generate problems in case of old Synopsys chips.
> +
> + /* Handle multiple unicast addresses (perfect filtering) */
> + if (netdev_uc_count(dev) > perfect_addr_number)
> + /* Switch to promiscuous mode if more than 16 addrs
> + * are required
> + */
> + value |= GMAC_FRAME_FILTER_PR;
> + else {
> + int reg = 1;
> + struct netdev_hw_addr *ha;
> +
> + netdev_for_each_uc_addr(ha, dev) {
> + dwmac1000_set_umac_addr(hw, ha->addr, reg);
> + reg++;
> + }
> + }
> +
> +#ifdef FRAME_FILTER_DEBUG
> + /* Enable Receive all mode (to debug filtering_fail errors) */
> + value |= GMAC_FRAME_FILTER_RA;
> +#endif
> + writel(value, ioaddr + GMAC_FRAME_FILTER);
> +}
> +
> +static void dwmac1000_set_filter(struct mac_device_info *hw,
> + struct net_device *dev)
> {
> void __iomem *ioaddr = (void __iomem *)dev->base_addr;
> unsigned int value = 0;
> @@ -130,7 +225,7 @@ static void dwmac1000_set_filter(struct net_device *dev, int id)
> }
>
> /* Extra 16 regs are available in cores newer than the 3.40. */
> - if (id > DWMAC_CORE_3_40)
> + if (hw->synopsys_uid > DWMAC_CORE_3_40)
> perfect_addr_number = GMAC_MAX_PERFECT_ADDRESSES;
> else
> perfect_addr_number = GMAC_MAX_PERFECT_ADDRESSES / 2;
> @@ -146,7 +241,7 @@ static void dwmac1000_set_filter(struct net_device *dev, int id)
> struct netdev_hw_addr *ha;
>
> netdev_for_each_uc_addr(ha, dev) {
> - dwmac1000_set_umac_addr(ioaddr, ha->addr, reg);
> + dwmac1000_set_umac_addr(hw, ha->addr, reg);
> reg++;
> }
> }
> @@ -162,9 +257,10 @@ static void dwmac1000_set_filter(struct net_device *dev, int id)
> readl(ioaddr + GMAC_HASH_HIGH), readl(ioaddr + GMAC_HASH_LOW));
> }
>
> -static void dwmac1000_flow_ctrl(void __iomem *ioaddr, unsigned int duplex,
> +static void dwmac1000_flow_ctrl(struct mac_device_info *hw, unsigned int duplex,
> unsigned int fc, unsigned int pause_time)
> {
> + void __iomem *ioaddr = hw->pcsr;
> unsigned int flow = 0;
>
> pr_debug("GMAC Flow-Control:\n");
> @@ -185,8 +281,9 @@ static void dwmac1000_flow_ctrl(void __iomem *ioaddr, unsigned int duplex,
> writel(flow, ioaddr + GMAC_FLOW_CTRL);
> }
>
> -static void dwmac1000_pmt(void __iomem *ioaddr, unsigned long mode)
> +static void dwmac1000_pmt(struct mac_device_info *hw, unsigned long mode)
> {
> + void __iomem *ioaddr = hw->pcsr;
> unsigned int pmt = 0;
>
> if (mode & WAKE_MAGIC) {
> @@ -201,9 +298,10 @@ static void dwmac1000_pmt(void __iomem *ioaddr, unsigned long mode)
> writel(pmt, ioaddr + GMAC_PMT);
> }
>
> -static int dwmac1000_irq_status(void __iomem *ioaddr,
> +static int dwmac1000_irq_status(struct mac_device_info *hw,
> struct stmmac_extra_stats *x)
> {
> + void __iomem *ioaddr = hw->pcsr;
> u32 intr_status = readl(ioaddr + GMAC_INT_STATUS);
> int ret = 0;
>
> @@ -268,8 +366,9 @@ static int dwmac1000_irq_status(void __iomem *ioaddr,
> return ret;
> }
>
> -static void dwmac1000_set_eee_mode(void __iomem *ioaddr)
> +static void dwmac1000_set_eee_mode(struct mac_device_info *hw)
> {
> + void __iomem *ioaddr = hw->pcsr;
> u32 value;
>
> /* Enable the link status receive on RGMII, SGMII ore SMII
> @@ -281,8 +380,9 @@ static void dwmac1000_set_eee_mode(void __iomem *ioaddr)
> writel(value, ioaddr + LPI_CTRL_STATUS);
> }
>
> -static void dwmac1000_reset_eee_mode(void __iomem *ioaddr)
> +static void dwmac1000_reset_eee_mode(struct mac_device_info *hw)
> {
> + void __iomem *ioaddr = hw->pcsr;
> u32 value;
>
> value = readl(ioaddr + LPI_CTRL_STATUS);
> @@ -290,8 +390,9 @@ static void dwmac1000_reset_eee_mode(void __iomem *ioaddr)
> writel(value, ioaddr + LPI_CTRL_STATUS);
> }
>
> -static void dwmac1000_set_eee_pls(void __iomem *ioaddr, int link)
> +static void dwmac1000_set_eee_pls(struct mac_device_info *hw, int link)
> {
> + void __iomem *ioaddr = hw->pcsr;
> u32 value;
>
> value = readl(ioaddr + LPI_CTRL_STATUS);
> @@ -304,8 +405,9 @@ static void dwmac1000_set_eee_pls(void __iomem *ioaddr, int link)
> writel(value, ioaddr + LPI_CTRL_STATUS);
> }
>
> -static void dwmac1000_set_eee_timer(void __iomem *ioaddr, int ls, int tw)
> +static void dwmac1000_set_eee_timer(struct mac_device_info *hw, int ls, int tw)
> {
> + void __iomem *ioaddr = hw->pcsr;
> int value = ((tw & 0xffff)) | ((ls & 0x7ff) << 16);
>
> /* Program the timers in the LPI timer control register:
> @@ -318,8 +420,9 @@ static void dwmac1000_set_eee_timer(void __iomem *ioaddr, int ls, int tw)
> writel(value, ioaddr + LPI_TIMER_CTRL);
> }
>
> -static void dwmac1000_ctrl_ane(void __iomem *ioaddr, bool restart)
> +static void dwmac1000_ctrl_ane(struct mac_device_info *hw, bool restart)
> {
> + void __iomem *ioaddr = hw->pcsr;
> u32 value;
>
> value = readl(ioaddr + GMAC_AN_CTRL);
> @@ -332,8 +435,9 @@ static void dwmac1000_ctrl_ane(void __iomem *ioaddr, bool restart)
> writel(value, ioaddr + GMAC_AN_CTRL);
> }
>
> -static void dwmac1000_get_adv(void __iomem *ioaddr, struct rgmii_adv *adv)
> +static void dwmac1000_get_adv(struct mac_device_info *hw, struct rgmii_adv *adv)
> {
> + void __iomem *ioaddr = hw->pcsr;
> u32 value = readl(ioaddr + GMAC_ANE_ADV);
>
> if (value & GMAC_ANE_FD)
> @@ -353,7 +457,7 @@ static void dwmac1000_get_adv(void __iomem *ioaddr, struct rgmii_adv *adv)
> adv->lp_pause = (value & GMAC_ANE_PSE) >> GMAC_ANE_PSE_SHIFT;
> }
>
> -static const struct stmmac_ops dwmac1000_ops = {
> +static struct stmmac_ops dwmac1000_ops = {
> .core_init = dwmac1000_core_init,
> .rx_ipc = dwmac1000_rx_ipc_enable,
> .dump_regs = dwmac1000_dump_regs,
> @@ -371,7 +475,8 @@ static const struct stmmac_ops dwmac1000_ops = {
> .get_adv = dwmac1000_get_adv,
> };
>
> -struct mac_device_info *dwmac1000_setup(void __iomem *ioaddr)
> +struct mac_device_info *dwmac1000_setup(void __iomem *ioaddr, int mcbins,
> + int perfect_uc_entries)
> {
> struct mac_device_info *mac;
> u32 hwid = readl(ioaddr + GMAC_VERSION);
> @@ -380,6 +485,16 @@ struct mac_device_info *dwmac1000_setup(void __iomem *ioaddr)
> if (!mac)
> return NULL;
>
> + mac->pcsr = ioaddr;
> + mac->multicast_filter_bins = mcbins;
> + mac->unicast_filter_entries = perfect_uc_entries;
> + mac->mcast_bits_log2 = 0;
> + if (mac->multicast_filter_bins)
> + mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
> +
> + if (mac->mcast_bits_log2 != 64)
> + dwmac1000_ops.set_filter = dwmac1000_set_filterex;
> +
> mac->mac = &dwmac1000_ops;
> mac->dma = &dwmac1000_dma_ops;
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> index 2ff767b..3ee3ab5 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> @@ -28,12 +28,13 @@
> Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> *******************************************************************************/
>
> -#include <linux/crc32.h>
> #include <asm/io.h>
> +#include <linux/crc32.h>
> #include "dwmac100.h"
>
> -static void dwmac100_core_init(void __iomem *ioaddr, int mtu)
> +static void dwmac100_core_init(struct mac_device_info *hw, int mtu)
> {
> + void __iomem *ioaddr = hw->pcsr;
> u32 value = readl(ioaddr + MAC_CONTROL);
>
> writel((value | MAC_CORE_INIT), ioaddr + MAC_CONTROL);
> @@ -43,8 +44,9 @@ static void dwmac100_core_init(void __iomem *ioaddr, int mtu)
> #endif
> }
>
> -static void dwmac100_dump_mac_regs(void __iomem *ioaddr)
> +static void dwmac100_dump_mac_regs(struct mac_device_info *hw)
> {
> + void __iomem *ioaddr = hw->pcsr;
> pr_info("\t----------------------------------------------\n"
> "\t DWMAC 100 CSR (base addr = 0x%p)\n"
> "\t----------------------------------------------\n", ioaddr);
> @@ -66,30 +68,35 @@ static void dwmac100_dump_mac_regs(void __iomem *ioaddr)
> readl(ioaddr + MAC_VLAN2));
> }
>
> -static int dwmac100_rx_ipc_enable(void __iomem *ioaddr)
> +static int dwmac100_rx_ipc_enable(struct mac_device_info *hw)
> {
> return 0;
> }
>
> -static int dwmac100_irq_status(void __iomem *ioaddr,
> +static int dwmac100_irq_status(struct mac_device_info *hw,
> struct stmmac_extra_stats *x)
> {
> return 0;
> }
>
> -static void dwmac100_set_umac_addr(void __iomem *ioaddr, unsigned char *addr,
> +static void dwmac100_set_umac_addr(struct mac_device_info *hw,
> + unsigned char *addr,
> unsigned int reg_n)
> {
> + void __iomem *ioaddr = hw->pcsr;
> stmmac_set_mac_addr(ioaddr, addr, MAC_ADDR_HIGH, MAC_ADDR_LOW);
> }
>
> -static void dwmac100_get_umac_addr(void __iomem *ioaddr, unsigned char *addr,
> +static void dwmac100_get_umac_addr(struct mac_device_info *hw,
> + unsigned char *addr,
> unsigned int reg_n)
> {
> + void __iomem *ioaddr = hw->pcsr;
> stmmac_get_mac_addr(ioaddr, addr, MAC_ADDR_HIGH, MAC_ADDR_LOW);
> }
>
> -static void dwmac100_set_filter(struct net_device *dev, int id)
> +static void dwmac100_set_filter(struct mac_device_info *hw,
> + struct net_device *dev)
> {
> void __iomem *ioaddr = (void __iomem *)dev->base_addr;
> u32 value = readl(ioaddr + MAC_CONTROL);
> @@ -137,9 +144,10 @@ static void dwmac100_set_filter(struct net_device *dev, int id)
> writel(value, ioaddr + MAC_CONTROL);
> }
>
> -static void dwmac100_flow_ctrl(void __iomem *ioaddr, unsigned int duplex,
> +static void dwmac100_flow_ctrl(struct mac_device_info *hw, unsigned int duplex,
> unsigned int fc, unsigned int pause_time)
> {
> + void __iomem *ioaddr = hw->pcsr;
> unsigned int flow = MAC_FLOW_CTRL_ENABLE;
>
> if (duplex)
> @@ -148,7 +156,7 @@ static void dwmac100_flow_ctrl(void __iomem *ioaddr, unsigned int duplex,
> }
>
> /* No PMT module supported on ST boards with this Eth chip. */
> -static void dwmac100_pmt(void __iomem *ioaddr, unsigned long mode)
> +static void dwmac100_pmt(struct mac_device_info *hw, unsigned long mode)
> {
> return;
> }
> @@ -175,6 +183,7 @@ struct mac_device_info *dwmac100_setup(void __iomem *ioaddr)
>
> pr_info("\tDWMAC100\n");
>
> + mac->pcsr = ioaddr;
> mac->mac = &dwmac100_ops;
> mac->dma = &dwmac100_dma_ops;
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> index c5f9cb8..e679fa6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> @@ -262,7 +262,7 @@ static int stmmac_ethtool_getsettings(struct net_device *dev,
>
> /* Get and convert ADV/LP_ADV from the HW AN registers */
> if (priv->hw->mac->get_adv)
> - priv->hw->mac->get_adv(priv->ioaddr, &adv);
> + priv->hw->mac->get_adv(priv->hw, &adv);
> else
> return -EOPNOTSUPP; /* should never happen indeed */
>
> @@ -352,7 +352,7 @@ static int stmmac_ethtool_setsettings(struct net_device *dev,
>
> spin_lock(&priv->lock);
> if (priv->hw->mac->ctrl_ane)
> - priv->hw->mac->ctrl_ane(priv->ioaddr, 1);
> + priv->hw->mac->ctrl_ane(priv->hw, 1);
> spin_unlock(&priv->lock);
> }
>
> @@ -471,7 +471,7 @@ stmmac_set_pauseparam(struct net_device *netdev,
> if (netif_running(netdev))
> ret = phy_start_aneg(phy);
> } else
> - priv->hw->mac->flow_ctrl(priv->ioaddr, phy->duplex,
> + priv->hw->mac->flow_ctrl(priv->hw, phy->duplex,
> priv->flow_ctrl, priv->pause);
> spin_unlock(&priv->lock);
> return ret;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index d93aa87..aaa14b2 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -233,7 +233,7 @@ static void stmmac_enable_eee_mode(struct stmmac_priv *priv)
> /* Check and enter in LPI mode */
> if ((priv->dirty_tx == priv->cur_tx) &&
> (priv->tx_path_in_lpi_mode == false))
> - priv->hw->mac->set_eee_mode(priv->ioaddr);
> + priv->hw->mac->set_eee_mode(priv->hw);
> }
>
> /**
> @@ -244,7 +244,7 @@ static void stmmac_enable_eee_mode(struct stmmac_priv *priv)
> */
> void stmmac_disable_eee_mode(struct stmmac_priv *priv)
> {
> - priv->hw->mac->reset_eee_mode(priv->ioaddr);
> + priv->hw->mac->reset_eee_mode(priv->hw);
> del_timer_sync(&priv->eee_ctrl_timer);
> priv->tx_path_in_lpi_mode = false;
> }
> @@ -298,12 +298,12 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
> priv->eee_ctrl_timer.expires = STMMAC_LPI_T(eee_timer);
> add_timer(&priv->eee_ctrl_timer);
>
> - priv->hw->mac->set_eee_timer(priv->ioaddr,
> + priv->hw->mac->set_eee_timer(priv->hw,
> STMMAC_DEFAULT_LIT_LS,
> priv->tx_lpi_timer);
> } else
> /* Set HW EEE according to the speed */
> - priv->hw->mac->set_eee_pls(priv->ioaddr,
> + priv->hw->mac->set_eee_pls(priv->hw,
> priv->phydev->link);
>
> pr_info("stmmac: Energy-Efficient Ethernet initialized\n");
> @@ -678,7 +678,7 @@ static void stmmac_adjust_link(struct net_device *dev)
> }
> /* Flow Control operation */
> if (phydev->pause)
> - priv->hw->mac->flow_ctrl(priv->ioaddr, phydev->duplex,
> + priv->hw->mac->flow_ctrl(priv->hw, phydev->duplex,
> fc, pause_time);
>
> if (phydev->speed != priv->speed) {
> @@ -1519,8 +1519,7 @@ static int stmmac_get_hw_features(struct stmmac_priv *priv)
> static void stmmac_check_ether_addr(struct stmmac_priv *priv)
> {
> if (!is_valid_ether_addr(priv->dev->dev_addr)) {
> - priv->hw->mac->get_umac_addr((void __iomem *)
> - priv->dev->base_addr,
> + priv->hw->mac->get_umac_addr(priv->hw,
> priv->dev->dev_addr, 0);
> if (!is_valid_ether_addr(priv->dev->dev_addr))
> eth_hw_addr_random(priv->dev);
> @@ -1617,14 +1616,14 @@ static int stmmac_hw_setup(struct net_device *dev)
> }
>
> /* Copy the MAC addr into the HW */
> - priv->hw->mac->set_umac_addr(priv->ioaddr, dev->dev_addr, 0);
> + priv->hw->mac->set_umac_addr(priv->hw, dev->dev_addr, 0);
>
> /* If required, perform hw setup of the bus. */
> if (priv->plat->bus_setup)
> priv->plat->bus_setup(priv->ioaddr);
>
> /* Initialize the MAC Core */
> - priv->hw->mac->core_init(priv->ioaddr, dev->mtu);
> + priv->hw->mac->core_init(priv->hw, dev->mtu);
>
> /* Enable the MAC Rx/Tx */
> stmmac_set_mac(priv->ioaddr, true);
> @@ -1650,7 +1649,7 @@ static int stmmac_hw_setup(struct net_device *dev)
>
> /* Dump DMA/MAC registers */
> if (netif_msg_hw(priv)) {
> - priv->hw->mac->dump_regs(priv->ioaddr);
> + priv->hw->mac->dump_regs(priv->hw);
> priv->hw->dma->dump_regs(priv->ioaddr);
> }
> priv->tx_lpi_timer = STMMAC_DEFAULT_TWT_LS;
> @@ -1665,7 +1664,7 @@ static int stmmac_hw_setup(struct net_device *dev)
> }
>
> if (priv->pcs && priv->hw->mac->ctrl_ane)
> - priv->hw->mac->ctrl_ane(priv->ioaddr, 0);
> + priv->hw->mac->ctrl_ane(priv->hw, 0);
>
> return 0;
> }
> @@ -2244,7 +2243,7 @@ static void stmmac_set_rx_mode(struct net_device *dev)
> struct stmmac_priv *priv = netdev_priv(dev);
>
> spin_lock(&priv->lock);
> - priv->hw->mac->set_filter(dev, priv->synopsys_id);
> + priv->hw->mac->set_filter(priv->hw, dev);
> spin_unlock(&priv->lock);
> }
>
> @@ -2334,8 +2333,7 @@ static irqreturn_t stmmac_interrupt(int irq, void *dev_id)
>
> /* To handle GMAC own interrupts */
> if (priv->plat->has_gmac) {
> - int status = priv->hw->mac->host_irq_status((void __iomem *)
> - dev->base_addr,
> + int status = priv->hw->mac->host_irq_status(priv->hw,
> &priv->xstats);
> if (unlikely(status)) {
> /* For LPI we need to save the tx status */
> @@ -2619,7 +2617,9 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
> /* Identify the MAC HW device */
> if (priv->plat->has_gmac) {
> priv->dev->priv_flags |= IFF_UNICAST_FLT;
> - mac = dwmac1000_setup(priv->ioaddr);
> + mac = dwmac1000_setup(priv->ioaddr,
> + priv->plat->multicast_filter_bins,
> + priv->plat->unicast_filter_entries);
> } else {
> mac = dwmac100_setup(priv->ioaddr);
> }
> @@ -2668,7 +2668,7 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
> /* To use alternate (extended) or normal descriptor structures */
> stmmac_selec_desc_mode(priv);
>
> - ret = priv->hw->mac->rx_ipc(priv->ioaddr);
> + ret = priv->hw->mac->rx_ipc(priv->hw);
> if (!ret) {
> pr_warn(" RX IPC Checksum Offload not configured.\n");
> priv->plat->rx_coe = STMMAC_RX_COE_NONE;
> @@ -2888,7 +2888,7 @@ int stmmac_suspend(struct net_device *ndev)
>
> /* Enable Power down mode by programming the PMT regs */
> if (device_may_wakeup(priv->device)) {
> - priv->hw->mac->pmt(priv->ioaddr, priv->wolopts);
> + priv->hw->mac->pmt(priv->hw, priv->wolopts);
> priv->irq_wake = 1;
> } else {
> stmmac_set_mac(priv->ioaddr, false);
> @@ -2917,7 +2917,7 @@ int stmmac_resume(struct net_device *ndev)
> * from another devices (e.g. serial console).
> */
> if (device_may_wakeup(priv->device)) {
> - priv->hw->mac->pmt(priv->ioaddr, 0);
> + priv->hw->mac->pmt(priv->hw, 0);
> priv->irq_wake = 0;
> } else {
> pinctrl_pm_select_default_state(priv->device);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 5884a7d..4502cde 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -43,6 +43,42 @@ static const struct of_device_id stmmac_dt_ids[] = {
> };
> MODULE_DEVICE_TABLE(of, stmmac_dt_ids);
>
> +
> +static int stmmac_validate_mcast_bins(int mcast_bins)
> +{
> + int x = mcast_bins;
> + switch (x) {
> + case 0:
> + case HASH_TABLE_SIZE:
> + case 128:
> + case 256:
> + break;
> + default:
> + x = HASH_TABLE_SIZE;
> + pr_info("Hash table entries set to unexpected value %d",
> + mcast_bins);
> + break;
> + }
> + return x;
> +}
> +
> +static int stmmac_validate_ucast_entries(int ucast_entries)
> +{
> + int x = ucast_entries;
> + switch (x) {
> + case 32:
> + case 64:
> + case 128:
> + break;
> + default:
> + x = 32;
> + pr_info("Unicast table entries set to unexpected value %d\n",
> + ucast_entries);
> + break;
> + }
> + return x;
> +}
> +
> #ifdef CONFIG_OF
> static int stmmac_probe_config_dt(struct platform_device *pdev,
> struct plat_stmmacenet_data *plat,
> @@ -107,6 +143,13 @@ static int stmmac_probe_config_dt(struct platform_device *pdev,
> */
> plat->maxmtu = JUMBO_LEN;
>
> + /* Set default value for multicast hash bins */
> + plat->multicast_filter_bins = HASH_TABLE_SIZE;
> +
> + /* Set default value for unicast filter entries */
> + plat->unicast_filter_entries = GMAC_MAX_PERFECT_ADDRESSES;
> +
> +
> /*
> * Currently only the properties needed on SPEAr600
> * are provided. All other properties should be added
> @@ -123,6 +166,14 @@ static int stmmac_probe_config_dt(struct platform_device *pdev,
> * are clearly MTUs
> */
> of_property_read_u32(np, "max-frame-size", &plat->maxmtu);
> + of_property_read_u32(np, "snps,multicast-filter-bins",
> + &plat->multicast_filter_bins);
> + of_property_read_u32(np, "snps,perfect-filter-entries",
> + &plat->unicast_filter_entries);
> + plat->unicast_filter_entries = stmmac_validate_ucast_entries(
> + plat->unicast_filter_entries);
> + plat->multicast_filter_bins = stmmac_validate_mcast_bins(
> + plat->multicast_filter_bins);
Can this validation be done in main?
> plat->has_gmac = 1;
> plat->pmt = 1;
> }
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 6f27d4f..cd63851 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -112,6 +112,8 @@ struct plat_stmmacenet_data {
> int riwt_off;
> int max_speed;
> int maxmtu;
> + int multicast_filter_bins;
> + int unicast_filter_entries;
> void (*fix_mac_speed)(void *priv, unsigned int speed);
> void (*bus_setup)(void __iomem *ioaddr);
> void *(*setup)(struct platform_device *pdev);
>
^ permalink raw reply
* Re: [PATCH] arm64: Add architecture support for PCI
From: Arnd Bergmann @ 2014-02-04 9:44 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: devicetree@vger.kernel.org, linaro-kernel@lists.linaro.org,
linux-pci, Liviu Dudau, LKML, Catalin Marinas, Bjorn Helgaas,
LAKML
In-Reply-To: <20140203233137.GH2519@obsidianresearch.com>
On Monday 03 February 2014 16:31:37 Jason Gunthorpe wrote:
> Specifying 'use EHCI, AHCI, etc' - which are all PCI based standards
> without clearly specifying exactly how PCI is suppose to work is
> completely bonkers.
>
> What is needed is a spec that says:
> 1) Here is how you generate config TLPs. A MMIO region that
> conforms to the already specified x86 ECAM would
> be perfect
> 2) Here is a dword by dword break down of the entire config space in
> a SOC. Here is where a on-board AHCI controller must show up in
> config space. Here is how an external PCI-E port must show
> up. Etc. Most of this is already specified, but it clearly needs
> to be layed out explicitly for ARM SOCs to actually follow it.
> 3) Here is how you specify the aperture(s) associated with PCI BAR's
> and bridge windows in config space. And yes: The CONFIG SPACE
> BARS MUST WORK.
> 4) Here is how MSI works, these are the values you put in the
> address/data and here is how you collect the interrupt.
> 5) Here is how Legacy INTx must be mapped into the GIC.
>
> This is what x86 does, and they have been doing it well for 10
> years. If you want to play in the server game you have to properly
> implement PCI.
I'm pretty sure the authors of the SBSA actually thought that was
what they wrote, by referring to external specifications (pci-3.0,
ehci, ahci, ...). However, it seems they were either foolish enough
to believe that hardware designers would follow these specs, or
they were intentionally misled and got talked into putting ambiguous
terminology in because there were already hardware designs that
are not exactly in line with the spirit of the SBSA but can be
argued to be conforming to the text for a lax interpretation.
I think EHCI is a much better example than PCI here, because the
spec has exactly one line to say about it, where it spends a whole
chapter on PCI.
Here is how a sane person would read SBSA to create a compliant
implementation:
I have to use EHCI version 1.1 to provide USB-2.0 support. EHCI
is a PCI device, so I'll put it behind a PCIe port that complies
to the PCIe section of the SBSA. Since EHCI by itself only provides
high-speed USB, and USB-2.0 mandates I provide low-speed and
full-speed as well, I have to add a USB hub device. It would have
been easier to just use OHCI for these, but SBSA says I can't.
Now I want to integrate the EHCI into my SoC and not waste one
of my precious PCIe root ports, so I have to create another PCI
domain with its own ECAM compliant config space to put it into.
Fortunately SBSA lets me add an arbitrary number of PCI domains,
as long as they are all strictly compliant. To software it will
look exactly as if it was on an external card, I just have to
ensure the boot loader correctly sets up the clocks and the phy
before an SBSA compliant OS gets loaded, all runtime power
management will get handled through the EHCI-1.1 energy-efficiency
extensions.
Here is how a crazy person would read the same sentence in the SBSA:
I have an IP block that implements the EHCI register set, that
should be good enough. It's not a fast device, so I can put it
on a non-coherent bus. Since the SoC will be used for networking,
I'll put the registers into big-endian configuration to make it
easier for the OS to access. I'm not allowed to have USB-1.1
according to SBSA, so I can get away without a hub or an extra
OHCI. I can't support MSI because it's not a PCI device, and
the GIC is pretty full, so I'll just connect the IRQ line to
the GPIO controller. In order to do better power management,
I'll design a fancy PHY that the device driver will manage
for implementing autosuspend. I should also give the OS
fine-grained control over the clocks, but it will have to share
the clock domain with the other devices on the same edge of the
chip. The EHCI device is not part of PCI, which measn I don't
have to use the standard SMMU. However, my EHCI implementation
can only do 32-bit DMA, and I'll have to design my own IOMMU
to let it access the entire memory range. USB-OTG is a great
feature and we already paid for having this in our EHCI
implementation, better make sure it comes up in endpoint mode
after waking up from power saving.
Arnd
^ permalink raw reply
* Re: [PATCH v5 13/14] ARM: sun4i: dts: Add ahci / sata support
From: Maxime Ripard @ 2014-02-04 9:44 UTC (permalink / raw)
To: Hans de Goede
Cc: Tejun Heo, Oliver Schinagl, Richard Zhu, Roger Quadros,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <52EF70E2.6070803-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4066 bytes --]
On Mon, Feb 03, 2014 at 11:35:14AM +0100, Hans de Goede wrote:
> Hi,
>
> On 01/31/2014 02:45 PM, Maxime Ripard wrote:
> >Hi Hans,
> >
> >On Wed, Jan 22, 2014 at 08:04:48PM +0100, Hans de Goede wrote:
> >>From: Oliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
> >>
> >>This patch adds sunxi sata support to A10 boards that have such a connector.
> >>Some boards also feature a regulator via a GPIO and support for this is also
> >>added.
> >>
> >>Signed-off-by: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
> >>Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >>---
> >> arch/arm/boot/dts/sun4i-a10-a1000.dts | 4 ++++
> >> arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 6 +++++
> >> arch/arm/boot/dts/sun4i-a10.dtsi | 8 +++++++
> >> arch/arm/boot/dts/sunxi-ahci-reg.dtsi | 38 ++++++++++++++++++++++++++++++
> >
> >I'm still half convinced about this at the moment, given the number of
> >platforms we support, we can always change it back if things become too messy.
>
> I assume that this == sunxi-ahci-reg.dtsi ? To be sure I understand
> you correctly, you're ok with going this route for now, right ?
Yep.
> How about the same for the usb ohci/ehci controller dts patches ? Currently they
> are still using the put a regulator node in each dts file model, which leads to
> a lot of boilerplate code. So I would like to move to the same model as I'm
> using here for the sata supply.
That would make sense too.
> >> 4 files changed, 56 insertions(+)
> >> create mode 100644 arch/arm/boot/dts/sunxi-ahci-reg.dtsi
> >>
> >>diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >>index aef8207..3fb7305 100644
> >>--- a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >>+++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> >>@@ -48,6 +48,10 @@
> >> status = "okay";
> >> };
> >>
> >>+ ahci: sata@01c18000 {
> >>+ status = "okay";
> >>+ };
> >>+
> >> pinctrl@01c20800 {
> >> mmc0_cd_pin_a1000: mmc0_cd_pin@0 {
> >> allwinner,pins = "PH1";
> >>diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> >>index f50fb2b..6ae1110 100644
> >>--- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> >>+++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> >>@@ -12,6 +12,7 @@
> >>
> >> /dts-v1/;
> >> /include/ "sun4i-a10.dtsi"
> >>+/include/ "sunxi-ahci-reg.dtsi"
> >>
> >> / {
> >> model = "Cubietech Cubieboard";
> >>@@ -51,6 +52,11 @@
> >> status = "okay";
> >> };
> >>
> >>+ ahci: sata@01c18000 {
> >>+ target-supply = <®_ahci_5v>;
> >>+ status = "okay";
> >>+ };
> >>+
> >> pinctrl@01c20800 {
> >> mmc0_cd_pin_cubieboard: mmc0_cd_pin@0 {
> >> allwinner,pins = "PH1";
> >>diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> >>index 4736dd2..198dcda 100644
> >>--- a/arch/arm/boot/dts/sun4i-a10.dtsi
> >>+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> >>@@ -331,6 +331,14 @@
> >> status = "disabled";
> >> };
> >>
> >>+ ahci: sata@01c18000 {
> >>+ compatible = "allwinner,sun4i-a10-ahci";
> >
> >To be consistent with the rest of the sun4i devices compatible, It
> >should be sun4i-ahci.
> >
> >However, since these devices don't use the same compatible pattern as
> >their own machine compatible, and are consisent with the rest of the
> >compatibles for the other SoCs, we can probably make this a go to
> >transition progressively to this pattern.
>
> Ack, I think it would be good to be consistent and try to use
> sun?i-aXX-foo everywhere. I noticed that we already use that in various
> places, so I thought it would be good to do that for all new dts bindings.
Yes, that's my plan.
> >I'll cook up some patches for the other devices.
>
> Thanks.
And I sent them on sunday.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH 2/2] Documentation: devicetree: Add boost-opp binding to list boost mode OPPs
From: Thomas Abraham @ 2014-02-04 9:41 UTC (permalink / raw)
To: linux-pm, devicetree, linux-arm-kernel
Cc: rjw, linux-samsung-soc, kgene.kim, t.figa, l.majewski,
viresh.kumar, thomas.ab, Nishanth Menon, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala
In-Reply-To: <1391506890-7335-1-git-send-email-thomas.ab@samsung.com>
From: Thomas Abraham <thomas.ab@samsung.com>
Certain CPUs or devices can support optional boost operating modes. Add a new
binding to list OPPs to be additionally made available in boost operating modes.
Cc: Nishanth Menon <nm@ti.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
---
Documentation/devicetree/bindings/power/opp.txt | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
index 74499e5..4df5cca 100644
--- a/Documentation/devicetree/bindings/power/opp.txt
+++ b/Documentation/devicetree/bindings/power/opp.txt
@@ -10,6 +10,10 @@ Properties:
freq: clock frequency in kHz
vol: voltage in microvolt
+Optional Properties:
+- boost-opp: Similar to "operating-points" property but usable only in
+ optional boost operating modes.
+
Examples:
cpu@0 {
@@ -22,4 +26,9 @@ cpu@0 {
396000 950000
198000 850000
>;
+ boost-opp = <
+ /* kHz uV */
+ 1500000 1350000
+ 1400000 1285000
+ >;
};
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/2] PM / OPP: Add support for 'boost' mode OPP
From: Thomas Abraham @ 2014-02-04 9:41 UTC (permalink / raw)
To: linux-pm, devicetree, linux-arm-kernel
Cc: rjw, linux-samsung-soc, kgene.kim, t.figa, l.majewski,
viresh.kumar, thomas.ab, Nishanth Menon
In-Reply-To: <1391506890-7335-1-git-send-email-thomas.ab@samsung.com>
From: Thomas Abraham <thomas.ab@samsung.com>
Commit 6f19efc0 ("cpufreq: Add boost frequency support in core") adds
support for CPU boost mode. This patch adds support for finding available
boost OPPs from device tree and marking them as usable in boost mode.
Cc: Nishanth Menon <nm@ti.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
---
drivers/base/power/opp.c | 69 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 56 insertions(+), 13 deletions(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 2553867..de4d52d 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -62,6 +62,7 @@ struct dev_pm_opp {
struct list_head node;
bool available;
+ bool boost;
unsigned long rate;
unsigned long u_volt;
@@ -380,10 +381,12 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev,
EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);
/**
- * dev_pm_opp_add() - Add an OPP table from a table definitions
+ * dev_pm_opp_add_flags() - Add an OPP to device OPP list with flags
* @dev: device for which we do this operation
* @freq: Frequency in Hz for this OPP
* @u_volt: Voltage in uVolts for this OPP
+ * @available: initial availability of the OPP with adding it to the list.
+ * @boost: availability of this opp in controller's boost operating mode.
*
* This function adds an opp definition to the opp list and returns status.
* The opp is made available by default and it can be controlled using
@@ -395,7 +398,8 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);
* that this function is *NOT* called under RCU protection or in contexts where
* mutex cannot be locked.
*/
-int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
+static int dev_pm_opp_add_flags(struct device *dev, unsigned long freq,
+ unsigned long u_volt, bool available, bool boost)
{
struct device_opp *dev_opp = NULL;
struct dev_pm_opp *opp, *new_opp;
@@ -441,7 +445,8 @@ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
new_opp->dev_opp = dev_opp;
new_opp->rate = freq;
new_opp->u_volt = u_volt;
- new_opp->available = true;
+ new_opp->available = available;
+ new_opp->boost = boost;
/* Insert new OPP in order of increasing frequency */
head = &dev_opp->opp_list;
@@ -462,6 +467,27 @@ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ADD, new_opp);
return 0;
}
+
+/**
+ * dev_pm_opp_add() - Add an OPP table from a table definitions
+ * @dev: device for which we do this operation
+ * @freq: Frequency in Hz for this OPP
+ * @u_volt: Voltage in uVolts for this OPP
+ *
+ * This function adds an opp definition to the opp list and returns status.
+ * The opp is made available by default and it can be controlled using
+ * dev_pm_opp_enable/disable functions.
+ *
+ * Locking: The internal device_opp and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
+{
+ return dev_pm_opp_add_flags(dev, freq, u_volt, true, false);
+}
EXPORT_SYMBOL_GPL(dev_pm_opp_add);
/**
@@ -651,7 +677,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
list_for_each_entry(opp, &dev_opp->opp_list, node) {
if (opp->available) {
- freq_table[i].driver_data = i;
+ freq_table[i].driver_data =
+ opp->boost ? CPUFREQ_BOOST_FREQ : i;
freq_table[i].frequency = opp->rate / 1000;
i++;
}
@@ -701,19 +728,14 @@ struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev)
}
#ifdef CONFIG_OF
-/**
- * of_init_opp_table() - Initialize opp table from device tree
- * @dev: device pointer used to lookup device OPPs.
- *
- * Register the initial OPP table with the OPP library for given device.
- */
-int of_init_opp_table(struct device *dev)
+static int of_parse_opp_table(struct device *dev, const char *prop_name,
+ bool boost)
{
const struct property *prop;
const __be32 *val;
int nr;
- prop = of_find_property(dev->of_node, "operating-points", NULL);
+ prop = of_find_property(dev->of_node, prop_name, NULL);
if (!prop)
return -ENODEV;
if (!prop->value)
@@ -734,7 +756,7 @@ int of_init_opp_table(struct device *dev)
unsigned long freq = be32_to_cpup(val++) * 1000;
unsigned long volt = be32_to_cpup(val++);
- if (dev_pm_opp_add(dev, freq, volt)) {
+ if (dev_pm_opp_add_flags(dev, freq, volt, true, boost)) {
dev_warn(dev, "%s: Failed to add OPP %ld\n",
__func__, freq);
continue;
@@ -744,5 +766,26 @@ int of_init_opp_table(struct device *dev)
return 0;
}
+
+/**
+ * of_init_opp_table() - Initialize opp table from device tree
+ * @dev: device pointer used to lookup device OPPs.
+ *
+ * Register the initial OPP table with the OPP library for given device.
+ * Additional "boost" operating points of the controller, if any, are
+ * specified with "boost-opp" property.
+ */
+int of_init_opp_table(struct device *dev)
+{
+ int ret;
+
+ ret = of_parse_opp_table(dev, "operating-points", false);
+ if (!ret) {
+ ret = of_parse_opp_table(dev, "boost-opp", true);
+ if (ret == -ENODEV)
+ ret = 0;
+ }
+ return ret;
+}
EXPORT_SYMBOL_GPL(of_init_opp_table);
#endif
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/2] Add device tree based lookup of boost mode OPPs
From: Thomas Abraham @ 2014-02-04 9:41 UTC (permalink / raw)
To: linux-pm, devicetree, linux-arm-kernel
Cc: rjw, linux-samsung-soc, kgene.kim, t.figa, l.majewski,
viresh.kumar, thomas.ab
Commit 6f19efc0 ("cpufreq: Add boost frequency support in core") adds
support for CPU boost mode for CPUfreq drivers. To use the new boost
mode, CPUfreq drivers have to specify the boost mode frequency and
voltage within the CPUfreq driver, which is the case for Exynos4x12
CPUfreq driver.
But for CPUfreq drivers which obtain the OPPs from cpus node, this
patch series adds support to specify boost mode OPPs in dt node. This
requirement came up when Lukasz pointed out the regression caused by
the Exynos CPUfreq driver consolidation patches.
Thomas Abraham (2):
PM / OPP: Add support for 'boost' mode OPP
Documentation: devicetree: Add boost-opp binding to list boost mode OPPs
Documentation/devicetree/bindings/power/opp.txt | 9 +++
drivers/base/power/opp.c | 69 ++++++++++++++++++-----
2 files changed, 65 insertions(+), 13 deletions(-)
--
1.7.10.4
^ permalink raw reply
* Re: [PATCH v2 2/5] clk: sunxi: Add USB clock register defintions
From: Maxime Ripard @ 2014-02-04 9:40 UTC (permalink / raw)
To: Hans de Goede
Cc: Emilio Lopez, Mike Turquette, Philipp Zabel,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Roman Byshko
In-Reply-To: <52E77FCD.5050701-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2289 bytes --]
Hi Hans,
On Tue, Jan 28, 2014 at 11:00:45AM +0100, Hans de Goede wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi,
>
> On 01/28/2014 10:44 AM, Maxime Ripard wrote:
> > On Mon, Jan 27, 2014 at 03:54:14PM +0100, Hans de Goede wrote:
> >>>> "allwinner,sun5i-a13-usb-gates-clk" - for usb gates + resets on A13
> >>>
> >>> Maybe we can just remove the gates from there? Even though they
> >>> are gates, they are also (a bit) more than that.
> >>
> >> To be clear you mean s/usb-gates-clk/usb-clk/ right ?
> >
> > Yep, exactly
> >
> >>> I guess that means that we will have the OHCI0 gate declared
> >>> with <&...-gates-clk 6>, while it's actually the first gate for
> >>> this clock?
> >>
> >> Correct.
> >>
> >>> Maybe introducing an offset field in the gates_data would be a
> >>> good idea, so that we always start from indexing the gates from
> >>> 0 in the DT?
> >>
> >> Well for the other "gates" type clks we also have holes in the
> >> range, and we always refer to the clk with the bit number in the
> >> reg as the clock-cell value.
> >
> > Yes, we have holes, but I see two majors differences here: - the
> > other gates are just gates, while the usb clocks are a bit more
> > than that.
>
> The usb-clk registers contain more then that, but the bits we are
> talking about now are gates.
>
> > - the other gates' gating bits thus all start at bit 0, while
> > - here, since it's kind of a "mixed" clock, the gating bits start
> > - at bit 6 (on the A20 at least)
>
> Right, still I believe that the consistent thing to do is keeping
> the bit-number for the bit in the register controlling the gate as
> the specifier. When adding new dts entries / reviewing existing
> ones I'm used to matching the specifier to the bit-nr in the
> data-sheet, I think making things different just for this one
> register is counter productive.
And if you turn it the other way around, it would be inconsistent that
all gates indices start at 0, and we would start at 6 here :)
Plus, this clock is already a special case, since it's the only gate
that is more than just a gate so far.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/8] clk: sunxi: Add Allwinner A20/A31 GMAC clock unit
From: Maxime Ripard @ 2014-02-04 9:27 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Emilio Lopez, Mike Turquette, linux-arm-kernel, linux-sunxi,
devicetree, linux-kernel
In-Reply-To: <CAGb2v64+qiBqYtWyc23b6p4aWyWj7a9K1qveTsW7ZGh0ti8_Wg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3331 bytes --]
On Tue, Feb 04, 2014 at 10:43:33AM +0800, Chen-Yu Tsai wrote:
> Hi,
>
> On Tue, Feb 4, 2014 at 3:31 AM, Maxime Ripard
> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > Hi,
> >
> > On Mon, Feb 03, 2014 at 11:32:19AM +0800, Chen-Yu Tsai wrote:
> >> The Allwinner A20/A31 clock module controls the transmit clock source
> >> and interface type of the GMAC ethernet controller. Model this as
> >> a single clock for GMAC drivers to use.
> >>
> >> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
> >> ---
> >> Documentation/devicetree/bindings/clock/sunxi.txt | 26 +++++++
> >> drivers/clk/sunxi/clk-sunxi.c | 83 +++++++++++++++++++++++
> >> 2 files changed, 109 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> >> index 0cf679b..f43b4c0 100644
> >> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> >> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> >> @@ -37,6 +37,7 @@ Required properties:
> >> "allwinner,sun6i-a31-apb2-gates-clk" - for the APB2 gates on A31
> >> "allwinner,sun4i-mod0-clk" - for the module 0 family of clocks
> >> "allwinner,sun7i-a20-out-clk" - for the external output clocks
> >> + "allwinner,sun7i-a20-gmac-clk" - for the GMAC clock module on A20/A31
> >>
> >> Required properties for all clocks:
> >> - reg : shall be the control register address for the clock.
> >> @@ -50,6 +51,9 @@ Required properties for all clocks:
> >> If the clock module only has one output, the name shall be the
> >> module name.
> >>
>
>
> >> +For "allwinner,sun7i-a20-gmac-clk", the parent clocks shall be fixed rate
> >> +dummy clocks at 25 MHz and 125 MHz, respectively. See example.
> >> +
>
>
> >> Clock consumers should specify the desired clocks they use with a
> >> "clocks" phandle cell. Consumers that are using a gated clock should
> >> provide an additional ID in their clock property. This ID is the
> >> @@ -96,3 +100,25 @@ mmc0_clk: clk@01c20088 {
> >> clocks = <&osc24M>, <&pll6 1>, <&pll5 1>;
> >> clock-output-names = "mmc0";
> >> };
> >> +
> >> +mii_phy_tx_clk: clk@2 {
> >> + #clock-cells = <0>;
> >> + compatible = "fixed-clock";
> >> + clock-frequency = <25000000>;
> >> + clock-output-names = "mii_phy_tx";
> >> +};
> >> +
> >> +gmac_int_tx_clk: clk@3 {
> >> + #clock-cells = <0>;
> >> + compatible = "fixed-clock";
> >> + clock-frequency = <125000000>;
> >> + clock-output-names = "gmac_int_tx";
> >> +};
> >> +
> >> +gmac_clk: clk@01c20164 {
> >> + #clock-cells = <0>;
> >> + compatible = "allwinner,sun7i-a20-gmac-clk";
> >> + reg = <0x01c20164 0x4>;
> >> + clocks = <&mii_phy_tx_clk>, <&gmac_int_tx_clk>;
> >
> > You should also document in which order you expect the parents to
> > be. Or it will probably be easier to just use clock-names here.
>
> Is it not clear from the "Required properties" section above?
I'd make it clearer. But again, using clock-names would avoid any
ambiguity, and be more flexible.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 2/8] ARM: dts: sun7i: Add GMAC clock node to sun7i DTSI
From: Maxime Ripard @ 2014-02-04 9:13 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Emilio Lopez, Mike Turquette, linux-arm-kernel, linux-sunxi,
devicetree, linux-kernel
In-Reply-To: <CAGb2v660qoKDC6SKjoD29b-r5y9OwkeKoWZ9PWBF5tC5PgeEYQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2512 bytes --]
On Tue, Feb 04, 2014 at 11:06:24AM +0800, Chen-Yu Tsai wrote:
> On Tue, Feb 4, 2014 at 3:34 AM, Maxime Ripard
> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > On Mon, Feb 03, 2014 at 11:32:20AM +0800, Chen-Yu Tsai wrote:
> >> The GMAC uses 1 of 2 sources for its transmit clock, depending on the
> >> PHY interface mode. Add both sources as dummy clocks, and as parents
> >> to the GMAC clock node.
> >>
> >> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
> >> ---
> >> arch/arm/boot/dts/sun7i-a20.dtsi | 28 ++++++++++++++++++++++++++++
> >> 1 file changed, 28 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> >> index 1595e9a..fc7f470 100644
> >> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> >> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> >> @@ -314,6 +314,34 @@
> >> };
> >>
> >> /*
> >> + * The following two are dummy clocks, placeholders used
> >> + * on gmac_tx clock. The actual frequency and availability
> >> + * depends on the external PHY, operation mode and link
> >> + * speed.
> >> + */
> >
> > If it depends on the external PHY, I guess that means it also depends
> > on the board, right? Or is the GMAC supposed to always have that clock
> > running at 25MHz, no matter what PHY is connected to it?
>
> What I meant in the comment is that we cannot control the actual clock
> rate of the TX clock. We can only select the source, and this is what
> gmac_tx clock does. It is just a clock mux. The 125MHz and 25MHz clock
> rates are used by the clk_set_rate in the stmmac glue layer to do
> auto-reparenting.
>
> The board dependent factor is what _type_ of PHY it is using, i.e.
> MII, GMII, or RGMII. If it's MII, the PHY should provide the clock.
> If it's RGMII, the internal clock would be used. GMII is a mix of
> both. The actual clock rate depends on the link speed.
>
> I should rephrase the comment along the lines of:
>
> The following two are dummy clocks, placeholders used in the gmac_tx
> clock. The gmac driver will choose one parent depending on the PHY
> interface mode, using clk_set_rate auto-reparenting.
> The actual TX clock rate is not controlled by the gmac_tx clock.
Ok, thanks for the clarification.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Maxime Ripard @ 2014-02-04 9:09 UTC (permalink / raw)
To: Mark Brown
Cc: Mike Turquette, Emilio Lopez, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kevin.z.m.zh-Re5JQEeQqe8AvxtiuMwx3w,
sunny-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf,
shuge-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf,
zhuzhenhua-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf
In-Reply-To: <20140204002110.GP22609-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2355 bytes --]
Hi Mark,
On Tue, Feb 04, 2014 at 12:21:10AM +0000, Mark Brown wrote:
> On Fri, Jan 31, 2014 at 11:47:04PM +0100, Maxime Ripard wrote:
> > On Fri, Jan 31, 2014 at 12:48:09PM +0000, Mark Brown wrote:
> > > On Fri, Jan 31, 2014 at 11:55:50AM +0100, Maxime Ripard wrote:
>
> > > > + pm_runtime_enable(&pdev->dev);
> > > > + if (!pm_runtime_enabled(&pdev->dev)) {
> > > > + ret = sun6i_spi_runtime_resume(&pdev->dev);
> > > > + if (ret) {
> > > > + dev_err(&pdev->dev, "Couldn't resume the device\n");
> > > > + return ret;
> > > > + }
> > > > + }
>
> > > No, as discussed don't do this - notice how other drivers aren't written
> > > this way either. Like I said leave the device powered on startup and
> > > then let it be idled by runtime PM.
>
> > Well, some SPI drivers are actually written like that (all the tegra
>
> It's not been done consistently, no - that should be fixed.
>
> > SPI drivers for example). It's not an excuse, but waking up the device
> > only to put it back in suspend right away seems kind of
>
> It isn't awesome, no. Ideally the runtime PM code would do this but
> then you couldn't ifdef the operations which as far as I can tell is the
> main thing people want from disabling it and it gets complicated for
> devices that genuinely do power up on startup so here we are.
We discussed it with Kevin on IRC, and he suggested that we move that
pm_runtime initialization to the SPI core, but I guess that would also
mean that all drivers shouldn't ifdef the operations, so that the core
can call the runtime_resume callback directly.
However, I don't really get why any driver should be doing so, since
you still need these functions to at least to the device
suspend/resume in the probe/remove, and you don't really want to
duplicate the code.
Right now, about half of the SPI drivers using auto_runtime_pm are
using a ifdef, the other half is not.
> > inefficient. Plus, the pm_runtime_idle callback you suggested are
> > actually calling runtime_idle, while we want to call runtime_suspend.
>
> Yeah, I didn't actually check if I was looking at the right call there.
I was actually wrong, it does so in its very last line.
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] arm64: Add architecture support for PCI
From: Arnd Bergmann @ 2014-02-04 9:01 UTC (permalink / raw)
To: Rob Herring
Cc: linaro-kernel@lists.linaro.org, devicetree@vger.kernel.org,
Catalin Marinas, Liviu Dudau, LKML, linux-pci, Bjorn Helgaas,
LAKML
In-Reply-To: <CAL_JsqL+3S8w=_Fb-t0rVcq0sDiPSDM6nqn3mR2DHKw=VTffsg@mail.gmail.com>
On Monday 03 February 2014 17:07:48 Rob Herring wrote:
> On Mon, Feb 3, 2014 at 2:05 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> You might want to re-read the SBSA. Unless ARM provides an IP block or
> there is some other standard such as EHCI or AHCI, there is no generic
> implementation. You only have to go look at the Linux EHCI or AHCI
> drivers and see how meaningless and inadequate "use EHCI" is. For PCI,
> the text is so brief in the SBSA there will be no way PCI is going to
> just work given all the variations of root complexes, bridges, address
> windows, etc. we typically see on ARM platforms. I could be wrong and
> some AML magic will solve all the problems.
I don't think you need any AML, and SBSA seems to cover the PCI case
just fine, though I have to agree that the EHCI/AHCI/xHCI part is
rather ambiguous. What the existing PCI host controller drivers do is
essentially:
1. provide a config space access method
2. set up the I/O and memory address windows
3. set up clocks, PHYs etc
4. work around any deviations from the PCI standard
5. provide an MSI/MSI-X controller
For all I can tell, any SBSA compliant system should handle
those four like this:
1. config space is ECAM compliant, we only need to pass the
location in DT. (SBSA 8.1)
2. all address windows are set up by the boot loader, we only
need to know the location (IMHO this should be the
preferred way to do things regardless of SBSA).
3. any external hardware dependencies are set up statically
by the boot loader and are operational as we enter the
kernel.
4. deviations from PCI are not allowed (SBSA 8.8)
5. MSI has to be handled by GICv3 (SBSA 8.3.2)
So I definitely expect SBSA compliant systems to work fine with a
very simple generic PCI host bridge driver (which is likely what
Liviu has implemented and waiting for approval to submit).
The more important question is what systems will actually be
compliant with the above. X-Gene manages to get all five wrong,
for instance, and so would any system that reuses the existing
PCI hardware (alphabetically: exynos, imx6, mvebu, rcar-gen2,
tegra, designware), although points 2 and 3 are a question of
the boot loader, and it's possible that the designware based ones
get point 4 right.
Arnd
^ permalink raw reply
* Re: [PATCH 1/2] clocksource: sunxi: Add new compatibles
From: Maxime Ripard @ 2014-02-04 8:45 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx-hfZtesqFncYOwBW4kG4KsQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <52EFF96A.1020302-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]
Hi,
On Mon, Feb 03, 2014 at 09:17:46PM +0100, Daniel Lezcano wrote:
> On 02/03/2014 08:45 PM, Maxime Ripard wrote:
> >Hi Daniel,
> >
> >(Adding DT mailing-list in CC)
> >
> >On Mon, Feb 03, 2014 at 05:36:03PM +0100, Daniel Lezcano wrote:
> >>On 02/02/2014 02:37 PM, Maxime Ripard wrote:
> >>>The Allwinner A10 compatibles were following a slightly different compatible
> >>>patterns than the rest of the SoCs for historical reasons. Add compatibles
> >>>matching the other pattern to the timer driver for consistency, and keep the
> >>>older one for backward compatibility.
> >>
> >>Hi Maxime,
> >>
> >>is it really needed to keep the old pattern ?
> >
> >We agreed during the ARM Kernel Summit to consider the DT as a stable
> >ABI.
> >
> >While I'd be ok with removing the older ones, that also means that we
> >would break the boot of newer kernels with older DT, so yes, we
> >actually need to keep the old compatibles.
>
> Thanks for the clarification.
>
> So these old compatibles will stay there 'ad vitam aeternam', right ?
Except for what Rob told, yep, that was my feeling, but Gregory and I
seem to have a different interpretation of this rule :)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] arm64: Add architecture support for PCI
From: Arnd Bergmann @ 2014-02-04 8:44 UTC (permalink / raw)
To: linaro-kernel
Cc: Liviu Dudau, devicetree@vger.kernel.org, Catalin Marinas, LKML,
linux-pci, Bjorn Helgaas, LAKML
In-Reply-To: <20140203213658.GA24036@e106497-lin.cambridge.arm.com>
On Monday 03 February 2014 21:36:58 Liviu Dudau wrote:
> On Mon, Feb 03, 2014 at 08:05:56PM +0000, Arnd Bergmann wrote:
> > On Monday 03 February 2014 19:18:38 Liviu Dudau wrote:
> > > So ... defining it should mean no legacy ISA devices, right?
> >
> > I would read that comment as referring to systems that don't have
> > any I/O space. If you have PCI, you can by definition have ISA
> > compatible devices behind a bridge. A typical example would be
> > a VGA card that supports the 03c0-03df port range.
>
> But if you have PCI and don't want to support ISA do you have /dev/port? I guess yes.
Right, that is my interpretation. You could still have a tool
that tries to poke /dev/port from user space for any I/O BAR
the same way you'd use /dev/mem for memory BARs of PCI devices.
It's discouraged, but it's often the easiest solution for
a quick hack, and I'd expect tools to use this.
> > > > > #define IO_SPACE_LIMIT 0xffff
> > > >
> > > > You probably want to increase this a bit, to allow multiple host bridges
> > > > to have their own I/O space.
> > >
> > > OK, but to what size?
> >
> > 2 MB was a compromise on arm32 to allow up to 32 PCI host bridges but not
> > take up too much virtual space. On arm64 it should be at least as big.
> > Could be more than that, although I don't see a reason why it should be,
> > unless we expect to see systems with tons of host bridges, or buses
> > that exceed 64KB of I/O space.
>
> I will increase the size to 2MB for v2.
Thinking about this some more, I'd go a little higher. In case of
pci_mv, we already register a 1MB region for one logical host
(which has multiple I/O spaces behind an emulated bridge), so
going to 16MB or more would let us handle multiple 1MB windows
for some amount of future proofing.
Maybe Catalin can assign us some virtual address space to use,
with the constraints that it should be 16MB or more in an
area that doesn't require additional kernel page table pages.
> > > > > +#define ioport_map(port, nr) (PCI_IOBASE + ((port) & IO_SPACE_LIMIT))
> > > > > +#define ioport_unmap(addr)
> > > >
> > > > inline functions?
> > >
> > > Will do, thanks!
> >
> > I suppose you can actually use the generic implementation from
> > asm-generic/io.h, and fix it by using the definition you have
> > above, since it's currently broken.
>
> Not exactly broken, but it makes the assumption that your IO space starts at
> physical address zero and you have not remapped it. It does guard the
> definition with #ifndef CONFIG_GENERIC_IOMAP after all, so it does not
> expect to be generic :)
Well, I/O space never starts at physical zero in reality, so it is
broken in practice. The CONFIG_GENERIC_IOMAP option tries to solve
the problem of I/O spaces that are not memory mapped, which is
actually quite rare (x86, ia64, some rare powerpc bridges, and possibly
Alpha). The norm is that if you have I/O space, it is memory mapped
and you don't need GENERIC_IOMAP. I think most of the architectures
selecting GENERIC_IOMAP have incorrectly copied that from x86.
> > > > > diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
> > > > > new file mode 100644
> > > > > index 0000000..dd084a3
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/include/asm/pci.h
> > > > > @@ -0,0 +1,35 @@
> > > > > +#ifndef __ASM_PCI_H
> > > > > +#define __ASM_PCI_H
> > > > > +#ifdef __KERNEL__
> > > > > +
> > > > > +#include <linux/types.h>
> > > > > +#include <linux/slab.h>
> > > > > +#include <linux/dma-mapping.h>
> > > > > +
> > > > > +#include <asm/io.h>
> > > > > +#include <asm-generic/pci-bridge.h>
> > > > > +#include <asm-generic/pci-dma-compat.h>
> > > > > +
> > > > > +#define PCIBIOS_MIN_IO 0
> > > > > +#define PCIBIOS_MIN_MEM 0
> > > >
> > > > PCIBIOS_MIN_IO is normally set to 0x1000, to stay out of the ISA range.
> > >
> > > :) No ISA support! (Die ISA, die!!)
> >
> > If only it were that easy.
>
> Lets try! :)
>
> I wonder how many active devices that have an ISA slot are still supported
> by mainline kernel.
This is missing the point, but any architecture that has a PCI
slot can have an ISA device behind a bridge like this:
http://www.altera.com/products/ip/iup/pci/m-eur-pci-to-isa.html
The real reason is that a lot of PCI cards for practical reasons
expose some non-relocatable memory and I/O BARs in ISA-compatible
locations. Looking at /proc/ioports on my PC, I can spot a couple
of things that may well show up on any ARM machine:
0000-03af : PCI Bus 0000:00
02f8-02ff : serial
03c0-03df : PCI Bus 0000:40
03c0-03df : PCI Bus 0000:00
03c0-03df : vga+
03e0-0cf7 : PCI Bus 0000:00
03e8-03ef : serial
03f8-03ff : serial
0b00-0b0f : pnp 00:08
0b00-0b07 : piix4_smbus
0b20-0b3f : pnp 00:08
0b20-0b27 : piix4_smbus
0ca2-0ca3 : pnp 00:08
0ca2-0ca2 : ipmi_si
0ca3-0ca3 : ipmi_si
0cf8-0cff : PCI conf1
Nothing wrong with the above. Now, it's also possible that
someone decides to build an ARM server by using a PC south
bridge with integrated legacy PC peripherals, such as these:
0000-03af : PCI Bus 0000:00
0000-001f : dma1
0020-0021 : pic1
0040-0043 : timer0
0050-0053 : timer1
0060-0060 : keyboard
0064-0064 : keyboard
0070-0071 : rtc0
0080-008f : dma page reg
00a0-00a1 : pic2
00b0-00b0 : APEI ERST
00c0-00df : dma2
00f0-00ff : fpu
There is some hope that it won't happen, but these things
exist and come with a regular PCIe front-end bus in some
cases.
Finally, there is the LPC bus, which can give you additional
ISAPnP compatible devices.
> I will update PCIBIOS_MIN_xxxx to match arch/arm for v2.
Ok.
> > > > > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > > > > new file mode 100644
> > > > > index 0000000..7b652cf
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/kernel/pci.c
> > > > > @@ -0,0 +1,112 @@
> > > >
> > > > None of this looks really arm64 specific, nor should it be. I think
> > > > we should try a little harder to move this as a default implementation
> > > > into common code, even if we start out by having all architectures
> > > > override it.
> > >
> > > Agree. This is the RFC version. I didn't dare to post a patch with fixes
> > > for all architectures. :)
> >
> > No need to change the other architectures. You can make it opt-in for
> > now and just put the code into a common location.
> >
> > An interesting question however is what the transition plan is to
> > have the code shared between arm32 and arm64: We will certainly need
> > to share at least the dw-pcie and the generic SBSA compliant pci
> > implementation.
>
> My vote would be for updating the host controllers to the new API and
> to offer the CONFIG option to choose between arch APIs. The alternative
> is to use the existing API to wrap the generic implementation.
The problem with an either/or CONFIG option is that it breaks
multiplatform support. A lot of the arm32 PCI implementations
are only used on platforms that are not multiplatform enabled
though, so we could get away by requiring all multiplatform
configurations to use the new API.
> My main concern with the existing API is the requirement to have a subsys_initcall
> in your host bridge or mach code, due to the way the initialisation is done (you
> need the DT code to probe your driver, but you cannot start the scanning of the
> PCI bus until the arch code is initialised, so it gets deferred via
> subsys_initcall when it calls pci_common_init). I bet that if one tries to
> instantiate a Tegra PCI host bridge controller on a Marvell platform things will
> break pretty quickly (random example here).
I'm not following here. All the new host controller drivers should
be platform drivers that only bind to the host devices in DT
that are present. Both mvebu and tegra use a normal "module_platform_driver"
for initialization, not a "subsys_initcall".
> > Something like this (coded in mail client, don't try to compile):
> >
> > #define IO_SPACE_PAGES (IO_SPACE_LIMIT + 1) / PAGE_SIZE)
> > static DECLARE_BITMAP(pci_iospace, IO_SPACE_PAGES);
> > unsigned long pci_ioremap_io(const struct resource *bus, const struct resource phys)
> > {
> > unsigned long start, len, virt_start;
> > int error;
> >
> > /* use logical address == bus address if possible */
> > start = bus->start / PAGE_SIZE;
> > if (start > IO_SPACE_LIMIT / PAGE_SIZE)
> > start = 0;
> >
> > /*
> > * try finding free space for the whole size first,
> > * fall back to 64K if not available
> > */
> > len = min(resource_size(bus), resource_size(phys);
> > start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
> > start, len / PAGE_SIZE, 0);
> > if (start == IO_SPACE_PAGES && len > SZ_64K)
> > len = SZ_64K;
> > start = 0;
> > start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
> > start, len / PAGE_SIZE, 0);
> > }
> >
> > /* no 64K area found */
> > if (start == IO_SPACE_PAGES)
> > return -ENOMEM;
> >
> > /* ioremap physical aperture to virtual aperture */
> > virt_start = start * PAGE_SIZE + (unsigned long)PCI_IOBASE;
> > error = ioremap_page_range(virt_start, virt_start + len,
> > phys->start, __pgprot(PROT_DEVICE_nGnRE));
> > if (error)
> > return error;
> >
> > bitmap_set(start, len / PAGE_SIZE);
> >
> > /* return io_offset */
> > return start * PAGE_SIZE - bus->start;
> > }
> > EXPORT_SYMBOL_GPL(pci_ioremap_io);
> >
> > Arnd
> >
>
> I see. I need to think how this will change the existing code. Current users
> of pci_ioremap_io ask for multiples of SZ_64K offsets regardless of the
> actual need.
Right. I guess we can support both interfaces on ARM32 for the forseeable
future (renaming the new one) and just change the existing implementation
to update the bitmap. Any cross-platform host controller driver would
have to use the new interface however.
Arnd
^ permalink raw reply
* Re: [PATCH 1/2] clocksource: sunxi: Add new compatibles
From: Maxime Ripard @ 2014-02-04 8:41 UTC (permalink / raw)
To: Rob Herring
Cc: Daniel Lezcano, Thomas Gleixner,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqJuSS1Hh8Fb26PMKBB_uKmLuiqnZJjz0zvt1Ux7P8cCzw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1627 bytes --]
Hi Rob,
On Mon, Feb 03, 2014 at 02:11:28PM -0600, Rob Herring wrote:
> On Mon, Feb 3, 2014 at 1:45 PM, Maxime Ripard
> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > Hi Daniel,
> >
> > (Adding DT mailing-list in CC)
> >
> > On Mon, Feb 03, 2014 at 05:36:03PM +0100, Daniel Lezcano wrote:
> >> On 02/02/2014 02:37 PM, Maxime Ripard wrote:
> >> >The Allwinner A10 compatibles were following a slightly different compatible
> >> >patterns than the rest of the SoCs for historical reasons. Add compatibles
> >> >matching the other pattern to the timer driver for consistency, and keep the
> >> >older one for backward compatibility.
> >>
> >> Hi Maxime,
> >>
> >> is it really needed to keep the old pattern ?
> >
> > We agreed during the ARM Kernel Summit to consider the DT as a stable
> > ABI.
> >
> > While I'd be ok with removing the older ones, that also means that we
> > would break the boot of newer kernels with older DT, so yes, we
> > actually need to keep the old compatibles.
>
> It all depends if that would really cause problems for a given
> platform. So if Allwinner DT support is a moving target, then changing
> is probably okay. For example, if anyone using the platform is going
> to need to update their DTB to add more nodes to get various features
> anyway, then breaking it is not all that important.
We keep adding new stuff to the DT, yes, so I guess we can be
considered a moving target. Thanks for your input!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v10 0/7] mmc: omap_hsmmc: pbias dt and cleanup
From: Balaji T K @ 2014-02-04 8:32 UTC (permalink / raw)
To: Balaji T K, chris, tony
Cc: linux-omap, bcousson, devicetree, linux-mmc, broonie
In-Reply-To: <1389627375-9107-1-git-send-email-balajitk@ti.com>
On Monday 13 January 2014 09:06 PM, Balaji T K wrote:
> Few cleanups to reduce code indent,
> Add pbias_regulator support and adapt omap_hsmmc to use pbias regulator
> to configure required voltage on mmc1 pad(SD card) i/o rails on OMAP SoCs.
>
Hi Tony,
Considering the dependencies with regulator, defconfig, mmc and device tree,
Is it possible for you to pick this series.
Thanks and Regards,
Balaji T K
> Balaji T K (7):
> mmc: omap_hsmmc: use devm_regulator API
> mmc: omap_hsmmc: handle vcc and vcc_aux independently
> regulator: add pbias regulator support
> mmc: omap_hsmmc: adapt hsmmc to use pbias regulator
> ARM: dts: add pbias dt node
> ARM: OMAP: enable SYSCON and REGULATOR_PBIAS in omap2plus_defconfig
> mmc: omap_hsmmc: remove pbias workaround
>
> .../bindings/regulator/pbias-regulator.txt | 27 ++
> arch/arm/boot/dts/dra7.dtsi | 17 ++
> arch/arm/boot/dts/omap2430.dtsi | 17 ++
> arch/arm/boot/dts/omap3.dtsi | 17 ++
> arch/arm/boot/dts/omap4.dtsi | 17 ++
> arch/arm/boot/dts/omap5.dtsi | 17 ++
> arch/arm/configs/omap2plus_defconfig | 2 +
> drivers/mmc/host/omap_hsmmc.c | 111 +++++----
> drivers/regulator/Kconfig | 9 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/pbias-regulator.c | 255 ++++++++++++++++++++
> 11 files changed, 441 insertions(+), 49 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/regulator/pbias-regulator.txt
> create mode 100644 drivers/regulator/pbias-regulator.c
>
^ permalink raw reply
* Re: [PATCH 1/2] clocksource: sunxi: Add new compatibles
From: Gregory CLEMENT @ 2014-02-04 8:13 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Maxime Ripard, devicetree-u79uwXL29TY76Z2rM5mHXA,
tglx-hfZtesqFncYOwBW4kG4KsQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <52EFF96A.1020302-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Hi Daniel,
On 03/02/2014 21:17, Daniel Lezcano wrote:
> On 02/03/2014 08:45 PM, Maxime Ripard wrote:
>> Hi Daniel,
>>
>> (Adding DT mailing-list in CC)
>>
>> On Mon, Feb 03, 2014 at 05:36:03PM +0100, Daniel Lezcano wrote:
>>> On 02/02/2014 02:37 PM, Maxime Ripard wrote:
>>>> The Allwinner A10 compatibles were following a slightly different compatible
>>>> patterns than the rest of the SoCs for historical reasons. Add compatibles
>>>> matching the other pattern to the timer driver for consistency, and keep the
>>>> older one for backward compatibility.
>>>
>>> Hi Maxime,
>>>
>>> is it really needed to keep the old pattern ?
>>
>> We agreed during the ARM Kernel Summit to consider the DT as a stable
>> ABI.
>>
>> While I'd be ok with removing the older ones, that also means that we
>> would break the boot of newer kernels with older DT, so yes, we
>> actually need to keep the old compatibles.
>
> Thanks for the clarification.
>
> So these old compatibles will stay there 'ad vitam aeternam', right ?
From what I have understood during the ARM Kernel Summit, it was
acceptable to remove them after a few release.
Gregory
>
>
>>>> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>> ---
>>>> Documentation/devicetree/bindings/timer/allwinner,sun4i-timer.txt | 5 +++--
>>>> drivers/clocksource/sun4i_timer.c | 4 ++++
>>>> 2 files changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/timer/allwinner,sun4i-timer.txt b/Documentation/devicetree/bindings/timer/allwinner,sun4i-timer.txt
>>>> index 48aeb78..d9e35ae 100644
>>>> --- a/Documentation/devicetree/bindings/timer/allwinner,sun4i-timer.txt
>>>> +++ b/Documentation/devicetree/bindings/timer/allwinner,sun4i-timer.txt
>>>> @@ -2,7 +2,8 @@ Allwinner A1X SoCs Timer Controller
>>>>
>>>> Required properties:
>>>>
>>>> -- compatible : should be "allwinner,sun4i-timer"
>>>> +- compatible : should be "allwinner,sun4i-a10-timer"
>>>> + (Deprecated "allwinner,sun4i-timer")
>>>> - reg : Specifies base physical address and size of the registers.
>>>> - interrupts : The interrupt of the first timer
>>>> - clocks: phandle to the source clock (usually a 24 MHz fixed clock)
>>>> @@ -10,7 +11,7 @@ Required properties:
>>>> Example:
>>>>
>>>> timer {
>>>> - compatible = "allwinner,sun4i-timer";
>>>> + compatible = "allwinner,sun4i-a10-timer";
>>>> reg = <0x01c20c00 0x400>;
>>>> interrupts = <22>;
>>>> clocks = <&osc>;
>>>> diff --git a/drivers/clocksource/sun4i_timer.c b/drivers/clocksource/sun4i_timer.c
>>>> index bf497af..de03895 100644
>>>> --- a/drivers/clocksource/sun4i_timer.c
>>>> +++ b/drivers/clocksource/sun4i_timer.c
>>>> @@ -196,5 +196,9 @@ static void __init sun4i_timer_init(struct device_node *node)
>>>> clockevents_config_and_register(&sun4i_clockevent, rate,
>>>> TIMER_SYNC_TICKS, 0xffffffff);
>>>> }
>>>> +CLOCKSOURCE_OF_DECLARE(sun4i, "allwinner,sun4i-a10-timer",
>>>> + sun4i_timer_init);
>>>> +
>>>> +/* Deprecated */
>>>> CLOCKSOURCE_OF_DECLARE(sun4i, "allwinner,sun4i-timer",
>>>> sun4i_timer_init);
>>>>
>>>
>>>
>>> --
>>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>>
>>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
>>> <http://twitter.com/#!/linaroorg> Twitter |
>>> <http://www.linaro.org/linaro-blog/> Blog
>>>
>>
>
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] spi: rspi: fix build error when CONFIG_OF is not set
From: Shimoda, Yoshihiro @ 2014-02-04 5:56 UTC (permalink / raw)
To: Geert Uytterhoeven, Grant Likely, Rob Herring
Cc: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA, SH-Linux,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Development
In-Reply-To: <alpine.DEB.2.02.1402030939520.25794-97SZ98TBZzA1xEWliksxXw@public.gmane.org>
Hi Geert-san,
(2014/02/03 17:47), Geert Uytterhoeven wrote:
> Hi Shimoda-san,
>
> On Mon, 3 Feb 2014, Shimoda, Yoshihiro wrote:
>> This patch fixes an issue that the following build error happens when
>> the CONFIG_OF is not set:
>>
>> drivers/spi/spi-rspi.c: In function 'rspi_probe':
>> drivers/spi/spi-rspi.c:1203:26: error: 'rspi_of_match' undeclared (first use in this function)
>>
>> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>> ---
>> This patch is based on the latest origin/topic/rspi branch in the spi.git.
>>
>> drivers/spi/spi-rspi.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
>> index 34ad4bc..e5cfc3d 100644
>> --- a/drivers/spi/spi-rspi.c
>> +++ b/drivers/spi/spi-rspi.c
>> @@ -1164,6 +1164,7 @@ static int rspi_parse_dt(struct device *dev, struct spi_master *master)
>> return 0;
>> }
>> #else
>> +#define rspi_of_match NULL
>> static inline int rspi_parse_dt(struct device *dev, struct spi_master *master)
>> {
>> return -EINVAL;
>> --
>> 1.7.1
>
> Thanks, obviously I missed that of_match_device() still uses the ID table
> parameter if CONFIG_OF=n :-(
>
> Below I have two alternative solutions:
> 1. Uses rspi_of_match() to nullify the ID table pointer, like is done in
> the platform_driver structure,
> 2. Fixes it at the OF subsystem level, by nullifying the ID table pointer
> inside of_match_device().
>
> If 2 is accepted, drivers don't have to care about this anymore.
>
> What do you think?
Thank you for the reply.
I think the 2nd one is a nice idea.
If I applied it without my patch, the build error disappeared.
Best regards,
Yoshihiro Shimoda
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Vinod Koul @ 2014-02-04 5:28 UTC (permalink / raw)
To: Srikanth Thokala
Cc: Lars-Peter Clausen, dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
michal.simek-gjFFaj9aHVfQT0dZR+AlfA, Grant Likely,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dmaengine-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CA+mB=1LyX0_VVXZQ-=m3gR2RBeumLrnPTUW3j+Zm1nj7a4exHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jan 31, 2014 at 12:22:52PM +0530, Srikanth Thokala wrote:
> >>> >> [...]
> >>> >>> +/**
> >>> >>> + * xilinx_vdma_device_control - Configure DMA channel of the device
> >>> >>> + * @dchan: DMA Channel pointer
> >>> >>> + * @cmd: DMA control command
> >>> >>> + * @arg: Channel configuration
> >>> >>> + *
> >>> >>> + * Return: '0' on success and failure value on error
> >>> >>> + */
> >>> >>> +static int xilinx_vdma_device_control(struct dma_chan *dchan,
> >>> >>> + enum dma_ctrl_cmd cmd, unsigned long arg)
> >>> >>> +{
> >>> >>> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
> >>> >>> +
> >>> >>> + switch (cmd) {
> >>> >>> + case DMA_TERMINATE_ALL:
> >>> >>> + xilinx_vdma_terminate_all(chan);
> >>> >>> + return 0;
> >>> >>> + case DMA_SLAVE_CONFIG:
> >>> >>> + return xilinx_vdma_slave_config(chan,
> >>> >>> + (struct xilinx_vdma_config *)arg);
> >>> >>
> >>> >> You really shouldn't be overloading the generic API with your own semantics.
> >>> >> DMA_SLAVE_CONFIG should take a dma_slave_config and nothing else.
> >>> >
> >>> > Ok. The driver needs few additional configuration from the slave
> >>> > device like Vertical
> >>> > Size, Horizontal Size, Stride etc., for the DMA transfers, in that case do you
> >>> > suggest me to define a separate dma_ctrl_cmd like the one FSLDMA_EXTERNAL_START
> >>> > defined for Freescale drivers?
> >>>
> >>> In my opinion it is not a good idea to have driver implement a generic API,
> >>> but at the same time let the driver have custom semantics for those API
> >>> calls. It's a bit like having a gpio driver that expects 23 and 42 as the
> >>> values passed to gpio_set_value instead of 0 and 1. It completely defeats
> >>> the purpose of a generic API, namely that you are able to write generic code
> >>> that makes use of the API without having to know about which implementation
> >>> API it is talking to. The dmaengine framework provides the
> >>> dmaengine_prep_interleaved_dma() function to setup two dimensional
> >>> transfers, e.g. take a look at sirf-dma.c or imx-dma.c.
> >>
> >> The question here i think would be waht this device supports? Is the hardware
> >> capable of doing interleaved transfers, then would make sense.
> >>
> >> While we do try to get users use dma_slave_config, but there will always be
> >> someone who have specfic params. If we can generalize then we might want to add
> >> to the dma_slave_config as well
> >
> > There are many configuration parameters which are specific to IP and I
> > would like to
> > give an overview of some of parameteres here:
> >
> > 1) Park Mode ('cfg->park'): In Park mode, engine will park on frame
> > referenced by
> > 'cfg->park_frm', so user will have control on each frame in this mode.
> >
> > 2) Interrupt Coalesce ('cfg->coalesce'): Used for setting interrupt
> > threshold. This value
> > determines the number of frame buffers to process. To use this feature,
> > 'cfg->frm_cnt_en' should be set.
> >
> > 3) Frame Synchronization Source ('cfg->ext_fsync'): Can be an
> > external/internal frame
> > synchronization source. Used to synchronize one channel (MM2S/S2MM) with
> > another (S2MM/MM2S) channel.
> >
> > 4) Genlock Synchronization ('cfg->genlock'): Used to avoid mismatch rate between
> > master and slave. In master mode (cfg->master), frames are not dropped and
> > slave can drop frames to adjust to master frame rate.
> >
> > And in future, this Engine being a soft IP, we could expect some more additional
> > parameters. Isn't a good idea to have a private member in dma_slave_config for
> > sharing additional configuration between slave device and dma engine? Or a new
> > dma_ctrl_cmd like FSLDMA_EXTERNAL_START?
The idea of a generic API is that we can use it for most of the controllers. Even
if you are planning to support a family of controllers
ATM, lets not discuss the possiblity of private member and try to exhanust all
possible options. Worst case you can embed the dma_slave_config in
xilinx_dma_slave_config and retrieve it in dmac driver
--
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 3/3] Phytec phyFLEX-i.MX6 : Added SATA Support
From: Ashutosh singh @ 2014-02-04 4:35 UTC (permalink / raw)
To: mark.rutland-5wv7dgnIgG8
Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ,
c.hemp-guT5V/WYfQezQB+pC5nmwQ, Ashutosh singh
This patch adds support for SATA on Phytec phyFLEX-i.MX6 Quad module.
Signed-off-by: Ashutosh singh <ashutosh.s-mS2nBM426Az/PtFMR13I2A@public.gmane.org>
---
arch/arm/boot/dts/imx6q-phytec-pbab01.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
index 21c8b37..5607c33 100644
--- a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
+++ b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
@@ -25,6 +25,10 @@
status = "okay";
};
+&sata {
+ status = "okay";
+};
+
&uart4 {
status = "okay";
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/3] Phytec phyFLEX-i.MX6 : Added GPMI-NAND Support
From: Ashutosh singh @ 2014-02-04 4:35 UTC (permalink / raw)
To: mark.rutland-5wv7dgnIgG8
Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ,
c.hemp-guT5V/WYfQezQB+pC5nmwQ, Ashutosh singh
This patch adds support for GPMI-NAND on Phytec phyFLEX-i.MX6 Quad module.
Signed-off-by: Ashutosh singh <ashutosh.s-mS2nBM426Az/PtFMR13I2A@public.gmane.org>
---
arch/arm/boot/dts/imx6q-phytec-pbab01.dts | 4 ++++
arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi | 7 +++++++
2 files changed, 11 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
index 91aecba..21c8b37 100644
--- a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
+++ b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
@@ -21,6 +21,10 @@
status = "okay";
};
+&gpmi {
+ status = "okay";
+};
+
&uart4 {
status = "okay";
};
diff --git a/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
index fb39dae..8787101 100644
--- a/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
+++ b/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
@@ -176,6 +176,13 @@
status = "disabled";
};
+&gpmi {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_gpmi_nand_1>;
+ nand-on-flash-bbt;
+ status = "disabled";
+};
+
&uart4 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart4_1>;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 1/3] Phytec phyFLEX-i.MX6 : Added USB_HOST Support
From: Ashutosh singh @ 2014-02-04 4:34 UTC (permalink / raw)
To: mark.rutland-5wv7dgnIgG8
Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ,
c.hemp-guT5V/WYfQezQB+pC5nmwQ, Ashutosh singh
This patch adds support for USB_HOST on Phytec phyFLEX-i.MX6 Quad module.
Signed-off-by: Ashutosh singh <ashutosh.s-mS2nBM426Az/PtFMR13I2A@public.gmane.org>
---
arch/arm/boot/dts/imx6q-phytec-pbab01.dts | 4 ++++
arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi | 15 +++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
index 87c3702..91aecba 100644
--- a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
+++ b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
@@ -25,6 +25,10 @@
status = "okay";
};
+&usbh1 {
+ status = "okay";
+};
+
&usbotg {
status = "okay";
};
diff --git a/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
index e682bf8..fb39dae 100644
--- a/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
+++ b/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
@@ -27,6 +27,15 @@
gpio = <&gpio4 15 0>;
enable-active-low;
};
+
+ reg_usb_h1_vbus: regulator@1 {
+ compatible = "regulator-fixed";
+ regulator-name = "usb_h1_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ gpio = <&gpio1 0 0>;
+ enable-active-low;
+ };
};
&ecspi3 {
@@ -144,6 +153,7 @@
MX6QDL_PAD_DISP0_DAT3__GPIO4_IO24 0x80000000 /* SPI NOR chipselect */
MX6QDL_PAD_DI0_PIN15__GPIO4_IO17 0x80000000 /* PMIC interrupt */
MX6QDL_PAD_KEY_ROW4__GPIO4_IO15 0x80000000 /* USB_OTG_PWR_EN */
+ MX6QDL_PAD_GPIO_0__USB_H1_PWR 0x80000000 /* USB_H1_PWR_EN */
>;
};
};
@@ -172,6 +182,11 @@
status = "disabled";
};
+&usbh1 {
+ vbus-supply = <®_usb_h1_vbus>;
+ status = "disabled";
+};
+
&usbotg {
vbus-supply = <®_usb_otg_vbus>;
pinctrl-names = "default";
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2] Phytec phyFLEX-i.MX6 : Added USB_OTG Support
From: Ashutosh singh @ 2014-02-04 3:39 UTC (permalink / raw)
To: mark.rutland-5wv7dgnIgG8
Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, festevam-Re5JQEeQqe8AvxtiuMwx3w,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, Ashutosh singh
This patch adds support for USB_OTG on Phytec phyFLEX-i.MX6 Quad module.
Signed-off-by: Ashutosh singh <ashutosh.s-mS2nBM426Az/PtFMR13I2A@public.gmane.org>
---
arch/arm/boot/dts/imx6q-phytec-pbab01.dts | 4 ++++
arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi | 18 ++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
index 7d37ec6..87c3702 100644
--- a/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
+++ b/arch/arm/boot/dts/imx6q-phytec-pbab01.dts
@@ -25,6 +25,10 @@
status = "okay";
};
+&usbotg {
+ status = "okay";
+};
+
&usdhc2 {
status = "okay";
};
diff --git a/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
index 1a3b50d..e682bf8 100644
--- a/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
+++ b/arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi
@@ -18,6 +18,15 @@
memory {
reg = <0x10000000 0x80000000>;
};
+
+ reg_usb_otg_vbus: regulator@0 {
+ compatible = "regulator-fixed";
+ regulator-name = "usb_otg_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ gpio = <&gpio4 15 0>;
+ enable-active-low;
+ };
};
&ecspi3 {
@@ -134,6 +143,7 @@
MX6QDL_PAD_EIM_D23__GPIO3_IO23 0x80000000
MX6QDL_PAD_DISP0_DAT3__GPIO4_IO24 0x80000000 /* SPI NOR chipselect */
MX6QDL_PAD_DI0_PIN15__GPIO4_IO17 0x80000000 /* PMIC interrupt */
+ MX6QDL_PAD_KEY_ROW4__GPIO4_IO15 0x80000000 /* USB_OTG_PWR_EN */
>;
};
};
@@ -162,6 +172,14 @@
status = "disabled";
};
+&usbotg {
+ vbus-supply = <®_usb_otg_vbus>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usbotg_1>;
+ disable-over-current;
+ status = "disabled";
+};
+
&usdhc2 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_usdhc2_2>;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v3 2/8] ARM: dts: sun7i: Add GMAC clock node to sun7i DTSI
From: Chen-Yu Tsai @ 2014-02-04 3:06 UTC (permalink / raw)
To: Maxime Ripard
Cc: Emilio Lopez, Mike Turquette, linux-arm-kernel, linux-sunxi,
devicetree, linux-kernel
In-Reply-To: <20140203193431.GD25625@lukather>
On Tue, Feb 4, 2014 at 3:34 AM, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Mon, Feb 03, 2014 at 11:32:20AM +0800, Chen-Yu Tsai wrote:
>> The GMAC uses 1 of 2 sources for its transmit clock, depending on the
>> PHY interface mode. Add both sources as dummy clocks, and as parents
>> to the GMAC clock node.
>>
>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>> ---
>> arch/arm/boot/dts/sun7i-a20.dtsi | 28 ++++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
>> index 1595e9a..fc7f470 100644
>> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
>> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
>> @@ -314,6 +314,34 @@
>> };
>>
>> /*
>> + * The following two are dummy clocks, placeholders used
>> + * on gmac_tx clock. The actual frequency and availability
>> + * depends on the external PHY, operation mode and link
>> + * speed.
>> + */
>
> If it depends on the external PHY, I guess that means it also depends
> on the board, right? Or is the GMAC supposed to always have that clock
> running at 25MHz, no matter what PHY is connected to it?
What I meant in the comment is that we cannot control the actual clock
rate of the TX clock. We can only select the source, and this is what
gmac_tx clock does. It is just a clock mux. The 125MHz and 25MHz clock
rates are used by the clk_set_rate in the stmmac glue layer to do
auto-reparenting.
The board dependent factor is what _type_ of PHY it is using, i.e.
MII, GMII, or RGMII. If it's MII, the PHY should provide the clock.
If it's RGMII, the internal clock would be used. GMII is a mix of
both. The actual clock rate depends on the link speed.
I should rephrase the comment along the lines of:
The following two are dummy clocks, placeholders used in the gmac_tx
clock. The gmac driver will choose one parent depending on the PHY
interface mode, using clk_set_rate auto-reparenting.
The actual TX clock rate is not controlled by the gmac_tx clock.
Cheers
ChenYu
^ permalink raw reply
* Re: [PATCH v3 1/8] clk: sunxi: Add Allwinner A20/A31 GMAC clock unit
From: Chen-Yu Tsai @ 2014-02-04 2:43 UTC (permalink / raw)
To: Maxime Ripard
Cc: Emilio Lopez, Mike Turquette, linux-arm-kernel, linux-sunxi,
devicetree, linux-kernel
In-Reply-To: <20140203193108.GC25625@lukather>
Hi,
On Tue, Feb 4, 2014 at 3:31 AM, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hi,
>
> On Mon, Feb 03, 2014 at 11:32:19AM +0800, Chen-Yu Tsai wrote:
>> The Allwinner A20/A31 clock module controls the transmit clock source
>> and interface type of the GMAC ethernet controller. Model this as
>> a single clock for GMAC drivers to use.
>>
>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>> ---
>> Documentation/devicetree/bindings/clock/sunxi.txt | 26 +++++++
>> drivers/clk/sunxi/clk-sunxi.c | 83 +++++++++++++++++++++++
>> 2 files changed, 109 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
>> index 0cf679b..f43b4c0 100644
>> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
>> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
>> @@ -37,6 +37,7 @@ Required properties:
>> "allwinner,sun6i-a31-apb2-gates-clk" - for the APB2 gates on A31
>> "allwinner,sun4i-mod0-clk" - for the module 0 family of clocks
>> "allwinner,sun7i-a20-out-clk" - for the external output clocks
>> + "allwinner,sun7i-a20-gmac-clk" - for the GMAC clock module on A20/A31
>>
>> Required properties for all clocks:
>> - reg : shall be the control register address for the clock.
>> @@ -50,6 +51,9 @@ Required properties for all clocks:
>> If the clock module only has one output, the name shall be the
>> module name.
>>
>> +For "allwinner,sun7i-a20-gmac-clk", the parent clocks shall be fixed rate
>> +dummy clocks at 25 MHz and 125 MHz, respectively. See example.
>> +
>> Clock consumers should specify the desired clocks they use with a
>> "clocks" phandle cell. Consumers that are using a gated clock should
>> provide an additional ID in their clock property. This ID is the
>> @@ -96,3 +100,25 @@ mmc0_clk: clk@01c20088 {
>> clocks = <&osc24M>, <&pll6 1>, <&pll5 1>;
>> clock-output-names = "mmc0";
>> };
>> +
>> +mii_phy_tx_clk: clk@2 {
>> + #clock-cells = <0>;
>> + compatible = "fixed-clock";
>> + clock-frequency = <25000000>;
>> + clock-output-names = "mii_phy_tx";
>> +};
>> +
>> +gmac_int_tx_clk: clk@3 {
>> + #clock-cells = <0>;
>> + compatible = "fixed-clock";
>> + clock-frequency = <125000000>;
>> + clock-output-names = "gmac_int_tx";
>> +};
>> +
>> +gmac_clk: clk@01c20164 {
>> + #clock-cells = <0>;
>> + compatible = "allwinner,sun7i-a20-gmac-clk";
>> + reg = <0x01c20164 0x4>;
>> + clocks = <&mii_phy_tx_clk>, <&gmac_int_tx_clk>;
>
> You should also document in which order you expect the parents to
> be. Or it will probably be easier to just use clock-names here.
Is it not clear from the "Required properties" section above?
>
>> + clock-output-names = "gmac";
>> +};
>> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
>> index 736fb60..0b361d2 100644
>> --- a/drivers/clk/sunxi/clk-sunxi.c
>> +++ b/drivers/clk/sunxi/clk-sunxi.c
>> @@ -379,6 +379,89 @@ static void sun7i_a20_get_out_factors(u32 *freq, u32 parent_rate,
>>
>>
>> /**
>> + * sun7i_a20_gmac_clk_setup - Setup function for A20/A31 GMAC clock module
>> + *
>> + * This clock looks something like this
>> + * ________________________
>> + * MII TX clock from PHY >-----|___________ _________|----> to GMAC core
>> + * GMAC Int. RGMII TX clk >----|___________\__/__gate---|----> to PHY
>> + * Ext. 125MHz RGMII TX clk >--|__divider__/ |
>> + * |________________________|
>> + *
>> + * The external 125 MHz reference is optional, i.e. GMAC can use its
>> + * internal TX clock just fine. The A31 GMAC clock module does not have
>> + * the divider controls for the external reference.
>> + *
>> + * To keep it simple, let the GMAC use either the MII TX clock for MII mode,
>> + * and its internal TX clock for GMII and RGMII modes. The GMAC driver should
>> + * select the appropriate source and gate/ungate the output to the PHY.
>> + *
>> + * Only the GMAC should use this clock. Altering the clock so that it doesn't
>> + * match the GMAC's operation parameters will result in the GMAC not being
>> + * able to send traffic out. The GMAC driver should set the clock rate and
>> + * enable/disable this clock to configure the required state. The clock
>> + * driver then responds by auto-reparenting the clock.
>> + */
>> +
>> +#define SUN7I_A20_GMAC_GPIT 2
>> +#define SUN7I_A20_GMAC_MASK 0x3
>> +#define SUN7I_A20_GMAC_MAX_PARENTS 2
>> +
>> +static void __init sun7i_a20_gmac_clk_setup(struct device_node *node)
>> +{
>> + struct clk *clk;
>> + struct clk_mux *mux;
>> + struct clk_gate *gate;
>> + const char *clk_name = node->name;
>> + const char *parents[SUN7I_A20_GMAC_MAX_PARENTS];
>> + void *reg;
>> + int i = 0;
>> +
>> + /* allocate mux and gate clock structs */
>> + mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
>> + if (!mux)
>> + return;
>
> Newline.
>
>> + gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
>> + if (!gate) {
>> + kfree(mux);
>> + return;
>> + }
>> +
>> + reg = of_iomap(node, 0);
>
> You should check for the return code here.
>
>> + of_property_read_string(node, "clock-output-names", &clk_name);
>
> And here too, since you made the clock-output-names property mandatory
>
>> + while (i < SUN7I_A20_GMAC_MAX_PARENTS &&
>> + (parents[i] = of_clk_get_parent_name(node, i)) != NULL)
>
> You should check for an error here too, but if you switch to using
> clock-names, that will probably be refactored anyway.
>
>> + i++;
>> +
>> + /* set up gate and fixed rate properties */
>> + gate->reg = reg;
>> + gate->bit_idx = SUN7I_A20_GMAC_GPIT;
>> + gate->lock = &clk_lock;
>> + mux->reg = reg;
>> + mux->mask = SUN7I_A20_GMAC_MASK;
>> + mux->flags = CLK_MUX_INDEX_BIT;
>> + mux->lock = &clk_lock;
>> +
>> + clk = clk_register_composite(NULL, clk_name,
>> + parents, i,
>> + &mux->hw, &clk_mux_ops,
>> + NULL, NULL,
>> + &gate->hw, &clk_gate_ops,
>> + 0);
>> +
>> + if (!IS_ERR(clk)) {
>> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
>> + clk_register_clkdev(clk, clk_name, NULL);
>> + }
>> +}
>> +CLK_OF_DECLARE(sun7i_a20_gmac, "allwinner,sun7i-a20-gmac-clk",
>> + sun7i_a20_gmac_clk_setup);
>> +
>> +
>> +
>> +/**
>> * sunxi_factors_clk_setup() - Setup function for factor clocks
>> */
>>
>> --
>> 1.9.rc1
>>
>
> It looks fine otherwise.
I'll fix the rest.
Cheers
ChenYu
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox