* Re: [PATCH 5/8] arm64: use ZONE_DMA on DMA addressing limited devices
From: Nicolas Saenz Julienne @ 2019-08-01 16:40 UTC (permalink / raw)
To: Robin Murphy, Catalin Marinas
Cc: phill, devicetree, f.fainelli, linux-mm, marc.zyngier,
Will Deacon, linux-kernel, eric, iommu, robh+dt, linux-rpi-kernel,
mbrugger, akpm, m.szyprowski, frowand.list, hch, linux-arm-kernel,
wahrenst
In-Reply-To: <e35dd4a5-281b-d281-59c9-3fc7108eb8be@arm.com>
[-- Attachment #1: Type: text/plain, Size: 2044 bytes --]
On Thu, 2019-08-01 at 17:07 +0100, Robin Murphy wrote:
> On 2019-08-01 4:44 pm, Nicolas Saenz Julienne wrote:
> > On Wed, 2019-07-31 at 18:07 +0100, Catalin Marinas wrote:
> > > On Wed, Jul 31, 2019 at 05:47:48PM +0200, Nicolas Saenz Julienne wrote:
> > > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > > > index 1c4ffabbe1cb..f5279ef85756 100644
> > > > --- a/arch/arm64/mm/init.c
> > > > +++ b/arch/arm64/mm/init.c
> > > > @@ -50,6 +50,13 @@
> > > > s64 memstart_addr __ro_after_init = -1;
> > > > EXPORT_SYMBOL(memstart_addr);
> > > >
> > > > +/*
> > > > + * We might create both a ZONE_DMA and ZONE_DMA32. ZONE_DMA is needed
> > > > if
> > > > there
> > > > + * are periferals unable to address the first naturally aligned 4GB of
> > > > ram.
> > > > + * ZONE_DMA32 will be expanded to cover the rest of that memory. If
> > > > such
> > > > + * limitations doesn't exist only ZONE_DMA32 is created.
> > > > + */
> > >
> > > Shouldn't we instead only create ZONE_DMA to cover the whole 32-bit
> > > range and leave ZONE_DMA32 empty? Can__GFP_DMA allocations fall back
> > > onto ZONE_DMA32?
> >
> > Hi Catalin, thanks for the review.
> >
> > You're right, the GFP_DMA page allocation will fail with a nasty dmesg error
> > if
> > ZONE_DMA is configured but empty. Unsurprisingly the opposite situation is
> > fine
> > (GFP_DMA32 with an empty ZONE_DMA32).
>
> Was that tested on something other than RPi4 with more than 4GB of RAM?
> (i.e. with a non-empty ZONE_NORMAL either way)
No, all I did is play around with RPi4's memory size (1 GB vs 4 GB).
I'll see If I can get access to a dts based board with more than 4 GB, If not
I'll try to fake it. It's not ideal but I can set the limit on 3 GB and have
the 3 areas created (with and witouth an empty ZONE_DMA32).
On top of that, now that you ask, I realise I neglected all the ACPI based
servers. I have access to some so I'll make sure I test everything on them too
for the next series.
Regards,
Nicolas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v7 10/20] clk: tegra: clk-dfll: Add suspend and resume support
From: Dmitry Osipenko @ 2019-08-01 17:10 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding@gmail.com, Jonathan Hunter,
tglx@linutronix.de, jason@lakedaemon.net, marc.zyngier@arm.com,
linus.walleij@linaro.org, stefan@agner.ch, mark.rutland@arm.com
Cc: Peter De Schrijver, Prashant Gaikwad, sboyd@kernel.org,
linux-clk@vger.kernel.org, linux-gpio@vger.kernel.org,
Jui Chang Kuo, Joseph Lo, Timo Alho, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, Mikko Perttunen, Sandipan Patra,
robh+dt@kernel.org, devicetree@vger.kernel.org, rjw@rjwysocki.net,
viresh.kumar@linaro.org, linux-pm@vger.kernel.org
In-Reply-To: <BYAPR12MB3398C388471BC5811614C8FEC2DE0@BYAPR12MB3398.namprd12.prod.outlook.com>
01.08.2019 19:10, Sowjanya Komatineni пишет:
> I didn’t updated any patches. This is still same v7 just resent with
> CPUFreq maintainers in CC as I missed to add them earlier.
There are now two different threads for the same patches, which is not
very good. When I said that CPUFreq maintainers should be CC'ed, I
didn't mean to resend it all, sorry for not being clear about it. You
should've wait for more comments to the original patches and then make a
v8. I suggest to do the same in the current situation as well, please
address all the current comments and wait for 1-2 days, then make a v8.
^ permalink raw reply
* Re: [PATCH v2 4/6] irqchip/irq-pruss-intc: Add helper functions to configure internal mapping
From: Suman Anna @ 2019-08-01 17:10 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner, Jason Cooper
Cc: devicetree, Grygorii Strashko, David Lechner, Tony Lindgren,
Sekhar Nori, linux-kernel, Andrew F. Davis, Lokesh Vutla,
Rob Herring, Murali Karicheri, linux-omap, linux-arm-kernel,
Roger Quadros
In-Reply-To: <1a63eb50-7c5c-eb3d-3cbe-bd1cc59ce3fe@kernel.org>
Hi Marc,
On 8/1/19 3:45 AM, Marc Zyngier wrote:
> On 31/07/2019 23:41, Suman Anna wrote:
>> The PRUSS INTC receives a number of system input interrupt source events
>> and supports individual control configuration and hardware prioritization.
>> These input events can be mapped to some output interrupt lines through 2
>> levels of many-to-one mapping i.e. events to channel mapping and channels
>> to output interrupts.
>>
>> This mapping information is provided through the PRU firmware that is
>> loaded onto a PRU core/s or through the device tree node of the PRU
>> application. The mapping is configured by the PRU remoteproc driver, and
>> is setup before the PRU core is started and cleaned up after the PRU core
>> is stopped. This event mapping configuration logic programs the Channel
>> Map Registers (CMRx) and Host-Interrupt Map Registers (HMRx) only when a
>> new program is being loaded/started and the same events and interrupt
>> channels are reset to zero when stopping a PRU.
>>
>> Add two helper functions: pruss_intc_configure() & pruss_intc_unconfigure()
>> that the PRU remoteproc driver can use to configure the PRUSS INTC.
>
> So let me see if I correctly understand this: this adds yet another
> firmware description parser, with a private interface to another
> (undisclosed?) driver, bypassing the standard irqchip configuration
> mechanism. It sounds great, doesn't it?
>
> What I cannot really infer from this message (-ETOOMUCHJARGON) is what
> interrupts this affects:
>
> - Interrupts from random devices to the PRUSS?
> - Interrupts from the PRUSS to the host?
> - Something else?
The interrupt sources (called system events) can be from internal PRUSS
peripherals, SoC-level peripherals or just software triggering (limited
to some events).
So, the PRUSS INTC behaves as a funnel and is both an interrupt router
and multiplexer. The INTC itself is part of the PRUSS, and all PRU
application related interrupts/events that need to trigger an interrupt
to either the PRU cores or other host processors (like DSP, ARM) have to
go through this INTC, and routed out to a limited number of output
interrupts that are then connected to different processors.
The split of interrupt handling between a PRU and its peer host
processor will be a application design choice (We can implement soft IPs
like UARTs, ADCs, I2Cs etc using PRUs). Some of the input events
themselves are multiplexed and controlled by a single MMR (outside of
INTC) that feeds different sets of events into the INTC. The MMR
configuration is outside of scope of this driver and will depend on the
application/client driver being run.
>
> When does this happen? Under control of what? It isn't even clear why
> this is part of this irqchip driver.
The mapping configuration is per PRU application and firmware, and is
done in line with acquiring and release a PRU which is treated as an
exclusive resource. We establish the mapping for all events through this
driver including the events that are to be routed to PRUs. This is done
to save the tiny/limited Instruction RAM space that PRUs have.
We have designed this as an irqchip driver (instead of some custom SoC
driver exporting custom functions) to use standard Linux semantics/irq
API and better integrate with Linux DT, but we need some semantics for
establishing the routing at runtime depending on the PRU client driver
we are running. The exported functions will be called only by the PRU
remoteproc driver during a pru_rproc_get()/pru_rproc_put(), and are
transparent to PRU client drivers.
Please also see the discussion from v1 [1] on why we can't use an
extended number of interrupt-cells infrastructure for achieving this.
[1] https://patchwork.kernel.org/patch/11034563/
> Depending what this does, there may be ways to fit it into the standard
> interrupt configuration framework. After all, we already have standard
> interfaces to route interrupts to virtual CPUs, effectively passing full
> control of an interrupt to another entity. If you squint hard enough,
> your PRUSS can fit that description.
Yeah, I am open to suggestions if there is a better way of doing this.
regards
Suman
>
> If that doesn't work, then we need to make the IRQ framework grok that
> kind of requirement (hence my request for clarification). But I'm
> strongly opposed to inventing a SoC-private way of configuring
> interrupts behind the kernel's back.
>
> Thanks,
>
> M.
>
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: dsa: mt7530: Convert to PHYLINK API
From: René van Dorst @ 2019-08-01 17:21 UTC (permalink / raw)
To: davem
Cc: netdev, frank-w, sean.wang, f.fainelli, matthias.bgg, andrew,
vivien.didelot, john, linux-mediatek, linux-mips, robh+dt,
devicetree, Russell King - ARM Linux admin
In-Reply-To: <20190727184828.GT1330@shell.armlinux.org.uk>
Quoting Russell King - ARM Linux admin <linux@armlinux.org.uk>:
> Hi,
>
> Just a couple of minor points.
>
> On Wed, Jul 24, 2019 at 09:25:47PM +0200, René van Dorst wrote:
<snip>
>> +
>> +static void mt7530_phylink_validate(struct dsa_switch *ds, int port,
>> + unsigned long *supported,
>> + struct phylink_link_state *state)
>> +{
>> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>> +
>> + switch (port) {
>> + case 0: /* Internal phy */
>> + case 1:
>> + case 2:
>> + case 3:
>> + case 4:
>> + if (state->interface != PHY_INTERFACE_MODE_NA &&
>> + state->interface != PHY_INTERFACE_MODE_GMII)
>> + goto unsupported;
>> + break;
>> + /* case 5: Port 5 not supported! */
>> + case 6: /* 1st cpu port */
>> + if (state->interface != PHY_INTERFACE_MODE_NA &&
>> + state->interface != PHY_INTERFACE_MODE_RGMII &&
>> + state->interface != PHY_INTERFACE_MODE_TRGMII)
>> + goto unsupported;
>> + break;
>> + default:
>> + linkmode_zero(supported);
>> + dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
>
> You could reorder this as:
>
> default:
> dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
> unsupported:
> linkmode_zero(supported);
>
Hi David,
> and save having the "unsupported" at the end of the function. Not sure
> what DaveM would think of it though.
Can you give your option about this?
So I know what to do with it and make a new series.
Greats,
René
>
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: dsa: mt7530: Convert to PHYLINK API
From: David Miller @ 2019-08-01 17:22 UTC (permalink / raw)
To: opensource
Cc: netdev, frank-w, sean.wang, f.fainelli, matthias.bgg, andrew,
vivien.didelot, john, linux-mediatek, linux-mips, robh+dt,
devicetree, linux
In-Reply-To: <20190801172104.Horde.Cuwt4jywUX_mcO9-n8QpWTN@www.vdorst.com>
From: René van Dorst <opensource@vdorst.com>
Date: Thu, 01 Aug 2019 17:21:04 +0000
> Quoting Russell King - ARM Linux admin <linux@armlinux.org.uk>:
>
>> Hi,
>>
>> Just a couple of minor points.
>>
>> On Wed, Jul 24, 2019 at 09:25:47PM +0200, René van Dorst wrote:
>
> <snip>
>
>>> +
>>> +static void mt7530_phylink_validate(struct dsa_switch *ds, int port,
>>> + unsigned long *supported,
>>> + struct phylink_link_state *state)
>>> +{
>>> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>>> +
>>> + switch (port) {
>>> + case 0: /* Internal phy */
>>> + case 1:
>>> + case 2:
>>> + case 3:
>>> + case 4:
>>> + if (state->interface != PHY_INTERFACE_MODE_NA &&
>>> + state->interface != PHY_INTERFACE_MODE_GMII)
>>> + goto unsupported;
>>> + break;
>>> + /* case 5: Port 5 not supported! */
>>> + case 6: /* 1st cpu port */
>>> + if (state->interface != PHY_INTERFACE_MODE_NA &&
>>> + state->interface != PHY_INTERFACE_MODE_RGMII &&
>>> + state->interface != PHY_INTERFACE_MODE_TRGMII)
>>> + goto unsupported;
>>> + break;
>>> + default:
>>> + linkmode_zero(supported);
>>> + dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
>>
>> You could reorder this as:
>>
>> default:
>> dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
>> unsupported:
>> linkmode_zero(supported);
>>
>
> Hi David,
>
>> and save having the "unsupported" at the end of the function. Not
>> sure
>> what DaveM would think of it though.
>
> Can you give your option about this?
> So I know what to do with it and make a new series.
Russell's suggestion is fine.
^ permalink raw reply
* Re: [PATCH v1 2/2] net: npcm: add NPCM7xx EMC 10/100 Ethernet driver
From: Willem de Bruijn @ 2019-08-01 17:25 UTC (permalink / raw)
To: Avi Fishman
Cc: Patrick Venture, Nancy Yuen, benjaminfair, David Miller, robh+dt,
mark.rutland, Greg Kroah-Hartman, tmaimon77, tali.perry1, openbmc,
Network Development, devicetree, linux-kernel, tglx
In-Reply-To: <20190801072611.27935-3-avifishman70@gmail.com>
On Thu, Aug 1, 2019 at 3:28 AM Avi Fishman <avifishman70@gmail.com> wrote:
>
> EMC Ethernet Media Access Controller supports 10/100 Mbps and
> RMII.
> This driver has been working on Nuvoton BMC NPCM7xx.
>
> Signed-off-by: Avi Fishman <avifishman70@gmail.com>
> +/* global setting for driver */
> +#define RX_QUEUE_LEN 128
> +#define TX_QUEUE_LEN 64
> +#define MAX_RBUFF_SZ 0x600
> +#define MAX_TBUFF_SZ 0x600
> +#define TX_TIMEOUT 50
> +#define DELAY 1000
> +#define CAM0 0x0
> +#define RX_POLL_SIZE 16
> +
> +#ifdef CONFIG_VLAN_8021Q
> +#define IS_VLAN 1
> +#else
> +#define IS_VLAN 0
> +#endif
> +
> +#define MAX_PACKET_SIZE (1514 + (IS_VLAN * 4))
1514 -> ETH_FRAME_LEN
4 -> VLAN_HLEN
Does this device support stacked VLAN?
Is this really the device maximum?
> +#define MAX_PACKET_SIZE_W_CRC (MAX_PACKET_SIZE + 4) /* 1518 */
4 -> ETH_FCS_LEN
> +#if defined CONFIG_NPCM7XX_EMC_ETH_DEBUG || defined CONFIG_DEBUG_FS
> +#define REG_PRINT(reg_name) {t = scnprintf(next, size, "%-10s = %08X\n", \
> + #reg_name, readl(ether->reg + (reg_name))); size -= t; next += t; }
> +#define DUMP_PRINT(f, x...) {t = scnprintf(next, size, f, ## x); size -= t; \
> + next += t; }
> +
> +static int npcm7xx_info_dump(char *buf, int count, struct net_device *netdev)
> +{
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> + struct npcm7xx_txbd *txbd;
> + struct npcm7xx_rxbd *rxbd;
> + unsigned long flags;
> + unsigned int i, cur, txd_offset, rxd_offset;
> + char *next = buf;
> + unsigned int size = count;
> + int t;
> + int is_locked = spin_is_locked(ðer->lock);
> +
> + if (!is_locked)
> + spin_lock_irqsave(ðer->lock, flags);
> +
> + /* ------basic driver information ---- */
> + DUMP_PRINT("NPCM7XX EMC %s driver version: %s\n", netdev->name,
> + DRV_MODULE_VERSION);
> +
> + REG_PRINT(REG_CAMCMR);
> + REG_PRINT(REG_CAMEN);
> + REG_PRINT(REG_CAMM_BASE);
> + REG_PRINT(REG_CAML_BASE);
> + REG_PRINT(REG_TXDLSA);
> + REG_PRINT(REG_RXDLSA);
> + REG_PRINT(REG_MCMDR);
> + REG_PRINT(REG_MIID);
> + REG_PRINT(REG_MIIDA);
> + REG_PRINT(REG_FFTCR);
> + REG_PRINT(REG_TSDR);
> + REG_PRINT(REG_RSDR);
> + REG_PRINT(REG_DMARFC);
> + REG_PRINT(REG_MIEN);
> + REG_PRINT(REG_MISTA);
> + REG_PRINT(REG_MGSTA);
> + REG_PRINT(REG_MPCNT);
> + writel(0x7FFF, (ether->reg + REG_MPCNT));
> + REG_PRINT(REG_MRPC);
> + REG_PRINT(REG_MRPCC);
> + REG_PRINT(REG_MREPC);
> + REG_PRINT(REG_DMARFS);
> + REG_PRINT(REG_CTXDSA);
> + REG_PRINT(REG_CTXBSA);
> + REG_PRINT(REG_CRXDSA);
> + REG_PRINT(REG_CRXBSA);
> + REG_PRINT(REG_RXFSM);
> + REG_PRINT(REG_TXFSM);
> + REG_PRINT(REG_FSM0);
> + REG_PRINT(REG_FSM1);
> + REG_PRINT(REG_DCR);
> + REG_PRINT(REG_DMMIR);
> + REG_PRINT(REG_BISTR);
> + DUMP_PRINT("\n");
> +
> + DUMP_PRINT("netif_queue %s\n\n", netif_queue_stopped(netdev) ?
> + "Stopped" : "Running");
> + if (ether->rdesc)
> + DUMP_PRINT("napi is %s\n\n", test_bit(NAPI_STATE_SCHED,
> + ðer->napi.state) ?
> + "scheduled" :
> + "not scheduled");
> +
> + txd_offset = (readl((ether->reg + REG_CTXDSA)) -
> + readl((ether->reg + REG_TXDLSA))) /
> + sizeof(struct npcm7xx_txbd);
> + DUMP_PRINT("TXD offset %6d\n", txd_offset);
> + DUMP_PRINT("cur_tx %6d\n", ether->cur_tx);
> + DUMP_PRINT("finish_tx %6d\n", ether->finish_tx);
> + DUMP_PRINT("pending_tx %6d\n", ether->pending_tx);
> + /* debug counters */
> + DUMP_PRINT("tx_tdu %6d\n", ether->tx_tdu);
> + ether->tx_tdu = 0;
> + DUMP_PRINT("tx_tdu_i %6d\n", ether->tx_tdu_i);
> + ether->tx_tdu_i = 0;
> + DUMP_PRINT("tx_cp_i %6d\n", ether->tx_cp_i);
> + ether->tx_cp_i = 0;
> + DUMP_PRINT("tx_int_count %6d\n", ether->tx_int_count);
> + ether->tx_int_count = 0;
> + DUMP_PRINT("count_xmit tx %6d\n", ether->count_xmit);
> + ether->count_xmit = 0;
> + DUMP_PRINT("count_finish %6d\n", ether->count_finish);
> + ether->count_finish = 0;
> + DUMP_PRINT("\n");
> +
> + rxd_offset = (readl((ether->reg + REG_CRXDSA)) -
> + readl((ether->reg + REG_RXDLSA)))
> + / sizeof(struct npcm7xx_txbd);
> + DUMP_PRINT("RXD offset %6d\n", rxd_offset);
> + DUMP_PRINT("cur_rx %6d\n", ether->cur_rx);
> + DUMP_PRINT("rx_err %6d\n", ether->rx_err);
> + ether->rx_err = 0;
> + DUMP_PRINT("rx_berr %6d\n", ether->rx_berr);
> + ether->rx_berr = 0;
> + DUMP_PRINT("rx_stuck %6d\n", ether->rx_stuck);
> + ether->rx_stuck = 0;
> + DUMP_PRINT("rdu %6d\n", ether->rdu);
> + ether->rdu = 0;
> + DUMP_PRINT("rxov rx %6d\n", ether->rxov);
> + ether->rxov = 0;
> + /* debug counters */
> + DUMP_PRINT("rx_int_count %6d\n", ether->rx_int_count);
> + ether->rx_int_count = 0;
> + DUMP_PRINT("rx_err_count %6d\n", ether->rx_err_count);
> + ether->rx_err_count = 0;
Basic counters like tx_packets and rx_errors are probably better
exported regardless of debug level as net_device_stats. And then don't
need to be copied in debug output.
Less standard counters like tx interrupt count are probably better
candidates for ethtool -S.
> +#ifdef CONFIG_NPCM7XX_EMC_ETH_DEBUG
> +static void npcm7xx_info_print(struct net_device *netdev)
> +{
> + char *emc_dump_buf;
> + int count;
> + struct npcm7xx_ether *ether;
> + struct platform_device *pdev;
> + char c;
> + char *tmp_buf;
> + const size_t print_size = 5 * PAGE_SIZE;
> +
> + ether = netdev_priv(netdev);
> + pdev = ether->pdev;
> +
> + emc_dump_buf = kmalloc(print_size, GFP_KERNEL);
> + if (!emc_dump_buf)
> + return;
> +
> + tmp_buf = emc_dump_buf;
> + count = npcm7xx_info_dump(emc_dump_buf, print_size, netdev);
> + while (count > 512) {
> + c = tmp_buf[512];
> + tmp_buf[512] = 0;
> + dev_info(&pdev->dev, "%s", tmp_buf);
> + tmp_buf += 512;
> + tmp_buf[0] = c;
> + count -= 512;
Missing closing parenthesis.
Also, why this buffering to printk?
> +static void npcm7xx_write_cam(struct net_device *netdev,
> + unsigned int x, unsigned char *pval)
> +{
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> + __le32 msw, lsw;
> +
> + msw = (pval[0] << 24) | (pval[1] << 16) | (pval[2] << 8) | pval[3];
> +
> + lsw = (pval[4] << 24) | (pval[5] << 16);
Does __le32 plus this explicit shifting define host endianness? Better
to keep independent?
> +
> + writel(lsw, (ether->reg + REG_CAML_BASE) + x * CAM_ENTRY_SIZE);
> + writel(msw, (ether->reg + REG_CAMM_BASE) + x * CAM_ENTRY_SIZE);
> + dev_dbg(ðer->pdev->dev,
> + "REG_CAML_BASE = 0x%08X REG_CAMM_BASE = 0x%08X", lsw, msw);
> +}
> +
> +static struct sk_buff *get_new_skb(struct net_device *netdev, u32 i)
> +{
> + __le32 buffer;
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> + struct sk_buff *skb = netdev_alloc_skb(netdev,
> + roundup(MAX_PACKET_SIZE_W_CRC, 4));
> +
> + if (unlikely(!skb)) {
> + if (net_ratelimit())
> + netdev_warn(netdev, "failed to allocate rx skb\n");
can use net_warn_ratelimited (here and elsewhere)
> + buffer = ether->rx_scratch_dma;
> + } else {
> + /* Do not unmark the following skb_reserve() Receive Buffer
> + * Starting Address must be aligned to 4 bytes and the following
> + * line if unmarked will make it align to 2 and this likely will
> + * hult the RX and crash the linux
halt?
> + * skb_reserve(skb, NET_IP_ALIGN);
> + */
> + skb->dev = netdev;
> + buffer = dma_map_single(&netdev->dev,
> + skb->data,
> + roundup(MAX_PACKET_SIZE_W_CRC, 4),
> + DMA_FROM_DEVICE);
> + if (unlikely(dma_mapping_error(&netdev->dev, buffer))) {
> + if (net_ratelimit())
> + netdev_err(netdev, "failed to map rx page\n");
> + dev_kfree_skb_any(skb);
> + buffer = ether->rx_scratch_dma;
> + skb = NULL;
> + }
> + }
> + ether->rx_skb[i] = skb;
> + ether->rdesc[i].buffer = buffer;
> +
> + return skb;
> +}
> +
> +static int npcm7xx_ether_close(struct net_device *netdev)
> +{
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> +
> + npcm7xx_return_default_idle(netdev);
> +
> + if (ether->phy_dev)
> + phy_stop(ether->phy_dev);
> + else if (ether->use_ncsi)
> + ncsi_stop_dev(ether->ncsidev);
> +
> + msleep(20);
> +
> + free_irq(ether->txirq, netdev);
> + free_irq(ether->rxirq, netdev);
> +
> + netif_stop_queue(netdev);
> + napi_disable(ðer->napi);
Cleanup state in reverse of allocation.
> +static int npcm7xx_ether_start_xmit(struct sk_buff *skb, struct net_device *netdev)
> +{
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> + struct npcm7xx_txbd *txbd;
> + unsigned long flags;
> +
> + ether->count_xmit++;
> +
> + /* Insert new buffer */
> + txbd = (ether->tdesc + ether->cur_tx);
> + txbd->buffer = dma_map_single(&netdev->dev, skb->data, skb->len,
> + DMA_TO_DEVICE);
> + ether->tx_skb[ether->cur_tx] = skb;
> + if (skb->len > MAX_PACKET_SIZE)
> + dev_err(ðer->pdev->dev,
> + "skb->len (= %d) > MAX_PACKET_SIZE (= %d)\n",
> + skb->len, MAX_PACKET_SIZE);
> + txbd->sl = skb->len > MAX_PACKET_SIZE ? MAX_PACKET_SIZE : skb->len;
Check for errors before mapping to device, and drop packet? Probably
don't want to output truncated packets.
Also rate limit such messages.
> + dma_wmb();
> +
> + txbd->mode = TX_OWN_DMA | PADDINGMODE | CRCMODE;
> +
> + /* make sure that data is in memory before we trigger TX */
> + wmb();
> +
> + /* trigger TX */
> + writel(ENSTART, (ether->reg + REG_TSDR));
> +
> + if (++ether->cur_tx >= TX_QUEUE_LEN)
> + ether->cur_tx = 0;
> +
> + spin_lock_irqsave(ðer->lock, flags);
> + ether->pending_tx++;
> +
> + /* sometimes we miss the tx interrupt due to HW issue, so NAPI will not
> + * clean the pending tx, so we clean it also here
> + */
> + npcm7xx_clean_tx(netdev, true);
> +
> + if (ether->pending_tx >= TX_QUEUE_LEN - 1) {
> + __le32 reg_mien;
> + unsigned int index_to_wake = ether->cur_tx +
> + ((TX_QUEUE_LEN * 3) / 4);
> +
> + if (index_to_wake >= TX_QUEUE_LEN)
> + index_to_wake -= TX_QUEUE_LEN;
> +
> + txbd = (ether->tdesc + index_to_wake);
> + txbd->mode = TX_OWN_DMA | PADDINGMODE | CRCMODE | MACTXINTEN;
> +
> + /* make sure that data is in memory before we trigger TX */
> + wmb();
> +
> + /* Clear TDU interrupt */
> + writel(MISTA_TDU, (ether->reg + REG_MISTA));
> +
> + /* due to HW issue somtimes, we miss the TX interrupt we just
somtimes -> sometimes
> + * set (MACTXINTEN), so we also set TDU for Transmit
> + * Descriptor Unavailable interrupt
> + */
> + reg_mien = readl((ether->reg + REG_MIEN));
> + if (reg_mien != 0)
> + /* Enable TDU interrupt */
> + writel(reg_mien | ENTDU, (ether->reg + REG_MIEN));
> +
> + ether->tx_tdu++;
> + netif_stop_queue(netdev);
> + }
> +
> + spin_unlock_irqrestore(ðer->lock, flags);
> +
> + return 0;
> +}
> +
> +static irqreturn_t npcm7xx_tx_interrupt(int irq, void *dev_id)
> +{
> + struct npcm7xx_ether *ether;
> + struct platform_device *pdev;
> + struct net_device *netdev;
> + __le32 status;
> + unsigned long flags;
> +
> + netdev = dev_id;
> + ether = netdev_priv(netdev);
> + pdev = ether->pdev;
> +
> + npcm7xx_get_and_clear_int(netdev, &status, 0xFFFF0000);
> +
> + ether->tx_int_count++;
> +
> + if (status & MISTA_EXDEF)
> + dev_err(&pdev->dev, "emc defer exceed interrupt status=0x%08X\n"
> + , status);
> + else if (status & MISTA_TXBERR) {
> + dev_err(&pdev->dev, "emc bus error interrupt status=0x%08X\n",
> + status);
> +#ifdef CONFIG_NPCM7XX_EMC_ETH_DEBUG
> + npcm7xx_info_print(netdev);
> +#endif
> + spin_lock_irqsave(ðer->lock, flags);
irqsave in hard interrupt context?
> + writel(0, (ether->reg + REG_MIEN)); /* disable any interrupt */
> + spin_unlock_irqrestore(ðer->lock, flags);
> + ether->need_reset = 1;
> + } else if (status & ~(MISTA_TXINTR | MISTA_TXCP | MISTA_TDU))
> + dev_err(&pdev->dev, "emc other error interrupt status=0x%08X\n",
> + status);
> +
> + /* if we got MISTA_TXCP | MISTA_TDU remove those interrupt and call napi */
The goal of napi is to keep interrupts disabled until napi completes.
> + if (status & (MISTA_TXCP | MISTA_TDU) &
> + readl((ether->reg + REG_MIEN))) {
> + __le32 reg_mien;
> +
> + spin_lock_irqsave(ðer->lock, flags);
> + reg_mien = readl((ether->reg + REG_MIEN));
> + if (reg_mien & ENTDU)
> + /* Disable TDU interrupt */
> + writel(reg_mien & (~ENTDU), (ether->reg + REG_MIEN));
> +
> + spin_unlock_irqrestore(ðer->lock, flags);
> +
> + if (status & MISTA_TXCP)
> + ether->tx_cp_i++;
> + if (status & MISTA_TDU)
> + ether->tx_tdu_i++;
> + } else {
> + dev_dbg(&pdev->dev, "status=0x%08X\n", status);
> + }
> +
> + napi_schedule(ðer->napi);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t npcm7xx_rx_interrupt(int irq, void *dev_id)
> +{
> + struct net_device *netdev = (struct net_device *)dev_id;
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> + struct platform_device *pdev = ether->pdev;
> + __le32 status;
> + unsigned long flags;
> + unsigned int any_err = 0;
> + __le32 rxfsm;
> +
> + npcm7xx_get_and_clear_int(netdev, &status, 0xFFFF);
Same here
> +static int npcm7xx_poll(struct napi_struct *napi, int budget)
> +{
> + struct npcm7xx_ether *ether =
> + container_of(napi, struct npcm7xx_ether, napi);
> + struct npcm7xx_rxbd *rxbd;
> + struct net_device *netdev = ether->netdev;
> + struct platform_device *pdev = ether->pdev;
> + struct sk_buff *s;
> + unsigned int length;
> + __le32 status;
> + unsigned long flags;
> + int rx_cnt = 0;
> + int complete = 0;
> + unsigned int rx_offset = (readl((ether->reg + REG_CRXDSA)) -
> + ether->start_rx_ptr) /
> + sizeof(struct npcm7xx_txbd);
> + unsigned int local_count = (rx_offset >= ether->cur_rx) ?
> + rx_offset - ether->cur_rx : rx_offset +
> + RX_QUEUE_LEN - ether->cur_rx;
> +
> + if (local_count > ether->max_waiting_rx)
> + ether->max_waiting_rx = local_count;
> +
> + if (local_count > (4 * RX_POLL_SIZE))
> + /* we are porbably in a storm of short packets and we don't
porbably - probably
> + * want to get into RDU since short packets in RDU cause
> + * many RXOV which may cause EMC halt, so we filter out all
> + * coming packets
> + */
> + writel(0, (ether->reg + REG_CAMCMR));
> +
> + if (local_count <= budget)
> + /* we can restore accepting of packets */
> + writel(ether->camcmr, (ether->reg + REG_CAMCMR));
> +
> + spin_lock_irqsave(ðer->lock, flags);
> + npcm7xx_clean_tx(netdev, false);
> + spin_unlock_irqrestore(ðer->lock, flags);
> +
> + rxbd = (ether->rdesc + ether->cur_rx);
> +
> + while (rx_cnt < budget) {
> + status = rxbd->sl;
> + if ((status & RX_OWN_DMA) == RX_OWN_DMA) {
> + complete = 1;
> + break;
> + }
> + /* for debug puposes we save the previous value */
puposes -> purposes
> + rxbd->reserved = status;
> + s = ether->rx_skb[ether->cur_rx];
> + length = status & 0xFFFF;
> +
> + /* If VLAN is not supporte RXDS_PTLE (packet too long) is also
supporte -> supported (stopping pointing out typos after this).
> +static const struct net_device_ops npcm7xx_ether_netdev_ops = {
> + .ndo_open = npcm7xx_ether_open,
> + .ndo_stop = npcm7xx_ether_close,
> + .ndo_start_xmit = npcm7xx_ether_start_xmit,
> + .ndo_get_stats = npcm7xx_ether_stats,
> + .ndo_set_rx_mode = npcm7xx_ether_set_rx_mode,
> + .ndo_set_mac_address = npcm7xx_set_mac_address,
> + .ndo_do_ioctl = npcm7xx_ether_ioctl,
> + .ndo_validate_addr = eth_validate_addr,
> + .ndo_change_mtu = eth_change_mtu,
This is marked as deprecated. Also in light of the hardcoded
MAX_PACKET_SIZE, probably want to set dev->max_mtu.
> +static int npcm7xx_ether_probe(struct platform_device *pdev)
> +{
> + struct npcm7xx_ether *ether;
> + struct net_device *netdev;
> + int error;
> +
> + struct clk *emc_clk = NULL;
> + struct device_node *np = pdev->dev.of_node;
> +
> + pdev->id = of_alias_get_id(np, "ethernet");
> + if (pdev->id < 0)
> + pdev->id = 0;
> +
> + emc_clk = devm_clk_get(&pdev->dev, NULL);
> +
> + if (IS_ERR(emc_clk))
> + return PTR_ERR(emc_clk);
> +
> + /* Enable Clock */
> + clk_prepare_enable(emc_clk);
> +
> + error = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> + if (error)
> + return -ENODEV;
> +
> + netdev = alloc_etherdev(sizeof(struct npcm7xx_ether));
> + if (!netdev)
> + return -ENOMEM;
> +
> + ether = netdev_priv(netdev);
> +
> + ether->reset = devm_reset_control_get(&pdev->dev, NULL);
> + if (IS_ERR(ether->reset))
> + return PTR_ERR(ether->reset);
Memory leak on error path
Also missing netif_napi_del in npcm7xx_ether_remove?
^ permalink raw reply
* Re: [PATCH 0/4] new driver for TI eQEP
From: David Lechner @ 2019-08-01 17:37 UTC (permalink / raw)
To: William Breathitt Gray
Cc: linux-iio, linux-omap, devicetree, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Thierry Reding, linux-kernel,
linux-pwm
In-Reply-To: <20190730044536.GA5063@icarus>
On 7/29/19 11:45 PM, William Breathitt Gray wrote:
> On Thu, Jul 25, 2019 at 05:52:21PM -0500, David Lechner wrote:
>> On 7/25/19 7:40 AM, William Breathitt Gray wrote:
>>> On Mon, Jul 22, 2019 at 10:45:34AM -0500, David Lechner wrote:
>>>> This series adds device tree bindings and a new counter driver for the Texas
>>>> Instruments Enhanced Quadrature Encoder Pulse (eQEP).
>>>>
>>>> As mentioned in one of the commit messages, to start with, the driver only
>>>> supports reading the current counter value and setting the min/max values.
>>>> Other features can be added on an as-needed basis.
>>>>
>>>> The only other feature I am interested in is adding is getting time data in
>>>> order to calculate the rotational speed of a motor. However, there probably
>>>> needs to be a higher level discussion of how this can fit into the counter
>>>> subsystem in general first.
>>>
>>> I believe exposing some sort of time data has merit. Quadrature counter
>>> devices in particular are commonly used for position tracking of
>>> automation systems, and such systems would benefit from velocity/speed
>>> information. So let's try to introduce that sort of functionality in this
>>> driver if possible.
>>>
>>> First, let's discuss your specific use case and requirements, and hopefully we
>>> can generalize it enough to be of use for future drivers. From your description,
>>> it sounds like you're attaching some sort of rotary encoder to the eQEP device.
>>> Is that correct? What sort of time data are you hoping to use; does the eQEP
>>> device provide a clock value, or would you be grabbing a timestamp from the
>>> system?
>>
>> My use case is robotics using LEGO MINDSTORMS. More specifically, I am using
>> motors that have a cheap optical rotary encoder (plastic wheel and infrared
>> LED/detectors) that give 360 counts per 1 rotation of the motor shaft. One count
>> is defined as the rising edge or falling edge of the A signal. We are looking at
>> anywhere from 0 to around 2000 counts per second. We use the speed as feedback in
>> a control algorithm to drive the motor at a constant speed. The control loop
>> updates on the order of 1 to 10 ms.
>>
>> Because the encoder resolution and speeds are relatively low, we are currently
>> logging a timestamp for each count. If no count occurs for 50ms, then we log the
>> same count again with a new timestamp (otherwise we would never see 0 speed). To
>> get the actual speed, we find the first timestamp > 20 ms before the current
>> timestamp then compute the speed as the change in position divided by the change
>> in time between these two samples. This give a fairly accurate speed across most
>> of the range, but does get a bit noisy once we get below 100 counts per second.
>> It also means that we need a ring buffer that holds about 50 samples.
>>
>> The timestamp itself comes from the eQEP, not the system. There are latching
>> registers to ensure that the timestamp read is from exactly the moment when
>> the count register was read.
>
> So if I understand correctly, there are two registers you're reading: a
> count register and a timestamp register. The count register is updated
> by the rotation of the motor shaft, while the timestamp register is
> updated by reading the count register (thus logging the time associated
> with the read count value).
That is correct.
>
>>> I'm not sure yet if it would make sense to expose rotational speed directly as
>>> an attribute. If we were to expose just the count value and timestamp since the
>>> last read, that should be enough for a user to compute the delta and derive
>>> speed. I'll think more about this since some devices may simplify that case if
>>> the hardware is able to compute the speed for us.
>>>
>>
>> I agree that it probably doesn't make sense to expect drivers to compute the
>> speed. There isn't really a general way to do that works for an arbitrary
>> speed. For example at high speeds, it is better to just look at the change
>> in counts over a fixed interval rather than triggering a timestamp based on
>> a certain number of counts.
>
> This is a good point. Depending on the resolution the user cares about,
> they may be more interested in the speed over a short time interval
> versus a long time interval. It doesn't seem practical to have the driver
> try to handle all possible speed calculations when the user can decide
> themselves how best to use the data.
>
>> I also don't think having a timestamp sysfs attribute would be very useful.
>> To make it work at all, I think it would have to be implemented such that
>> it returns the timestamp for the count that was most recently read via sysfs.
>> And it would require 4 syscalls (2 seeks and 2 reads) to get a single count/
>> timestamp pair in a control loop. On a 300MHz ARM9 processor, this is not
>> a negligible amount of time.
>
> This is a concern I've had as well. The sysfs interface is useful in
> that it provides an intuitive and human-friendly way to expose data
> about devices. But as you note, there is considerable overhead in the
> amount of syscalls we have to make to interact with multiple attributes.
>
> One solution that may work is providing a character device interface in
> addition to the sysfs interface. I believe that should reduce the
> syscall overhead since a user can pass in a data structure with a
> configuration defining what data/actions they want, and receive back
> all data in a single syscall.
Just toying with the idea here, but I've been thinking that it might
work well to be able to mmap a char device to access a ring buffer.
Then there should basically be no overhead at all from getting information
from the kernel to userspace.
>
> I think concern over latency was one of the reasons the GPIO subsystem
> gained a character device interface as well. It's an addition to the
> Counter subsystem that is worth considering, but the possible downsides
> to such an interface should also be investigated.
>
>> I noticed that several of the other counter drivers also register an IIO
>> device. So this got me thinking that perhaps the counter subsystem should
>> just be for configuring a counter device an then the IIO subsystem should
>> be used for triggers and ring buffers.
>>
>> For the general case a counter device could have two possible triggers.
>> One that triggers an interrupt after X counts and another that triggers
>> with a period of T nanoseconds (or microseconds). Both triggers would add
>> a count/timestamp pair to an IIO ring buffer.
>>
>> To fully reproduce our current methodology the first trigger would actually
>> need two configurable settings, the count X that triggers every X counts and
>> a watchdog time setting (using terminology from eQEP docs) that will also
>> trigger if and only if the count does not change before the time has elapsed.
>> Note, this is different from the other proposed time based trigger which
>> would cause a trigger interrupt at a fixed period regardless of whether
>> the count changed or not.
>
> The counter drivers in the kernel right now are registering IIO devices
> in order to keep the preexisting (but now deprecated) IIO Counter
> interface working for these devices -- some users may be using this
> older interface so we don't want to remove it cold turkey. Regardless,
> there's nothing the prevents incorporating the IIO interface with your
> Counter drivers; in fact, in some circumstances it's better that you do
> just that.
>
> The key idea to recognize is how the Counter subsystem differs from the
> IIO subsystem on a conceptual level: the IIO subsystem provides an
> interface for your device by describing it on a hardware level, whereas
> the Counter subsystem provides an interface for your device by
> describing it on a more abstract level.
>
> What I mean is that every interface interaction in the Counter subsystem
> relates to the abstract concept of an ideal "counter device" (Counts,
> Synapses, Signals); if a device functionality or data does not relate
> directly to those ideal counter device components, then the Counter
> subsystem isn't that right interface for it.
>
> For example, it makes sense to have an "enable" attribute or "present"
> attribute, because these functionalities/data are directly related to
> the Count, Synapse, and Signal components conceptually. However, in the
> Counter subsystem you will likely not see something like the IIO
> "in_voltageY_supply_raw" attribute -- not because that data is not
> useful to know about for the operation of the counter device hardware,
> but because it is outside the scope of the Counter subsystem paradigm
> (i.e. it does not directly related to Counts, Synapses, or Signals).
> As such, this would be a case where the counter driver should register
> both a Counter device and IIO device, one to handle the counter device
> on an abstract level while the other provides an interface for control
> of the more specific hardware details.
Makes sense. I that case, I don't see a need for an IIO device for the
eQEP.
>
>> ---
>>
>> Thinking more generally though, I think what I would propose is adding a new
>> component to the existing list of Count, Signal and Synapse. The new component
>> could be called Event. Event would be more general than the trigger conditions
>> I have just discussed. In addition to those two, it could be any event
>> generated by the hardware, such as an error condition or a change in direction.
>>
>> Drivers could register an arbitrary number of events for each Count, so we
>> would have /sys/bus/counter/devices/counterX/eventY/*. There should be a few
>> standard attributes, like "name" and "enable". Configurable events would need
>> ext attributes to allow configuration.
>>
>> However, I see that there are already preset and error_noise "events" for
>> count objects, so maybe we don't do the eventY thing and keep it flatter (or
>> is the counter subsystem still considered in "staging" where breaking ABI
>> changes could be made?).
>
> The components for handling events already exist in the Counter
> interface paradigm: Signals and Synapses. Although, the Counter
> subsystem is currently lacking the implementation (I still need to code
> in support for interrupts and such), the paradigm itself supports the
> concept of events and triggers.
>
> Recall that the Counter subsystem represents hardware via the
> abstraction of an idealized "counter device". This is important to
> understand because it means that Signals are not necessarily limited to
> the physical wires of the hardware. To summarize the Counter interface
> paradigm:
>
> * A Signal is a stream of data to be evaluated.
> * A Synapse is a trigger condition based on the evaluation of the
> data streams (i.e. the Signals).
> * A Count is the accumulation of the effects of Synapses (i.e. the
> triggers).
>
> As such, in order to represent an event, you would add in a Signal to
> represent the stream of device events, and a Synapse defining the
> specific event that will trigger the action. I'll give an example in
> order to demonstrate what I mean.
>
> A simple clock can be conceptualize as a proper counter device: an
> oscillation is a Signal, a rising edge from that oscillation line can be
> the Synapse, and the current clock value is the Count.
>
> Count Synapse Signal
> ----- ------- ------
> +---------------------+
> | Data: Clock ticks | Rising Edge _____________
> | Function: Increase | <------------- / Oscillation \
> | | _________________
> +---------------------+
>
> Now, in order to represent your timestamping clock we need two Signals:
> a simple clock and an event stream. The simple clock is the source of
> the current clock ticks we will store, while the event stream provides
> the rotation count register read notification that will trigger the
> timestamp.
>
> Count Synapse Signal
> ----- ------- ------
> +-------------------------------+
> | Data: Timestamp | None _______
> | Function: Current clock ticks | <------------ / Clock \
> | | ___________
> | |
> | | Read event ________
> | | <------------ / Events \
> | | ____________
> +-------------------------------+
>
> Note that in this case both Signals either do not exist in or are not
> exposed by the hardware (maybe the simple clock is exposed, but it's not
> necessary to be) -- they are meant to be abstract representations of the
> components of the timestamp clock as an idealized "counter device".
>
> By organizing the timestamp clock in this way, we can control and
> configure the components using the standard Counter interface: common
> attributes such as "name", "preset", "enable", etc. can now be exposed
> to users like every other counter device component.
This way of looking at things makes very much sense. Thanks for the
detailed explanation.
>
> In theory we can sleep on the timestamp count attribute read (or
> character device equivalent if we go down that route), and be woken when
> an event triggers updating the timestamp value. However, the current
> Counter subsystem implementation is lacking the code for this so it
> needs to be added to the core functionality first.
>
>> When thinking about what events would actually do when enabled though, it
>> seems like we should be using IIO events and triggers (we have found reading
>> sysfs attributes to be insufficient performance-wise). It seems like unnecessary
>> work to reproduce all of this in the counter subsystem. Which makes me wonder if
>> it would be better to have counter devices just be a different device type (i.e.
>> different struct device_type for dev->type) in the IIO subsystem instead of
>> creating a completely new subsystem.
>
> I plan on adding interrupt support for the 104-QUAD-8 counter driver
> since this device has some useful interrupts on configured threshold
> conditions and such, so having the ability to handle an event rather
> than constantly read and loop is something I want to have in the Counter
> subsystem.
>
> It's possible that I can reuse some code from the IIO subsystem, as
> Jonathan pointed out, but overall I believe these should be separate
> subsystems. From the reasons described above, the IIO subsystem and
> Counter subsystem have different goals and thus different
> implementations. I don't think that's a bad thing, and we can share code
> in the few cases where the two may overlap.
>
> Regarding whether to use IIO events and triggers within the TI eQEP
> counter driver, I think we should wait for a proper Counter subsystem
> implementation to be added first. My fear is that we'll have a similar
> situation as what happened with IIO_COUNT, where we'll have to keep a
> IIO interface present with a newer Counter interface. If adding in event
> support to the Counter subsystem will take too long, we can add this TI
> eQEP driver as-is now and later add in the timestamp support.
I don't think we need triggers anymore since I now better understand
what a synapse does.
---
In summary, this has been a very helpful discussion. Back the the patch
series I have submitted, I think it still makes sense to merge it now
as-is (barring any serious issues) and the additional functionality we
have been discussing can be added in the future as the framework for it
is developed.
^ permalink raw reply
* Re: [PATCH 3/5] MIPS: lantiq: add an irq_domain and irq_chip for EBU
From: Martin Blumenstingl @ 2019-08-01 17:42 UTC (permalink / raw)
To: Marc Zyngier
Cc: tglx, jason, ralf, paul.burton, jhogan, robh+dt, linux-mips,
devicetree, linux-kernel, mark.rutland, john, Hauke Mehrtens
In-Reply-To: <86y30imq9p.wl-marc.zyngier@arm.com>
Hi Marc,
thank you for taking time to review this patch!
On Sun, Jul 28, 2019 at 12:01 PM Marc Zyngier <marc.zyngier@arm.com> wrote:
[...]
> > @@ -15,6 +19,19 @@
> >
> > #define LTQ_EBU_BUSCON0 0x0060
> > #define LTQ_EBU_BUSCON_WRDIS BIT(31)
> > +#define LTQ_EBU_PCC_CON 0x0090
> > +#define LTQ_EBU_PCC_CON_PCCARD_ON BIT(0)
> > +#define LTQ_EBU_PCC_CON_IREQ_RISING_EDGE 0x2
> > +#define LTQ_EBU_PCC_CON_IREQ_FALLING_EDGE 0x4
> > +#define LTQ_EBU_PCC_CON_IREQ_BOTH_EDGE 0x6
>
> So BOTH_EDGE is actually (RISING_EDGE | FALLING_EDGE). It'd be nice to
> express it this way.
I only notice this now - thank you for the hint
v2 will have this cleaned up
> > +#define LTQ_EBU_PCC_CON_IREQ_DIS 0x8
>
> What does "DIS" mean?
after reading all of your comments it may be "disable edge detection"
I don't have access to the datasheet but I'll ask someone at Intel (Lantiq)
> > +#define LTQ_EBU_PCC_CON_IREQ_HIGH_LEVEL_DETECT 0xa
> > +#define LTQ_EBU_PCC_CON_IREQ_LOW_LEVEL_DETECT 0xc
>
> Again, these two are (DIS | RISING) and (DIS | FALLING).
understood, v2 will use a better name for DIS (assuming there's a
better name) and I'll convert the macros based on your suggestion
[...]
> > + switch (flow_type & IRQ_TYPE_SENSE_MASK) {
> > + case IRQ_TYPE_NONE:
> > + val |= LTQ_EBU_PCC_CON_IREQ_DIS;
> > + break;
>
> I'm not sure IRQ_TYPE_NONE makes much sense here. What's the expected
> semantic?
if it's "disable edge detection" then this "case" will be removed
[...]
> > + default:
> > + pr_err("Invalid trigger mode %x for IRQ %d\n", flow_type,
> > + d->irq);
>
> irq_set_type will already complain in the kernel log, no need to add
> an extra message.
I'll drop this in v2, thank you for pointing this out
[...]
> > +static void ltq_ebu_irq_handler(struct irq_desc *desc)
> > +{
> > + struct irq_domain *domain = irq_desc_get_handler_data(desc);
> > + struct irq_chip *irqchip = irq_desc_get_chip(desc);
> > +
> > + chained_irq_enter(irqchip, desc);
> > +
> > + generic_handle_irq(irq_find_mapping(domain, 0));
>
> Having an irqdomain for a single interrupt is a bit over the top... Is
> that for the convenience of the DT infrastructure?
yes, I did it to get DT support
please let me know if there's a "better" way (preferably with another
driver as example)
[...]
> > + irq_create_mapping(domain, 0);
>
> Why do you need to perform this eagerly? I'd expect this interrupt to
> be mapped when it is actually claimed by a driver.
I don't remember why I added it, it may be left-over from copying from
another driver
in v2 I'll try to drop it
> > +
> > + irq_set_chained_handler_and_data(irq, ltq_ebu_irq_handler, domain);
>
> And there is no HW initialisation whatsoever? I'd expect, at the very
> least, the sole interrupt to be configured as disabled/masked.
I can add that. is there any "best practice" on what I should
initialize (just disable it or also set a "default" mode like
LEVEL_LOW)?
Martin
^ permalink raw reply
* Re: [PATCH v7 11/20] cpufreq: tegra124: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-08-01 17:51 UTC (permalink / raw)
To: Viresh Kumar
Cc: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland, pdeschrijver, pgaikwad,
sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
linux-kernel, mperttunen, spatra, robh+dt, digetx, devicetree,
rjw, linux-pm
In-Reply-To: <20190801054055.trmabmcaj3cpe4pc@vireshk-i7>
On 7/31/19 10:40 PM, Viresh Kumar wrote:
> On 31-07-19, 14:10, Sowjanya Komatineni wrote:
>> This patch adds suspend and resume pm ops for cpufreq driver.
>>
>> PLLP is the safe clock source for CPU during system suspend and
>> resume as PLLP rate is below the CPU Fmax at Vmin.
>>
>> CPUFreq driver suspend switches the CPU clock source to PLLP and
>> disables the DFLL clock.
>>
>> During system resume, warmboot code powers up the CPU with PLLP
>> clock source. So CPUFreq driver resume enabled DFLL clock and
>> switches CPU back to DFLL clock source.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/cpufreq/tegra124-cpufreq.c | 60 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 60 insertions(+)
> Is there any hard dependency of this patch on the rest of the patches?
> Can I apply it alone to cpufreq tree ?
>
This patch cannot be applied alone as it has dependency on dfll clock
suspend and resume sequence.
^ permalink raw reply
* Re: [PATCH v9 09/15] dt-bindings: memory: tegra30: Convert to Tegra124 YAML
From: Dmitry Osipenko @ 2019-08-01 17:52 UTC (permalink / raw)
To: Rob Herring
Cc: Michael Turquette, Joseph Lo, Thierry Reding, Jonathan Hunter,
Peter De Schrijver, Prashant Gaikwad, Stephen Boyd, devicetree,
linux-clk, linux-tegra, linux-kernel@vger.kernel.org
In-Reply-To: <CAL_JsqL_LA+cW2GbCMdzRTFuv2oKE3pzyOm2UwdzLVLyVTnmNw@mail.gmail.com>
01.08.2019 19:25, Rob Herring пишет:
> On Tue, Jul 30, 2019 at 10:58 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> The Tegra30 binding will actually differ from the Tegra124 a tad, in
>> particular the EMEM configuration description. Hence rename the binding
>> to Tegra124 during of the conversion to YAML.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>> .../nvidia,tegra124-mc.yaml | 158 ++++++++++++++++++
>> .../memory-controllers/nvidia,tegra30-mc.txt | 123 --------------
>> 2 files changed, 158 insertions(+), 123 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-mc.yaml
>> delete mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-mc.yaml
>> new file mode 100644
>> index 000000000000..2e2a116f1911
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-mc.yaml
>> @@ -0,0 +1,158 @@
>> +# SPDX-License-Identifier: (GPL-2.0)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/memory-controllers/nvidia,tegra124-mc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: NVIDIA Tegra124 SoC Memory Controller
>> +
>> +maintainers:
>> + - Jon Hunter <jonathanh@nvidia.com>
>> + - Thierry Reding <thierry.reding@gmail.com>
>> +
>> +description: |
>> + Tegra124 SoC features a hybrid 2x32-bit / 1x64-bit memory controller.
>> + These are interleaved to provide high performance with the load shared across
>> + two memory channels. The Tegra124 Memory Controller handles memory requests
>> + from internal clients and arbitrates among them to allocate memory bandwidth
>> + for DDR3L and LPDDR3 SDRAMs.
>> +
>> +properties:
>> + compatible:
>> + const: nvidia,tegra124-mc
>> +
>> + reg:
>> + maxItems: 1
>> + description:
>> + Physical base address.
>
> You don't really need a description when there's only 1 item. Same on
> the others below.
>
> With that,
>
> Reviewed-by: Rob Herring <robh@kernel.org>
Okay, I'll change that in the next revision. I also assume that the r-b
applies to all three patches, otherwise please let me know. Thanks!
^ permalink raw reply
* Re: [PATCH v7 10/20] clk: tegra: clk-dfll: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-08-01 17:53 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding@gmail.com, Jonathan Hunter,
tglx@linutronix.de, jason@lakedaemon.net, marc.zyngier@arm.com,
linus.walleij@linaro.org, stefan@agner.ch, mark.rutland@arm.com
Cc: Peter De Schrijver, Prashant Gaikwad, sboyd@kernel.org,
linux-clk@vger.kernel.org, linux-gpio@vger.kernel.org,
Jui Chang Kuo, Joseph Lo, Timo Alho, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, Mikko Perttunen, Sandipan Patra,
robh+dt@kernel.org, devicetree@vger.kernel.org, rjw@rjwysocki.net,
viresh.kumar@linaro.org, linux-pm@vger.kernel.org
In-Reply-To: <31990250-e237-ddb9-ce71-29b7c2302fc3@gmail.com>
On 8/1/19 10:10 AM, Dmitry Osipenko wrote:
> 01.08.2019 19:10, Sowjanya Komatineni пишет:
>> I didn’t updated any patches. This is still same v7 just resent with
>> CPUFreq maintainers in CC as I missed to add them earlier.
> There are now two different threads for the same patches, which is not
> very good. When I said that CPUFreq maintainers should be CC'ed, I
> didn't mean to resend it all, sorry for not being clear about it. You
> should've wait for more comments to the original patches and then make a
> v8. I suggest to do the same in the current situation as well, please
> address all the current comments and wait for 1-2 days, then make a v8.
Thought version bump is only when changes were made. Got it now.
Thanks Dmitry. Sure, will give some time for all feedbacks before
sending V8.
^ permalink raw reply
* Re: [PATCH v7 16/20] arm64: tegra: Enable wake from deep sleep on RTC alarm
From: Sowjanya Komatineni @ 2019-08-01 17:56 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <4f8b9ee7-5402-6463-d6d2-7b00e274a185@gmail.com>
On 8/1/19 3:43 AM, Dmitry Osipenko wrote:
> 01.08.2019 0:08, Sowjanya Komatineni пишет:
>> On 7/31/19 4:04 AM, Dmitry Osipenko wrote:
>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>> This patch updates device tree for RTC and PMC to allow system wake
>>>> from deep sleep on RTC alarm.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 ++++-
>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>> b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>> index 659753118e96..30a7c48385a2 100644
>>>> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>> @@ -768,7 +768,8 @@
>>>> rtc@7000e000 {
>>>> compatible = "nvidia,tegra210-rtc", "nvidia,tegra20-rtc";
>>>> reg = <0x0 0x7000e000 0x0 0x100>;
>>>> - interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
>>>> + interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
>>>> + interrupt-parent = <&pmc>;
>>>> clocks = <&tegra_car TEGRA210_CLK_RTC>;
>>>> clock-names = "rtc";
>>>> };
>>>> @@ -778,6 +779,8 @@
>>>> reg = <0x0 0x7000e400 0x0 0x400>;
>>>> clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
>>>> clock-names = "pclk", "clk32k_in";
>>>> + #interrupt-cells = <2>;
>>>> + interrupt-controller;
>>>> powergates {
>>>> pd_audio: aud {
>>>>
>>> Is this a backwards-compatible change? Or it's not really worth to care
>>> about the compatibility with older kernel versions, I'm not sure about
>>> overall state of T210 in the upstream kernel.
>> I don't think its required to be backwards-compatible as SC7 entry/exit
>> implementation for T210 is with this patch series onwards..
> The new device tree binary should work with older kernel versions, AFAIK
> this is the upstream rule. But if kernel support isn't in a very good
> shape and not much people are using it, then obviously it is not very
> important.
Yes, my response to backwards-compatible was with respect to interrupt
parent change as this will not be backward compatible and also there is
no Tegra210 suspend/resume earlier. Other functionality wise, it is
backward compatible.
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-01 17:58 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <67cf6c13-688d-0305-61e2-c63c8e8b4729@nvidia.com>
On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>
> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>> This patch implements save and restore context for peripheral fixed
>>>> clock ops, peripheral gate clock ops, sdmmc mux clock ops, and
>>>> peripheral clock ops.
>>>>
>>>> During system suspend, core power goes off and looses the settings
>>>> of the Tegra CAR controller registers.
>>>>
>>>> So during suspend entry clock and reset state of peripherals is saved
>>>> and on resume they are restored to have clocks back to same rate and
>>>> state as before suspend.
>>>>
>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>> ++++++++++++++++++++++++++++++++
>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>> +++++++++++++++++++++++++++++++++
>>>> drivers/clk/tegra/clk-periph.c | 37
>>>> ++++++++++++++++++++++++++++++++++++
>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>> +++++++++++++++++++++++++++
>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>> 5 files changed, 138 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>> index c088e7a280df..21b24530fa00 100644
>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>> @@ -60,11 +60,44 @@ tegra_clk_periph_fixed_recalc_rate(struct
>>>> clk_hw *hw,
>>>> return (unsigned long)rate;
>>>> }
>>>> +static int tegra_clk_periph_fixed_save_context(struct clk_hw *hw)
>>>> +{
>>>> + struct tegra_clk_periph_fixed *fixed =
>>>> to_tegra_clk_periph_fixed(hw);
>>>> + u32 mask = 1 << (fixed->num % 32);
>>>> +
>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>> fixed->regs->enb_reg) &
>>>> + mask;
>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>> fixed->regs->rst_reg) &
>>>> + mask;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void tegra_clk_periph_fixed_restore_context(struct clk_hw *hw)
>>>> +{
>>>> + struct tegra_clk_periph_fixed *fixed =
>>>> to_tegra_clk_periph_fixed(hw);
>>>> + u32 mask = 1 << (fixed->num % 32);
>>>> +
>>>> + if (fixed->enb_ctx)
>>>> + writel_relaxed(mask, fixed->base + fixed->regs->enb_set_reg);
>>>> + else
>>>> + writel_relaxed(mask, fixed->base + fixed->regs->enb_clr_reg);
>>>> +
>>>> + udelay(2);
>>>> +
>>>> + if (!fixed->rst_ctx) {
>>>> + udelay(5); /* reset propogation delay */
>>>> + writel_relaxed(mask, fixed->base + fixed->regs->rst_reg);
>>>> + }
>>>> +}
>>>> +
>>>> static const struct clk_ops tegra_clk_periph_fixed_ops = {
>>>> .is_enabled = tegra_clk_periph_fixed_is_enabled,
>>>> .enable = tegra_clk_periph_fixed_enable,
>>>> .disable = tegra_clk_periph_fixed_disable,
>>>> .recalc_rate = tegra_clk_periph_fixed_recalc_rate,
>>>> + .save_context = tegra_clk_periph_fixed_save_context,
>>>> + .restore_context = tegra_clk_periph_fixed_restore_context,
>>>> };
>>>> struct clk *tegra_clk_register_periph_fixed(const char *name,
>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>> @@ -25,6 +25,8 @@ static DEFINE_SPINLOCK(periph_ref_lock);
>>>> #define read_rst(gate) \
>>>> readl_relaxed(gate->clk_base + (gate->regs->rst_reg))
>>>> +#define write_rst_set(val, gate) \
>>>> + writel_relaxed(val, gate->clk_base + (gate->regs->rst_set_reg))
>>>> #define write_rst_clr(val, gate) \
>>>> writel_relaxed(val, gate->clk_base + (gate->regs->rst_clr_reg))
>>>> @@ -110,10 +112,42 @@ static void clk_periph_disable(struct
>>>> clk_hw *hw)
>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>> }
>>>> +static int clk_periph_gate_save_context(struct clk_hw *hw)
>>>> +{
>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>> +
>>>> + gate->clk_state_ctx = read_enb(gate) & periph_clk_to_bit(gate);
>>>> + gate->rst_state_ctx = read_rst(gate) & periph_clk_to_bit(gate);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void clk_periph_gate_restore_context(struct clk_hw *hw)
>>>> +{
>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>> +
>>>> + if (gate->clk_state_ctx)
>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>> + else
>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>> +
>>>> + udelay(5);
>>>> +
>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>> + if (gate->rst_state_ctx)
>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>> + else
>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>> + }
>>>> +}
>>>> +
>>>> const struct clk_ops tegra_clk_periph_gate_ops = {
>>>> .is_enabled = clk_periph_is_enabled,
>>>> .enable = clk_periph_enable,
>>>> .disable = clk_periph_disable,
>>>> + .save_context = clk_periph_gate_save_context,
>>>> + .restore_context = clk_periph_gate_restore_context,
>>>> };
>>>> struct clk *tegra_clk_register_periph_gate(const char *name,
>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>> b/drivers/clk/tegra/clk-periph.c
>>>> index 58437da25156..06fb62955768 100644
>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct clk_hw *hw)
>>>> gate_ops->disable(gate_hw);
>>>> }
>>>> +static int clk_periph_save_context(struct clk_hw *hw)
>>>> +{
>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>> +
>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>> + gate_ops->save_context(gate_hw);
>>>> +
>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void clk_periph_restore_context(struct clk_hw *hw)
>>>> +{
>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>> +
>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>> +
>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>> + div_ops->restore_context(div_hw);
>>> Could you please point to where the divider's save_context() happens?
>>> Because I can't see it.
>> Ah, I now see that there is no need to save the dividers context because
>> clk itself has enough info that is needed for the context's restoring
>> (like I pointed in the review to v6).
>>
>> Looks like you could also implement a new clk_hw_get_parent_index()
>> generic helper to get the index instead of storing it manually.
>
> clk_periph_get_parent basically invokes existing clk_mux_ops
> get_parent() which is then saved in tegra_clk_periph.
>
> All existing drivers are using directly get_parent() from clk_mux
> which actually gets index from the register read.
>
> To have this more generic w.r.t save/restore context point of view,
> probably instead of implementing new get_parent_index helper, I think
> its better to implement save_context and restore_context to
> clk_mux_ops along with creating parent_index field into clk_mux to
> cache index during set_parent.
>
> So we just need to invoke mux_ops save_context and restore_context.
>
I hope its ok to add save/restore context to clk_mux_ops to be more
generic w.r.t save/restore context rather than get_parent_index API.
Please confirm if you agree.
^ permalink raw reply
* Re: [PATCH v7 06/20] clk: tegra: Support for OSC context save and restore
From: Sowjanya Komatineni @ 2019-08-01 18:06 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <3b55a112-42a6-212b-beea-10b64d5341d9@gmail.com>
On 8/1/19 3:53 AM, Dmitry Osipenko wrote:
> 01.08.2019 0:04, Sowjanya Komatineni пишет:
>> On 7/31/19 4:11 AM, Dmitry Osipenko wrote:
>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>> This patch adds support for saving OSC clock frequency and the
>>>> drive-strength during OSC clock init and creates an API to restore
>>>> OSC control register value from the saved context.
>>>>
>>>> This API is invoked by Tegra210 clock driver during system resume
>>>> to restore the OSC clock settings.
>>>>
>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> drivers/clk/tegra/clk-tegra-fixed.c | 15 +++++++++++++++
>>>> drivers/clk/tegra/clk.h | 1 +
>>>> 2 files changed, 16 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/tegra/clk-tegra-fixed.c
>>>> b/drivers/clk/tegra/clk-tegra-fixed.c
>>>> index 8d91b2b191cf..7c6c8abfcde6 100644
>>>> --- a/drivers/clk/tegra/clk-tegra-fixed.c
>>>> +++ b/drivers/clk/tegra/clk-tegra-fixed.c
>>>> @@ -17,6 +17,10 @@
>>>> #define OSC_CTRL 0x50
>>>> #define OSC_CTRL_OSC_FREQ_SHIFT 28
>>>> #define OSC_CTRL_PLL_REF_DIV_SHIFT 26
>>>> +#define OSC_CTRL_MASK (0x3f2 | \
>>>> + (0xf << OSC_CTRL_OSC_FREQ_SHIFT))
>>>> +
>>>> +static u32 osc_ctrl_ctx;
>>>> int __init tegra_osc_clk_init(void __iomem *clk_base, struct
>>>> tegra_clk *clks,
>>>> unsigned long *input_freqs, unsigned int num,
>>>> @@ -29,6 +33,7 @@ int __init tegra_osc_clk_init(void __iomem
>>>> *clk_base, struct tegra_clk *clks,
>>>> unsigned osc_idx;
>>>> val = readl_relaxed(clk_base + OSC_CTRL);
>>>> + osc_ctrl_ctx = val & OSC_CTRL_MASK;
>>>> osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;
>>>> if (osc_idx < num)
>>>> @@ -96,3 +101,13 @@ void __init tegra_fixed_clk_init(struct tegra_clk
>>>> *tegra_clks)
>>>> *dt_clk = clk;
>>>> }
>>>> }
>>>> +
>>>> +void tegra_clk_osc_resume(void __iomem *clk_base)
>>>> +{
>>>> + u32 val;
>>>> +
>>>> + val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
>>>> + val |= osc_ctrl_ctx;
>>>> + writel_relaxed(val, clk_base + OSC_CTRL);
>>> Why a full raw u32 OSC_CTRL value couldn't be simply saved and restored?
>> Storing and restoring only required fields to avoid accidental
>> misconfiguration.
>>
>> OSC_CTRL register has other bits (PLL_REF_DIV) which are configured by
>> BR depending on OSC_FREQ and also setting PLL_REF_DIV while PLLS are in
>> use is not safe.
> I'm looking at the clk-driver sources and see that none of the Tegra
> drivers ever change the OSC_CTRL configuration, T30/114 even have
> #defines for the OSC_CTRL that are unused.
>
> So, this leads to a question.. does any bootloader really ever change
> the OSC_CTRL such that it differs after resume from suspend in
> comparison to the value at the time of kernel's booting up?
For Tegra210, bootloader programs OSC_CTRL register for drivestrength
programming.
These settings need to be restored to the same on SC7 exit as they gets
reset during SC7 entry.
^ permalink raw reply
* Re: [PATCH v9 09/15] dt-bindings: memory: tegra30: Convert to Tegra124 YAML
From: Rob Herring @ 2019-08-01 18:06 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Michael Turquette, Joseph Lo, Thierry Reding, Jonathan Hunter,
Peter De Schrijver, Prashant Gaikwad, Stephen Boyd, devicetree,
linux-clk, linux-tegra, linux-kernel@vger.kernel.org
In-Reply-To: <58e25c92-a61d-54f1-e784-ed85804236d7@gmail.com>
On Thu, Aug 1, 2019 at 11:52 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> 01.08.2019 19:25, Rob Herring пишет:
> > On Tue, Jul 30, 2019 at 10:58 AM Dmitry Osipenko <digetx@gmail.com> wrote:
> >>
> >> The Tegra30 binding will actually differ from the Tegra124 a tad, in
> >> particular the EMEM configuration description. Hence rename the binding
> >> to Tegra124 during of the conversion to YAML.
> >>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---
> >> .../nvidia,tegra124-mc.yaml | 158 ++++++++++++++++++
> >> .../memory-controllers/nvidia,tegra30-mc.txt | 123 --------------
> >> 2 files changed, 158 insertions(+), 123 deletions(-)
> >> create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-mc.yaml
> >> delete mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-mc.yaml
> >> new file mode 100644
> >> index 000000000000..2e2a116f1911
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-mc.yaml
> >> @@ -0,0 +1,158 @@
> >> +# SPDX-License-Identifier: (GPL-2.0)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/memory-controllers/nvidia,tegra124-mc.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: NVIDIA Tegra124 SoC Memory Controller
> >> +
> >> +maintainers:
> >> + - Jon Hunter <jonathanh@nvidia.com>
> >> + - Thierry Reding <thierry.reding@gmail.com>
> >> +
> >> +description: |
> >> + Tegra124 SoC features a hybrid 2x32-bit / 1x64-bit memory controller.
> >> + These are interleaved to provide high performance with the load shared across
> >> + two memory channels. The Tegra124 Memory Controller handles memory requests
> >> + from internal clients and arbitrates among them to allocate memory bandwidth
> >> + for DDR3L and LPDDR3 SDRAMs.
> >> +
> >> +properties:
> >> + compatible:
> >> + const: nvidia,tegra124-mc
> >> +
> >> + reg:
> >> + maxItems: 1
> >> + description:
> >> + Physical base address.
> >
> > You don't really need a description when there's only 1 item. Same on
> > the others below.
> >
> > With that,
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
>
> Okay, I'll change that in the next revision. I also assume that the r-b
> applies to all three patches, otherwise please let me know. Thanks!
No. I'm reviewing those.
Rob
^ permalink raw reply
* Re: [PATCH v9 11/15] dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory Controller
From: Rob Herring @ 2019-08-01 18:11 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Michael Turquette, Joseph Lo, Thierry Reding, Jonathan Hunter,
Peter De Schrijver, Prashant Gaikwad, Stephen Boyd, devicetree,
linux-clk, linux-tegra, linux-kernel@vger.kernel.org
In-Reply-To: <20190730165618.10122-12-digetx@gmail.com>
On Tue, Jul 30, 2019 at 10:58 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> Add device-tree binding for NVIDIA Tegra30 External Memory Controller.
> The binding is based on the Tegra124 EMC binding since hardware is
> similar, although there are couple significant differences.
>
> Note that the memory timing description is given in a platform-specific
> form because there is no detailed information on how to convert a
> typical-common DDR timing into the register values. The timing format is
> borrowed from downstream kernel, hence there is no hurdle in regards to
> upstreaming of memory timings for the boards.
>
> Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> .../nvidia,tegra30-emc.yaml | 341 ++++++++++++++++++
> 1 file changed, 341 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
>
> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
> new file mode 100644
> index 000000000000..6865cfb16e59
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
> @@ -0,0 +1,341 @@
> +# SPDX-License-Identifier: (GPL-2.0)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/nvidia,tegra30-emc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVIDIA Tegra30 SoC External Memory Controller
> +
> +maintainers:
> + - Dmitry Osipenko <digetx@gmail.com>
> + - Jon Hunter <jonathanh@nvidia.com>
> + - Thierry Reding <thierry.reding@gmail.com>
> +
> +description: |
> + The EMC interfaces with the off-chip SDRAM to service the request stream
> + sent from Memory Controller. The EMC also has various performance-affecting
> + settings beyond the obvious SDRAM configuration parameters and initialization
> + settings. Tegra30 EMC supports multiple JEDEC standard protocols: LPDDR2,
> + LPDDR3, and DDR3.
> +
> +properties:
> + compatible:
> + const: nvidia,tegra30-emc
> +
> + reg:
> + maxItems: 1
> + description:
> + Physical base address.
Same comment here.
> +
> + clocks:
> + maxItems: 1
> + description:
> + EMC clock.
> +
> + interrupts:
> + maxItems: 1
> + description:
> + EMC General interrupt.
> +
> + nvidia,memory-controller:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle of the Memory Controller node.
> +
> +patternProperties:
> + "^emc-timings-[0-9]+$":
> + type: object
> + properties:
> + nvidia,ram-code:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Value of RAM_CODE this timing set is used for.
> +
> + patternProperties:
> + "^timing-[0-9]+$":
> + type: object
> + properties:
> + clock-frequency:
> + description:
> + Memory clock rate in Hz.
> + minimum: 1000000
> + maximum: 900000000
> +
> + nvidia,emc-auto-cal-interval:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Pad calibration interval.
Any value 0 - 4G is valid?
> +
> + nvidia,emc-mode-1:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Mode Register 1.
> +
> + nvidia,emc-mode-2:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Mode Register 2.
> +
> + nvidia,emc-mode-reset:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Mode Register 0.
> +
> + nvidia,emc-zcal-cnt-long:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Number of EMC clocks to wait before issuing any commands after
> + sending ZCAL_MRW_CMD.
Valid range?
> +
> + nvidia,emc-cfg-dyn-self-ref:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Dynamic self-refresh enabled.
Sounds like a boolean?
> +
> + nvidia,emc-cfg-periodic-qrst:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + FBIO "read" FIFO periodic resetting enabled.
boolean?
> +
> + nvidia,emc-configuration:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description:
> + EMC timing characterization data. These are the registers
> + (see section "18.13.2 EMC Registers" in the TRM) whose values
> + need to be specified, according to the board documentation.
> + items:
> + - description: EMC_RC
> + - description: EMC_RFC
> + - description: EMC_RAS
> + - description: EMC_RP
> + - description: EMC_R2W
> + - description: EMC_W2R
> + - description: EMC_R2P
> + - description: EMC_W2P
> + - description: EMC_RD_RCD
> + - description: EMC_WR_RCD
> + - description: EMC_RRD
> + - description: EMC_REXT
> + - description: EMC_WEXT
> + - description: EMC_WDV
> + - description: EMC_QUSE
> + - description: EMC_QRST
> + - description: EMC_QSAFE
> + - description: EMC_RDV
> + - description: EMC_REFRESH
> + - description: EMC_BURST_REFRESH_NUM
> + - description: EMC_PRE_REFRESH_REQ_CNT
> + - description: EMC_PDEX2WR
> + - description: EMC_PDEX2RD
> + - description: EMC_PCHG2PDEN
> + - description: EMC_ACT2PDEN
> + - description: EMC_AR2PDEN
> + - description: EMC_RW2PDEN
> + - description: EMC_TXSR
> + - description: EMC_TXSRDLL
> + - description: EMC_TCKE
> + - description: EMC_TFAW
> + - description: EMC_TRPAB
> + - description: EMC_TCLKSTABLE
> + - description: EMC_TCLKSTOP
> + - description: EMC_TREFBW
> + - description: EMC_QUSE_EXTRA
> + - description: EMC_FBIO_CFG6
> + - description: EMC_ODT_WRITE
> + - description: EMC_ODT_READ
> + - description: EMC_FBIO_CFG5
> + - description: EMC_CFG_DIG_DLL
> + - description: EMC_CFG_DIG_DLL_PERIOD
> + - description: EMC_DLL_XFORM_DQS0
> + - description: EMC_DLL_XFORM_DQS1
> + - description: EMC_DLL_XFORM_DQS2
> + - description: EMC_DLL_XFORM_DQS3
> + - description: EMC_DLL_XFORM_DQS4
> + - description: EMC_DLL_XFORM_DQS5
> + - description: EMC_DLL_XFORM_DQS6
> + - description: EMC_DLL_XFORM_DQS7
> + - description: EMC_DLL_XFORM_QUSE0
> + - description: EMC_DLL_XFORM_QUSE1
> + - description: EMC_DLL_XFORM_QUSE2
> + - description: EMC_DLL_XFORM_QUSE3
> + - description: EMC_DLL_XFORM_QUSE4
> + - description: EMC_DLL_XFORM_QUSE5
> + - description: EMC_DLL_XFORM_QUSE6
> + - description: EMC_DLL_XFORM_QUSE7
> + - description: EMC_DLI_TRIM_TXDQS0
> + - description: EMC_DLI_TRIM_TXDQS1
> + - description: EMC_DLI_TRIM_TXDQS2
> + - description: EMC_DLI_TRIM_TXDQS3
> + - description: EMC_DLI_TRIM_TXDQS4
> + - description: EMC_DLI_TRIM_TXDQS5
> + - description: EMC_DLI_TRIM_TXDQS6
> + - description: EMC_DLI_TRIM_TXDQS7
> + - description: EMC_DLL_XFORM_DQ0
> + - description: EMC_DLL_XFORM_DQ1
> + - description: EMC_DLL_XFORM_DQ2
> + - description: EMC_DLL_XFORM_DQ3
> + - description: EMC_XM2CMDPADCTRL
> + - description: EMC_XM2DQSPADCTRL2
> + - description: EMC_XM2DQPADCTRL2
> + - description: EMC_XM2CLKPADCTRL
> + - description: EMC_XM2COMPPADCTRL
> + - description: EMC_XM2VTTGENPADCTRL
> + - description: EMC_XM2VTTGENPADCTRL2
> + - description: EMC_XM2QUSEPADCTRL
> + - description: EMC_XM2DQSPADCTRL3
> + - description: EMC_CTT_TERM_CTRL
> + - description: EMC_ZCAL_INTERVAL
> + - description: EMC_ZCAL_WAIT_CNT
> + - description: EMC_MRS_WAIT_CNT
> + - description: EMC_AUTO_CAL_CONFIG
> + - description: EMC_CTT
> + - description: EMC_CTT_DURATION
> + - description: EMC_DYN_SELF_REF_CONTROL
> + - description: EMC_FBIO_SPARE
> + - description: EMC_CFG_RSV
> +
> + required:
> + - clock-frequency
> + - nvidia,emc-auto-cal-interval
> + - nvidia,emc-mode-1
> + - nvidia,emc-mode-2
> + - nvidia,emc-mode-reset
> + - nvidia,emc-zcal-cnt-long
> + - nvidia,emc-cfg-dyn-self-ref
> + - nvidia,emc-cfg-periodic-qrst
> + - nvidia,emc-configuration
> +
> + additionalProperties: false
> +
> + required:
> + - nvidia,ram-code
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - nvidia,memory-controller
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + external-memory-controller@7000f400 {
> + compatible = "nvidia,tegra30-emc";
> + reg = <0x7000f400 0x400>;
> + interrupts = <0 78 4>;
> + clocks = <&tegra_car 57>;
> +
> + nvidia,memory-controller = <&mc>;
> +
> + emc-timings-1 {
> + nvidia,ram-code = <1>;
> +
> + timing-667000000 {
> + clock-frequency = <667000000>;
> +
> + nvidia,emc-auto-cal-interval = <0x001fffff>;
> + nvidia,emc-mode-1 = <0x80100002>;
> + nvidia,emc-mode-2 = <0x80200018>;
> + nvidia,emc-mode-reset = <0x80000b71>;
> + nvidia,emc-zcal-cnt-long = <0x00000040>;
> + nvidia,emc-cfg-dyn-self-ref = <0x00000000>;
> + nvidia,emc-cfg-periodic-qrst = <0x00000001>;
> +
> + nvidia,emc-configuration = <
> + 0x00000020 /* EMC_RC */
> + 0x0000006a /* EMC_RFC */
> + 0x00000017 /* EMC_RAS */
> + 0x00000007 /* EMC_RP */
> + 0x00000005 /* EMC_R2W */
> + 0x0000000c /* EMC_W2R */
> + 0x00000003 /* EMC_R2P */
> + 0x00000011 /* EMC_W2P */
> + 0x00000007 /* EMC_RD_RCD */
> + 0x00000007 /* EMC_WR_RCD */
> + 0x00000002 /* EMC_RRD */
> + 0x00000001 /* EMC_REXT */
> + 0x00000000 /* EMC_WEXT */
> + 0x00000007 /* EMC_WDV */
> + 0x0000000a /* EMC_QUSE */
> + 0x00000009 /* EMC_QRST */
> + 0x0000000b /* EMC_QSAFE */
> + 0x00000011 /* EMC_RDV */
> + 0x00001412 /* EMC_REFRESH */
> + 0x00000000 /* EMC_BURST_REFRESH_NUM */
> + 0x00000504 /* EMC_PRE_REFRESH_REQ_CNT */
> + 0x00000002 /* EMC_PDEX2WR */
> + 0x0000000e /* EMC_PDEX2RD */
> + 0x00000001 /* EMC_PCHG2PDEN */
> + 0x00000000 /* EMC_ACT2PDEN */
> + 0x0000000c /* EMC_AR2PDEN */
> + 0x00000016 /* EMC_RW2PDEN */
> + 0x00000072 /* EMC_TXSR */
> + 0x00000200 /* EMC_TXSRDLL */
> + 0x00000005 /* EMC_TCKE */
> + 0x00000015 /* EMC_TFAW */
> + 0x00000000 /* EMC_TRPAB */
> + 0x00000006 /* EMC_TCLKSTABLE */
> + 0x00000007 /* EMC_TCLKSTOP */
> + 0x00001453 /* EMC_TREFBW */
> + 0x0000000b /* EMC_QUSE_EXTRA */
> + 0x00000006 /* EMC_FBIO_CFG6 */
> + 0x00000000 /* EMC_ODT_WRITE */
> + 0x00000000 /* EMC_ODT_READ */
> + 0x00005088 /* EMC_FBIO_CFG5 */
> + 0xf00b0191 /* EMC_CFG_DIG_DLL */
> + 0x00008000 /* EMC_CFG_DIG_DLL_PERIOD */
> + 0x00000008 /* EMC_DLL_XFORM_DQS0 */
> + 0x00000008 /* EMC_DLL_XFORM_DQS1 */
> + 0x00000008 /* EMC_DLL_XFORM_DQS2 */
> + 0x00000008 /* EMC_DLL_XFORM_DQS3 */
> + 0x0000000a /* EMC_DLL_XFORM_DQS4 */
> + 0x0000000a /* EMC_DLL_XFORM_DQS5 */
> + 0x0000000a /* EMC_DLL_XFORM_DQS6 */
> + 0x0000000a /* EMC_DLL_XFORM_DQS7 */
> + 0x00018000 /* EMC_DLL_XFORM_QUSE0 */
> + 0x00018000 /* EMC_DLL_XFORM_QUSE1 */
> + 0x00018000 /* EMC_DLL_XFORM_QUSE2 */
> + 0x00018000 /* EMC_DLL_XFORM_QUSE3 */
> + 0x00000000 /* EMC_DLL_XFORM_QUSE4 */
> + 0x00000000 /* EMC_DLL_XFORM_QUSE5 */
> + 0x00000000 /* EMC_DLL_XFORM_QUSE6 */
> + 0x00000000 /* EMC_DLL_XFORM_QUSE7 */
> + 0x00000000 /* EMC_DLI_TRIM_TXDQS0 */
> + 0x00000000 /* EMC_DLI_TRIM_TXDQS1 */
> + 0x00000000 /* EMC_DLI_TRIM_TXDQS2 */
> + 0x00000000 /* EMC_DLI_TRIM_TXDQS3 */
> + 0x00000000 /* EMC_DLI_TRIM_TXDQS4 */
> + 0x00000000 /* EMC_DLI_TRIM_TXDQS5 */
> + 0x00000000 /* EMC_DLI_TRIM_TXDQS6 */
> + 0x00000000 /* EMC_DLI_TRIM_TXDQS7 */
> + 0x0000000a /* EMC_DLL_XFORM_DQ0 */
> + 0x0000000a /* EMC_DLL_XFORM_DQ1 */
> + 0x0000000a /* EMC_DLL_XFORM_DQ2 */
> + 0x0000000a /* EMC_DLL_XFORM_DQ3 */
> + 0x000002a0 /* EMC_XM2CMDPADCTRL */
> + 0x0800013d /* EMC_XM2DQSPADCTRL2 */
> + 0x22220000 /* EMC_XM2DQPADCTRL2 */
> + 0x77fff884 /* EMC_XM2CLKPADCTRL */
> + 0x01f1f501 /* EMC_XM2COMPPADCTRL */
> + 0x07077404 /* EMC_XM2VTTGENPADCTRL */
> + 0x54000000 /* EMC_XM2VTTGENPADCTRL2 */
> + 0x080001e8 /* EMC_XM2QUSEPADCTRL */
> + 0x0c000021 /* EMC_XM2DQSPADCTRL3 */
> + 0x00000802 /* EMC_CTT_TERM_CTRL */
> + 0x00020000 /* EMC_ZCAL_INTERVAL */
> + 0x00000100 /* EMC_ZCAL_WAIT_CNT */
> + 0x0155000c /* EMC_MRS_WAIT_CNT */
> + 0xa0f10000 /* EMC_AUTO_CAL_CONFIG */
> + 0x00000000 /* EMC_CTT */
> + 0x00000000 /* EMC_CTT_DURATION */
> + 0x800028a5 /* EMC_DYN_SELF_REF_CONTROL */
> + 0xe8000000 /* EMC_FBIO_SPARE */
> + 0xff00ff49 /* EMC_CFG_RSV */
> + >;
> + };
> + };
> + };
> --
> 2.22.0
>
^ permalink raw reply
* Re: [PATCH v2 4/6] irqchip/irq-pruss-intc: Add helper functions to configure internal mapping
From: David Lechner @ 2019-08-01 18:31 UTC (permalink / raw)
To: Suman Anna, Marc Zyngier, Thomas Gleixner, Jason Cooper
Cc: Rob Herring, Tony Lindgren, Andrew F. Davis, Roger Quadros,
Lokesh Vutla, Grygorii Strashko, Sekhar Nori, Murali Karicheri,
devicetree, linux-omap, linux-arm-kernel, linux-kernel
In-Reply-To: <89abc27f-5d02-a8ce-df0e-b185c2a647cd@ti.com>
On 8/1/19 12:10 PM, Suman Anna wrote:
> Hi Marc,
>
> On 8/1/19 3:45 AM, Marc Zyngier wrote:
>> On 31/07/2019 23:41, Suman Anna wrote:
>>> The PRUSS INTC receives a number of system input interrupt source events
>>> and supports individual control configuration and hardware prioritization.
>>> These input events can be mapped to some output interrupt lines through 2
>>> levels of many-to-one mapping i.e. events to channel mapping and channels
>>> to output interrupts.
>>>
>>> This mapping information is provided through the PRU firmware that is
>>> loaded onto a PRU core/s or through the device tree node of the PRU
>>> application. The mapping is configured by the PRU remoteproc driver, and
>>> is setup before the PRU core is started and cleaned up after the PRU core
>>> is stopped. This event mapping configuration logic programs the Channel
>>> Map Registers (CMRx) and Host-Interrupt Map Registers (HMRx) only when a
>>> new program is being loaded/started and the same events and interrupt
>>> channels are reset to zero when stopping a PRU.
>>>
>>> Add two helper functions: pruss_intc_configure() & pruss_intc_unconfigure()
>>> that the PRU remoteproc driver can use to configure the PRUSS INTC.
>>
>> So let me see if I correctly understand this: this adds yet another
>> firmware description parser, with a private interface to another
>> (undisclosed?) driver, bypassing the standard irqchip configuration
>> mechanism. It sounds great, doesn't it?
>>
>> What I cannot really infer from this message (-ETOOMUCHJARGON) is what
>> interrupts this affects:
>>
>> - Interrupts from random devices to the PRUSS?
>> - Interrupts from the PRUSS to the host?
>> - Something else?
>
> The interrupt sources (called system events) can be from internal PRUSS
> peripherals, SoC-level peripherals or just software triggering (limited
> to some events).
>
> So, the PRUSS INTC behaves as a funnel and is both an interrupt router
> and multiplexer. The INTC itself is part of the PRUSS, and all PRU
> application related interrupts/events that need to trigger an interrupt
> to either the PRU cores or other host processors (like DSP, ARM) have to
> go through this INTC, and routed out to a limited number of output
> interrupts that are then connected to different processors.
>
> The split of interrupt handling between a PRU and its peer host
> processor will be a application design choice (We can implement soft IPs
> like UARTs, ADCs, I2Cs etc using PRUs). Some of the input events
> themselves are multiplexed and controlled by a single MMR (outside of
> INTC) that feeds different sets of events into the INTC. The MMR
> configuration is outside of scope of this driver and will depend on the
> application/client driver being run.
>
>>
>> When does this happen? Under control of what? It isn't even clear why
>> this is part of this irqchip driver.
>
> The mapping configuration is per PRU application and firmware, and is
> done in line with acquiring and release a PRU which is treated as an
> exclusive resource. We establish the mapping for all events through this
> driver including the events that are to be routed to PRUs. This is done
> to save the tiny/limited Instruction RAM space that PRUs have.
>
> We have designed this as an irqchip driver (instead of some custom SoC
> driver exporting custom functions) to use standard Linux semantics/irq
> API and better integrate with Linux DT, but we need some semantics for
> establishing the routing at runtime depending on the PRU client driver
> we are running. The exported functions will be called only by the PRU
> remoteproc driver during a pru_rproc_get()/pru_rproc_put(), and are
> transparent to PRU client drivers.
>
> Please also see the discussion from v1 [1] on why we can't use an
> extended number of interrupt-cells infrastructure for achieving this.
>
> [1] https://patchwork.kernel.org/patch/11034563/
>
>
>> Depending what this does, there may be ways to fit it into the standard
>> interrupt configuration framework. After all, we already have standard
>> interfaces to route interrupts to virtual CPUs, effectively passing full
>> control of an interrupt to another entity. If you squint hard enough,
>> your PRUSS can fit that description.
>
> Yeah, I am open to suggestions if there is a better way of doing this.
Hi Suman,
Can you explain more about the use case where one PRU system event is
mapped to multiple host events?
I have an idea that we can use multiple struct irq_domains to make this
work in the existing IRQ framework, but it would be helpful to know more
about the bigger picture first.
>
> regards
> Suman
>
>>
>> If that doesn't work, then we need to make the IRQ framework grok that
>> kind of requirement (hence my request for clarification). But I'm
>> strongly opposed to inventing a SoC-private way of configuring
>> interrupts behind the kernel's back.
>>
>> Thanks,
>>
>> M.
>>
>
^ permalink raw reply
* Re: [PATCH v9 11/15] dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory Controller
From: Dmitry Osipenko @ 2019-08-01 18:35 UTC (permalink / raw)
To: Rob Herring
Cc: Michael Turquette, Joseph Lo, Thierry Reding, Jonathan Hunter,
Peter De Schrijver, Prashant Gaikwad, Stephen Boyd, devicetree,
linux-clk, linux-tegra, linux-kernel@vger.kernel.org
In-Reply-To: <CAL_JsqJgENCznrnYX8ARia2sNKJ7XxYRzzavk5qoePPYzYAQvA@mail.gmail.com>
01.08.2019 21:11, Rob Herring пишет:
> On Tue, Jul 30, 2019 at 10:58 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> Add device-tree binding for NVIDIA Tegra30 External Memory Controller.
>> The binding is based on the Tegra124 EMC binding since hardware is
>> similar, although there are couple significant differences.
>>
>> Note that the memory timing description is given in a platform-specific
>> form because there is no detailed information on how to convert a
>> typical-common DDR timing into the register values. The timing format is
>> borrowed from downstream kernel, hence there is no hurdle in regards to
>> upstreaming of memory timings for the boards.
>>
>> Acked-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>> .../nvidia,tegra30-emc.yaml | 341 ++++++++++++++++++
>> 1 file changed, 341 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
>> new file mode 100644
>> index 000000000000..6865cfb16e59
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
>> @@ -0,0 +1,341 @@
>> +# SPDX-License-Identifier: (GPL-2.0)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/memory-controllers/nvidia,tegra30-emc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: NVIDIA Tegra30 SoC External Memory Controller
>> +
>> +maintainers:
>> + - Dmitry Osipenko <digetx@gmail.com>
>> + - Jon Hunter <jonathanh@nvidia.com>
>> + - Thierry Reding <thierry.reding@gmail.com>
>> +
>> +description: |
>> + The EMC interfaces with the off-chip SDRAM to service the request stream
>> + sent from Memory Controller. The EMC also has various performance-affecting
>> + settings beyond the obvious SDRAM configuration parameters and initialization
>> + settings. Tegra30 EMC supports multiple JEDEC standard protocols: LPDDR2,
>> + LPDDR3, and DDR3.
>> +
>> +properties:
>> + compatible:
>> + const: nvidia,tegra30-emc
>> +
>> + reg:
>> + maxItems: 1
>> + description:
>> + Physical base address.
>
> Same comment here.
>
>> +
>> + clocks:
>> + maxItems: 1
>> + description:
>> + EMC clock.
>> +
>> + interrupts:
>> + maxItems: 1
>> + description:
>> + EMC General interrupt.
>> +
>> + nvidia,memory-controller:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description:
>> + Phandle of the Memory Controller node.
>> +
>> +patternProperties:
>> + "^emc-timings-[0-9]+$":
>> + type: object
>> + properties:
>> + nvidia,ram-code:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + Value of RAM_CODE this timing set is used for.
>> +
>> + patternProperties:
>> + "^timing-[0-9]+$":
>> + type: object
>> + properties:
>> + clock-frequency:
>> + description:
>> + Memory clock rate in Hz.
>> + minimum: 1000000
>> + maximum: 900000000
>> +
>> + nvidia,emc-auto-cal-interval:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + Pad calibration interval.
>
> Any value 0 - 4G is valid?
No, this is in microseconds and the maximum is 2 seconds.
>> +
>> + nvidia,emc-mode-1:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + Mode Register 1.
>> +
>> + nvidia,emc-mode-2:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + Mode Register 2.
>> +
>> + nvidia,emc-mode-reset:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + Mode Register 0.
>> +
>> + nvidia,emc-zcal-cnt-long:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + Number of EMC clocks to wait before issuing any commands after
>> + sending ZCAL_MRW_CMD.
>
> Valid range?
I'll add all the ranges in the next revision.
>> +
>> + nvidia,emc-cfg-dyn-self-ref:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + Dynamic self-refresh enabled.
>
> Sounds like a boolean?
>
>> +
>> + nvidia,emc-cfg-periodic-qrst:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + FBIO "read" FIFO periodic resetting enabled.
>
> boolean?
Yes, and it looked to me that it should be okay since that's what T124
binding uses and it makes the properties parsing in the driver's code a
bit neater :) I'll change them to booleans in the next revision to make
it clear.
^ permalink raw reply
* Re: [PATCH v7 16/20] arm64: tegra: Enable wake from deep sleep on RTC alarm
From: Dmitry Osipenko @ 2019-08-01 18:39 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <1e2e6282-9d94-e322-c4ba-8a1f3b902860@nvidia.com>
01.08.2019 20:56, Sowjanya Komatineni пишет:
>
> On 8/1/19 3:43 AM, Dmitry Osipenko wrote:
>> 01.08.2019 0:08, Sowjanya Komatineni пишет:
>>> On 7/31/19 4:04 AM, Dmitry Osipenko wrote:
>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>> This patch updates device tree for RTC and PMC to allow system wake
>>>>> from deep sleep on RTC alarm.
>>>>>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 ++++-
>>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>>> b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>>> index 659753118e96..30a7c48385a2 100644
>>>>> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>>> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>>> @@ -768,7 +768,8 @@
>>>>> rtc@7000e000 {
>>>>> compatible = "nvidia,tegra210-rtc", "nvidia,tegra20-rtc";
>>>>> reg = <0x0 0x7000e000 0x0 0x100>;
>>>>> - interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
>>>>> + interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
>>>>> + interrupt-parent = <&pmc>;
>>>>> clocks = <&tegra_car TEGRA210_CLK_RTC>;
>>>>> clock-names = "rtc";
>>>>> };
>>>>> @@ -778,6 +779,8 @@
>>>>> reg = <0x0 0x7000e400 0x0 0x400>;
>>>>> clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
>>>>> clock-names = "pclk", "clk32k_in";
>>>>> + #interrupt-cells = <2>;
>>>>> + interrupt-controller;
>>>>> powergates {
>>>>> pd_audio: aud {
>>>>>
>>>> Is this a backwards-compatible change? Or it's not really worth to care
>>>> about the compatibility with older kernel versions, I'm not sure about
>>>> overall state of T210 in the upstream kernel.
>>> I don't think its required to be backwards-compatible as SC7 entry/exit
>>> implementation for T210 is with this patch series onwards..
>> The new device tree binary should work with older kernel versions, AFAIK
>> this is the upstream rule. But if kernel support isn't in a very good
>> shape and not much people are using it, then obviously it is not very
>> important.
>
> Yes, my response to backwards-compatible was with respect to interrupt
> parent change as this will not be backward compatible and also there is
> no Tegra210 suspend/resume earlier. Other functionality wise, it is
> backward compatible.
Should be good then.
^ permalink raw reply
* Re: [PATCH v7 06/20] clk: tegra: Support for OSC context save and restore
From: Dmitry Osipenko @ 2019-08-01 18:42 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <cd9c5116-4235-758c-7a09-d7185d52b22b@nvidia.com>
01.08.2019 21:06, Sowjanya Komatineni пишет:
>
> On 8/1/19 3:53 AM, Dmitry Osipenko wrote:
>> 01.08.2019 0:04, Sowjanya Komatineni пишет:
>>> On 7/31/19 4:11 AM, Dmitry Osipenko wrote:
>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>> This patch adds support for saving OSC clock frequency and the
>>>>> drive-strength during OSC clock init and creates an API to restore
>>>>> OSC control register value from the saved context.
>>>>>
>>>>> This API is invoked by Tegra210 clock driver during system resume
>>>>> to restore the OSC clock settings.
>>>>>
>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>> drivers/clk/tegra/clk-tegra-fixed.c | 15 +++++++++++++++
>>>>> drivers/clk/tegra/clk.h | 1 +
>>>>> 2 files changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> b/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> index 8d91b2b191cf..7c6c8abfcde6 100644
>>>>> --- a/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> +++ b/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> @@ -17,6 +17,10 @@
>>>>> #define OSC_CTRL 0x50
>>>>> #define OSC_CTRL_OSC_FREQ_SHIFT 28
>>>>> #define OSC_CTRL_PLL_REF_DIV_SHIFT 26
>>>>> +#define OSC_CTRL_MASK (0x3f2 | \
>>>>> + (0xf << OSC_CTRL_OSC_FREQ_SHIFT))
>>>>> +
>>>>> +static u32 osc_ctrl_ctx;
>>>>> int __init tegra_osc_clk_init(void __iomem *clk_base, struct
>>>>> tegra_clk *clks,
>>>>> unsigned long *input_freqs, unsigned int num,
>>>>> @@ -29,6 +33,7 @@ int __init tegra_osc_clk_init(void __iomem
>>>>> *clk_base, struct tegra_clk *clks,
>>>>> unsigned osc_idx;
>>>>> val = readl_relaxed(clk_base + OSC_CTRL);
>>>>> + osc_ctrl_ctx = val & OSC_CTRL_MASK;
>>>>> osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;
>>>>> if (osc_idx < num)
>>>>> @@ -96,3 +101,13 @@ void __init tegra_fixed_clk_init(struct tegra_clk
>>>>> *tegra_clks)
>>>>> *dt_clk = clk;
>>>>> }
>>>>> }
>>>>> +
>>>>> +void tegra_clk_osc_resume(void __iomem *clk_base)
>>>>> +{
>>>>> + u32 val;
>>>>> +
>>>>> + val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
>>>>> + val |= osc_ctrl_ctx;
>>>>> + writel_relaxed(val, clk_base + OSC_CTRL);
>>>> Why a full raw u32 OSC_CTRL value couldn't be simply saved and
>>>> restored?
>>> Storing and restoring only required fields to avoid accidental
>>> misconfiguration.
>>>
>>> OSC_CTRL register has other bits (PLL_REF_DIV) which are configured by
>>> BR depending on OSC_FREQ and also setting PLL_REF_DIV while PLLS are in
>>> use is not safe.
>> I'm looking at the clk-driver sources and see that none of the Tegra
>> drivers ever change the OSC_CTRL configuration, T30/114 even have
>> #defines for the OSC_CTRL that are unused.
>>
>> So, this leads to a question.. does any bootloader really ever change
>> the OSC_CTRL such that it differs after resume from suspend in
>> comparison to the value at the time of kernel's booting up?
>
> For Tegra210, bootloader programs OSC_CTRL register for drivestrength
> programming.
>
> These settings need to be restored to the same on SC7 exit as they gets
> reset during SC7 entry.
Okay, thank you for the clarification.
^ permalink raw reply
* [PATCH] drivers/amba: add reset control to primecell probe
From: Dinh Nguyen @ 2019-08-01 18:43 UTC (permalink / raw)
To: devicetree
Cc: dinguyen, linux-kernel, robh+dt, frowand.list, keescook, anton,
ccross, tony.luck, Dinh Nguyen
From: Dinh Nguyen <dinh.nguyen@intel.com>
The primecell controller on some SoCs, i.e. SoCFPGA, is held in reset by
default. Until recently, the DMA controller was brought out of reset by the
bootloader(i.e. U-Boot). But a recent change in U-Boot, the peripherals that
are not used are held in reset and are left to Linux to bring them out of
reset.
Add a mechanism for getting the reset property and de-assert the primecell
module from reset if found. This is a not a hard fail if the reset property
is not present in the device tree node, so the driver will continue to probe.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
drivers/of/platform.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7801e25e6895..d8945705313d 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -21,6 +21,7 @@
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
+#include <linux/reset.h>
const struct of_device_id of_default_bus_match_table[] = {
{ .compatible = "simple-bus", },
@@ -229,6 +230,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
struct amba_device *dev;
const void *prop;
int i, ret;
+ struct reset_control *rstc;
pr_debug("Creating amba device %pOF\n", node);
@@ -270,6 +272,18 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
goto err_free;
}
+ /*
+ * reset control of the primecell block is optional
+ * and will not fail if the reset property is not found.
+ */
+ rstc = of_reset_control_get_exclusive(node, "dma");
+ if (!IS_ERR(rstc)) {
+ reset_control_deassert(rstc);
+ reset_control_put(rstc);
+ } else {
+ pr_debug("amba: reset control not found\n");
+ }
+
ret = amba_device_add(dev, &iomem_resource);
if (ret) {
pr_err("amba_device_add() failed (%d) for %pOF\n",
--
2.20.0
^ permalink raw reply related
* Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger
From: Brendan Higgins @ 2019-08-01 18:55 UTC (permalink / raw)
To: Petr Mladek
Cc: Jeff Dike, Kevin Hilman, Logan Gunthorpe, Michael Ellerman,
Daniel Vetter, Amir Goldstein, Frank Rowand, Steven Rostedt,
Kees Cook, David Rientjes, kunit-dev, Kieran Bingham,
Peter Zijlstra, Randy Dunlap, Joel Stanley, Luis Chamberlain,
Rob Herring, Stephen Boyd, shuah, wfg, Greg KH
In-Reply-To: <20190726083148.d4gf57w2nt5k7t6n@pathway.suse.cz>
On Fri, Jul 26, 2019 at 1:31 AM Petr Mladek <pmladek@suse.com> wrote:
>
> On Thu 2019-07-25 13:21:12, Brendan Higgins wrote:
> > On Wed, Jul 24, 2019 at 12:31 AM Petr Mladek <pmladek@suse.com> wrote:
> > >
> > > On Mon 2019-07-22 16:54:10, Stephen Boyd wrote:
> > > > Quoting Brendan Higgins (2019-07-22 15:30:49)
> > > > > On Mon, Jul 22, 2019 at 1:03 PM Stephen Boyd <sboyd@kernel.org> wrote:
> > > > > >
> > > > > >
> > > > > > What's the calling context of the assertions and expectations? I still
> > > > > > don't like the fact that string stream needs to allocate buffers and
> > > > > > throw them into a list somewhere because the calling context matters
> > > > > > there.
> > > > >
> > > > > The calling context is the same as before, which is anywhere.
> > > >
> > > > Ok. That's concerning then.
> > > >
> > > > >
> > > > > > I'd prefer we just wrote directly to the console/log via printk
> > > > > > instead. That way things are simple because we use the existing
> > > > > > buffering path of printk, but maybe there's some benefit to the string
> > > > > > stream that I don't see? Right now it looks like it builds a string and
> > > > > > then dumps it to printk so I'm sort of lost what the benefit is over
> > > > > > just writing directly with printk.
> > > > >
> > > > > It's just buffering it so the whole string gets printed uninterrupted.
> > > > > If we were to print out piecemeal to printk, couldn't we have another
> > > > > call to printk come in causing it to garble the KUnit message we are
> > > > > in the middle of printing?
> > > >
> > > > Yes, printing piecemeal by calling printk many times could lead to
> > > > interleaving of messages if something else comes in such as an interrupt
> > > > printing something. Printk has some support to hold "records" but I'm
> > > > not sure how that would work here because KERN_CONT talks about only
> > > > being used early on in boot code. I haven't looked at printk in detail
> > > > though so maybe I'm all wrong and KERN_CONT just works?
> > >
> > > KERN_CONT does not guarantee that the message will get printed
> > > together. The pieces get interleaved with messages printed in
> > > parallel.
> > >
> > > Note that KERN_CONT was originally really meant to be used only during
> > > boot. It was later used more widely and ended in the best effort category.
> > >
> > > There were several attempts to make it more reliable. But it was
> > > always either too complicated or error prone or both.
> > >
> > > You need to use your own buffering if you rely want perfect output.
> > > The question is if it is really worth the complexity. Also note that
> > > any buffering reduces the chance that the messages will reach
> > > the console.
> >
> > Seems like that settles it then. Thanks!
> >
> > > BTW: There is a work in progress on a lockless printk ring buffer.
> > > It will make printk() more secure regarding deadlocks. But it might
> > > make transparent handling of continuous lines even more tricky.
> > >
> > > I guess that local buffering, before calling printk(), will be
> > > even more important then. Well, it might really force us to create
> > > an API for it.
> >
> > Cool! Can you CC me on that discussion?
>
> Adding John Oggness into CC.
>
> John, please CC Brendan Higgins on the patchsets eventually switching
> printk() into the lockless buffer. The test framework is going to
> do its own buffering to keep the related messages together.
>
> The lockless ringbuffer might make handling of related (partial)
> lines worse or better. It might justify KUnit's extra buffering
> or it might allow to get rid of it.
Thanks for CC'ing me on the printk ringbuffer thread. It looks like it
actually probably won't affect my needs for KUnit logging. The biggest
reason I need some sort of buffering system is to be able to compose
messages piece meal into a single message that will be printed out to
the user as a single message with no messages from other printk
callers printed out in the middle of mine.
The prb does look interesting; however, it appears that to get the
semantics that I need, I would have to put my entire message in a
single data block and would consequently need to know the size of my
message a priori, which is problematic. Consequently, it seems as
though I will probably need to compose my entire message using my own
buffering system.
> > > Note that stroring the messages into the printk log is basically safe in any
> > > context. It uses temporary per-CPU buffers for recursive messages and
> > > in NMI. The only problem is panic() when some CPU gets stuck with the
> > > lock taken. This will get solved by the lockless ringbuffer. Also
> > > the temporary buffers will not be necessary any longer.
> >
> > Sure, I think Stephen's concern is all the supporting code that is
> > involved. Not printk specifically. It just means a lot more of KUnit
> > has to be IRQ safe.
>
> I see.
>
> BTW: I wonder if KUnit could reuse the existing seq_buf implementation
> for buffering messages.
>
> I am sorry if it has already been proposed and rejected for some
> reason. I might have missed it. Feel free to just point me to
> same older mail.
Yeah, we discussed it briefly here:
https://lkml.org/lkml/2019/5/17/497
Looks like I forgot to include my reasoning in the commit text, sorry
about that.
> > > Much bigger problems are with consoles. There are many of them. It
> > > means a lot of code and more locks involved, including scheduler
> > > locks. Note that console lock is a semaphore.
> >
> > That shouldn't affect us though, right? As long as we continue to use
> > the printk interface?
>
> I guess that it should not affect KUnit.
>
> The only problem might be if the testing framework calls printk()
> inside scheduler or console code. And only when the tested code
> uses the same locks that will be used by the called printk().
Yeah, well printk will not be our only problem in those instances.
> To be honest I do not fully understand KUnit design. I am not
> completely sure how the tested code is isolated from the running
> system. Namely, I do not know if the tested code shares
> the same locks with the system running the test.
No worries, I don't expect printk to be the hang up in those cases. It
sounds like KUnit has a long way to evolve before printk is going to
be a limitation.
Thanks!
^ permalink raw reply
* Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger
From: Brendan Higgins @ 2019-08-01 18:59 UTC (permalink / raw)
To: Stephen Boyd
Cc: devicetree, Peter Zijlstra, Amir Goldstein, dri-devel,
Sasha Levin, Masahiro Yamada, Michael Ellerman,
open list:KERNEL SELFTEST FRAMEWORK, shuah, Rob Herring,
linux-nvdimm, Timothy Bird, Frank Rowand, open list:DOCUMENTATION,
Knut Omang, Kieran Bingham, wfg-VuQAYsv1563Yd54FQh9/CA,
Joel Stanley, David Rientjes, Kevin Hilman, Dan Carpenter,
Petr Mladek
In-Reply-To: <CAFd5g46iAhDZ5C_chi7oYLVOkwcoj6+0nw+kPWuXhqWwWKd9jA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, Aug 1, 2019 at 11:55 AM Brendan Higgins
<brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>
> On Fri, Jul 26, 2019 at 1:31 AM Petr Mladek <pmladek-IBi9RG/b67k@public.gmane.org> wrote:
> >
> > On Thu 2019-07-25 13:21:12, Brendan Higgins wrote:
> > > On Wed, Jul 24, 2019 at 12:31 AM Petr Mladek <pmladek-IBi9RG/b67k@public.gmane.org> wrote:
> > > >
> > > > On Mon 2019-07-22 16:54:10, Stephen Boyd wrote:
> > > > > Quoting Brendan Higgins (2019-07-22 15:30:49)
> > > > > > On Mon, Jul 22, 2019 at 1:03 PM Stephen Boyd <sboyd-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > > > > > >
> > > > > > >
> > > > > > > What's the calling context of the assertions and expectations? I still
> > > > > > > don't like the fact that string stream needs to allocate buffers and
> > > > > > > throw them into a list somewhere because the calling context matters
> > > > > > > there.
> > > > > >
> > > > > > The calling context is the same as before, which is anywhere.
> > > > >
> > > > > Ok. That's concerning then.
> > > > >
> > > > > >
> > > > > > > I'd prefer we just wrote directly to the console/log via printk
> > > > > > > instead. That way things are simple because we use the existing
> > > > > > > buffering path of printk, but maybe there's some benefit to the string
> > > > > > > stream that I don't see? Right now it looks like it builds a string and
> > > > > > > then dumps it to printk so I'm sort of lost what the benefit is over
> > > > > > > just writing directly with printk.
> > > > > >
> > > > > > It's just buffering it so the whole string gets printed uninterrupted.
> > > > > > If we were to print out piecemeal to printk, couldn't we have another
> > > > > > call to printk come in causing it to garble the KUnit message we are
> > > > > > in the middle of printing?
> > > > >
> > > > > Yes, printing piecemeal by calling printk many times could lead to
> > > > > interleaving of messages if something else comes in such as an interrupt
> > > > > printing something. Printk has some support to hold "records" but I'm
> > > > > not sure how that would work here because KERN_CONT talks about only
> > > > > being used early on in boot code. I haven't looked at printk in detail
> > > > > though so maybe I'm all wrong and KERN_CONT just works?
> > > >
> > > > KERN_CONT does not guarantee that the message will get printed
> > > > together. The pieces get interleaved with messages printed in
> > > > parallel.
> > > >
> > > > Note that KERN_CONT was originally really meant to be used only during
> > > > boot. It was later used more widely and ended in the best effort category.
> > > >
> > > > There were several attempts to make it more reliable. But it was
> > > > always either too complicated or error prone or both.
> > > >
> > > > You need to use your own buffering if you rely want perfect output.
> > > > The question is if it is really worth the complexity. Also note that
> > > > any buffering reduces the chance that the messages will reach
> > > > the console.
> > >
> > > Seems like that settles it then. Thanks!
> > >
> > > > BTW: There is a work in progress on a lockless printk ring buffer.
> > > > It will make printk() more secure regarding deadlocks. But it might
> > > > make transparent handling of continuous lines even more tricky.
> > > >
> > > > I guess that local buffering, before calling printk(), will be
> > > > even more important then. Well, it might really force us to create
> > > > an API for it.
> > >
> > > Cool! Can you CC me on that discussion?
> >
> > Adding John Oggness into CC.
> >
> > John, please CC Brendan Higgins on the patchsets eventually switching
> > printk() into the lockless buffer. The test framework is going to
> > do its own buffering to keep the related messages together.
> >
> > The lockless ringbuffer might make handling of related (partial)
> > lines worse or better. It might justify KUnit's extra buffering
> > or it might allow to get rid of it.
>
> Thanks for CC'ing me on the printk ringbuffer thread. It looks like it
> actually probably won't affect my needs for KUnit logging. The biggest
> reason I need some sort of buffering system is to be able to compose
> messages piece meal into a single message that will be printed out to
> the user as a single message with no messages from other printk
> callers printed out in the middle of mine.
>
> The prb does look interesting; however, it appears that to get the
> semantics that I need, I would have to put my entire message in a
> single data block and would consequently need to know the size of my
> message a priori, which is problematic. Consequently, it seems as
> though I will probably need to compose my entire message using my own
> buffering system.
>
> > > > Note that stroring the messages into the printk log is basically safe in any
> > > > context. It uses temporary per-CPU buffers for recursive messages and
> > > > in NMI. The only problem is panic() when some CPU gets stuck with the
> > > > lock taken. This will get solved by the lockless ringbuffer. Also
> > > > the temporary buffers will not be necessary any longer.
> > >
> > > Sure, I think Stephen's concern is all the supporting code that is
> > > involved. Not printk specifically. It just means a lot more of KUnit
> > > has to be IRQ safe.
> >
> > I see.
> >
> > BTW: I wonder if KUnit could reuse the existing seq_buf implementation
> > for buffering messages.
> >
> > I am sorry if it has already been proposed and rejected for some
> > reason. I might have missed it. Feel free to just point me to
> > same older mail.
>
> Yeah, we discussed it briefly here:
>
> https://lkml.org/lkml/2019/5/17/497
>
> Looks like I forgot to include my reasoning in the commit text, sorry
> about that.
>
> > > > Much bigger problems are with consoles. There are many of them. It
> > > > means a lot of code and more locks involved, including scheduler
> > > > locks. Note that console lock is a semaphore.
> > >
> > > That shouldn't affect us though, right? As long as we continue to use
> > > the printk interface?
> >
> > I guess that it should not affect KUnit.
> >
> > The only problem might be if the testing framework calls printk()
> > inside scheduler or console code. And only when the tested code
> > uses the same locks that will be used by the called printk().
>
> Yeah, well printk will not be our only problem in those instances.
>
> > To be honest I do not fully understand KUnit design. I am not
> > completely sure how the tested code is isolated from the running
> > system. Namely, I do not know if the tested code shares
> > the same locks with the system running the test.
>
> No worries, I don't expect printk to be the hang up in those cases. It
> sounds like KUnit has a long way to evolve before printk is going to
> be a limitation.
So Stephen, what do you think?
Do you want me to go forward with the new kunit_assert API wrapping
the string_stream as I have it now? Would you prefer to punt this to a
later patch? Or would you prefer something else?
Cheers
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Dmitry Osipenko @ 2019-08-01 19:00 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <550de191-f982-4544-6fbc-bf16dfeae2c6@nvidia.com>
01.08.2019 20:58, Sowjanya Komatineni пишет:
>
> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>
>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>> This patch implements save and restore context for peripheral fixed
>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock ops, and
>>>>> peripheral clock ops.
>>>>>
>>>>> During system suspend, core power goes off and looses the settings
>>>>> of the Tegra CAR controller registers.
>>>>>
>>>>> So during suspend entry clock and reset state of peripherals is saved
>>>>> and on resume they are restored to have clocks back to same rate and
>>>>> state as before suspend.
>>>>>
>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>> ++++++++++++++++++++++++++++++++
>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>> +++++++++++++++++++++++++++++++++
>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>> ++++++++++++++++++++++++++++++++++++
>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>> +++++++++++++++++++++++++++
>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>> 5 files changed, 138 insertions(+)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>> index c088e7a280df..21b24530fa00 100644
>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>> @@ -60,11 +60,44 @@ tegra_clk_periph_fixed_recalc_rate(struct
>>>>> clk_hw *hw,
>>>>> return (unsigned long)rate;
>>>>> }
>>>>> +static int tegra_clk_periph_fixed_save_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>> to_tegra_clk_periph_fixed(hw);
>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>> +
>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>> fixed->regs->enb_reg) &
>>>>> + mask;
>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>> fixed->regs->rst_reg) &
>>>>> + mask;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static void tegra_clk_periph_fixed_restore_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>> to_tegra_clk_periph_fixed(hw);
>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>> +
>>>>> + if (fixed->enb_ctx)
>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->enb_set_reg);
>>>>> + else
>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->enb_clr_reg);
>>>>> +
>>>>> + udelay(2);
>>>>> +
>>>>> + if (!fixed->rst_ctx) {
>>>>> + udelay(5); /* reset propogation delay */
>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->rst_reg);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> static const struct clk_ops tegra_clk_periph_fixed_ops = {
>>>>> .is_enabled = tegra_clk_periph_fixed_is_enabled,
>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>> .recalc_rate = tegra_clk_periph_fixed_recalc_rate,
>>>>> + .save_context = tegra_clk_periph_fixed_save_context,
>>>>> + .restore_context = tegra_clk_periph_fixed_restore_context,
>>>>> };
>>>>> struct clk *tegra_clk_register_periph_fixed(const char *name,
>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>> @@ -25,6 +25,8 @@ static DEFINE_SPINLOCK(periph_ref_lock);
>>>>> #define read_rst(gate) \
>>>>> readl_relaxed(gate->clk_base + (gate->regs->rst_reg))
>>>>> +#define write_rst_set(val, gate) \
>>>>> + writel_relaxed(val, gate->clk_base + (gate->regs->rst_set_reg))
>>>>> #define write_rst_clr(val, gate) \
>>>>> writel_relaxed(val, gate->clk_base + (gate->regs->rst_clr_reg))
>>>>> @@ -110,10 +112,42 @@ static void clk_periph_disable(struct
>>>>> clk_hw *hw)
>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>> }
>>>>> +static int clk_periph_gate_save_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>> +
>>>>> + gate->clk_state_ctx = read_enb(gate) & periph_clk_to_bit(gate);
>>>>> + gate->rst_state_ctx = read_rst(gate) & periph_clk_to_bit(gate);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static void clk_periph_gate_restore_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>> +
>>>>> + if (gate->clk_state_ctx)
>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>> + else
>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>> +
>>>>> + udelay(5);
>>>>> +
>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>> + if (gate->rst_state_ctx)
>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>> + else
>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> const struct clk_ops tegra_clk_periph_gate_ops = {
>>>>> .is_enabled = clk_periph_is_enabled,
>>>>> .enable = clk_periph_enable,
>>>>> .disable = clk_periph_disable,
>>>>> + .save_context = clk_periph_gate_save_context,
>>>>> + .restore_context = clk_periph_gate_restore_context,
>>>>> };
>>>>> struct clk *tegra_clk_register_periph_gate(const char *name,
>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>> index 58437da25156..06fb62955768 100644
>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct clk_hw *hw)
>>>>> gate_ops->disable(gate_hw);
>>>>> }
>>>>> +static int clk_periph_save_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>> +
>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>> + gate_ops->save_context(gate_hw);
>>>>> +
>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static void clk_periph_restore_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>> +
>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>> +
>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>> + div_ops->restore_context(div_hw);
>>>> Could you please point to where the divider's save_context() happens?
>>>> Because I can't see it.
>>> Ah, I now see that there is no need to save the dividers context because
>>> clk itself has enough info that is needed for the context's restoring
>>> (like I pointed in the review to v6).
>>>
>>> Looks like you could also implement a new clk_hw_get_parent_index()
>>> generic helper to get the index instead of storing it manually.
>>
>> clk_periph_get_parent basically invokes existing clk_mux_ops
>> get_parent() which is then saved in tegra_clk_periph.
>>
>> All existing drivers are using directly get_parent() from clk_mux
>> which actually gets index from the register read.
>>
>> To have this more generic w.r.t save/restore context point of view,
>> probably instead of implementing new get_parent_index helper, I think
>> its better to implement save_context and restore_context to
>> clk_mux_ops along with creating parent_index field into clk_mux to
>> cache index during set_parent.
>>
>> So we just need to invoke mux_ops save_context and restore_context.
>>
> I hope its ok to add save/restore context to clk_mux_ops to be more
> generic w.r.t save/restore context rather than get_parent_index API.
> Please confirm if you agree.
Sounds like a good idea. I see that there is a 'restoring' helper for
the generic clk_gate, seems something similar could be done for the
clk_mux. And looks like anyway you'll need to associate the parent clock
with the hw index in order to restore the muxing.
^ permalink raw reply
* [PATCH v4 0/4] net: phy: realtek: Enable configuration of RTL8211E LEDs
From: Matthias Kaehlcke @ 2019-08-01 19:07 UTC (permalink / raw)
To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
Florian Fainelli, Heiner Kallweit
Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
Matthias Kaehlcke
The Realtek RTL8211E allows customization of the PHY LED behavior,
like which LEDs are on for certain link speeds and which LEDs blink
when there is traffic. By default EEE LED mode is enabled, in which
a blinking LED is on for 400ms and off for 2s. This series adds a
generic device tree binding for configuring PHY LEDs and adds LED
configuration support for the RTL8211E PHY.
Certain registers of the RTL8211E can only be accessed through
a vendor specific extended page mechanism. Extended pages need
to be accessed for the LED configuration. This series adds helpers
to facilitate accessing extended pages.
Matthias Kaehlcke (4):
dt-bindings: net: phy: Add subnode for LED configuration
net: phy: Add function to retrieve LED configuration from the DT
net: phy: realtek: Add helpers for accessing RTL8211E extension pages
net: phy: realtek: configure RTL8211E LEDs
.../devicetree/bindings/net/ethernet-phy.yaml | 47 +++++
drivers/net/phy/phy_device.c | 50 ++++++
drivers/net/phy/realtek.c | 169 ++++++++++++++++--
include/linux/phy.h | 15 ++
4 files changed, 266 insertions(+), 15 deletions(-)
--
2.22.0.770.g0f2c4a37fd-goog
^ 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