* [PATCH] ARM64: dts: meson-gxbb: add SCPI pre-1.0 compatible
From: Kevin Hilman @ 2016-11-28 18:25 UTC (permalink / raw)
To: linux-arm-kernel
The SCPI driver has an updated compatible to indicate the pre-released
(pre v1.0) status of the driver. Since Amlogic used a pre-1.0
version, add that compatible as well.
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index ac5ad3bf1bd6..51edd5b5c460 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -51,7 +51,7 @@
compatible = "amlogic,meson-gxbb";
scpi {
- compatible = "amlogic,meson-gxbb-scpi";
+ compatible = "amlogic,meson-gxbb-scpi", "arm,scpi-pre-1.0";
mboxes = <&mailbox 1 &mailbox 2>;
shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
--
2.9.3
^ permalink raw reply related
* [PATCH V3 0/8] IOMMU probe deferral support
From: Lorenzo Pieralisi @ 2016-11-28 18:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <000001d2499e$df7f2d80$9e7d8880$@codeaurora.org>
On Mon, Nov 28, 2016 at 11:12:08PM +0530, Sricharan wrote:
[...]
> >Cool. We're rather hoping that the ACPI stuff is good to go for 4.10
> >now, so it's probably worth pulling the rest of that in (beyond the one
> >patch I picked) to make sure the of_dma_configure/acpi_dma_configure
> >paths don't inadvertently diverge.
> >
>
> I rebased and was testing your branch with Lorenzo's series. One thing
> i am still trying to get right is the acpi_dma_configure call. With your
> series dma_configure calls pci_dma/of_dma configure, so i am just adding
> acpi_dma_configure call there for non-pci ACPI devices as well. I see that
> acpi_dma_configure right now is called from acpi_bind_one and
> iort_add_smmu_platform_device, both go through the really_probe function
> path, so moving acpi_dma_configure from the above the two functions
> to dma_configure. I remember we discussed this on another thread, so
> hopefully it is correct. I do not have an platform to test the ACPI though.
> I will take some testing help on V4 for this.
I am happy to test it for you please just send me a pointer at your v4
code.
Thank you !
Lorenzo
^ permalink raw reply
* [PATCH v7 2/8] drm/sun8i: Add DT bindings documentation of Allwinner DE2
From: Jean-Francois Moine @ 2016-11-28 18:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1480414715.git.moinejf@free.fr>
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
.../bindings/display/sunxi/sun8i-de2.txt | 121 +++++++++++++++++++++
1 file changed, 121 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
diff --git a/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
new file mode 100644
index 0000000..edf38b8
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
@@ -0,0 +1,121 @@
+Allwinner sun8i Display Engine 2 subsystem
+==========================================
+
+The Allwinner DE2 subsystem contains a display controller (DE2),
+one or two LCD controllers (Timing CONtrollers) and their external
+interfaces (encoders/connectors).
+
+ +-----------+
+ | DE2 |
+ | |
+ | +-------+ |
+ plane --->| | | +------+
+ | | mixer |---->| TCON |---> encoder ---> display
+ plane --->| | | +------+ connector device
+ | +-------+ |
+ | |
+ | +-------+ |
+ plane --->| | | +------+
+ | | mixer |---->| TCON |---> encoder ---> display
+ plane --->| | | +------+ connector device
+ | +-------+ |
+ +-----------+
+
+The DE2 contains a processor which mixes the input planes and creates
+the images which are sent to the TCONs.
+From the software point of vue, there are 2 independent real-time
+mixers, each one being statically associated to one TCON.
+
+The TCONs adjust the image format to the one of the display device.
+
+Display controller (DE2)
+========================
+
+Required properties:
+
+- compatible: value should be one of the following
+ "allwinner,sun8i-a83t-display-engine"
+ "allwinner,sun8i-h3-display-engine"
+
+- reg: base address and size of the I/O memory
+
+- clocks: must include clock specifiers corresponding to entries in the
+ clock-names property.
+
+- clock-names: must contain
+ "bus": bus gate
+ "clock": clock
+
+- resets: phandle to the reset of the device
+
+- ports: must contain a list of 2 phandles, indexed by mixer number,
+ and pointing to display interface ports of TCONs
+
+LCD controller (TCON)
+=====================
+
+Required properties:
+
+- compatible: should be
+ "allwinner,sun8i-a83t-tcon"
+
+- reg: base address and size of the I/O memory
+
+- clocks: must include clock specifiers corresponding to entries in the
+ clock-names property.
+
+- clock-names: must contain
+ "bus": bus gate
+ "clock": pixel clock
+
+- resets: phandle to the reset of the device
+
+- interrupts: interrupt number for the TCON
+
+- port: port node with endpoint definitions as defined in
+ Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+
+ de: de-controller at 01000000 {
+ compatible = "allwinner,sun8i-h3-display-engine";
+ reg = <0x01000000 0x400000>;
+ clocks = <&ccu CLK_BUS_DE>, <&ccu CLK_DE>;
+ clock-names = "bus", "clock";
+ resets = <&ccu RST_BUS_DE>;
+ ports = <&tcon0_p>, <&tcon1_p>;
+ };
+
+ tcon0: lcd-controller at 01c0c000 {
+ compatible = "allwinner,sun8i-a83t-tcon";
+ reg = <0x01c0c000 0x400>;
+ clocks = <&ccu CLK_BUS_TCON0>, <&ccu CLK_TCON0>;
+ clock-names = "bus", "clock";
+ resets = <&ccu RST_BUS_TCON0>;
+ interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ tcon0_p: port {
+ tcon0_hdmi: endpoint {
+ remote-endpoint = <&hdmi_tcon0>;
+ };
+ };
+ };
+
+ /* not used */
+ tcon1: lcd-controller at 01c0d000 {
+ compatible = "allwinner,sun8i-h3-tcon";
+ reg = <0x01c0d000 0x400>;
+ clocks = <&ccu CLK_BUS_TCON1>,
+ <&ccu CLK_TCON0>; /* no clock */
+ clock-names = "bus", "clock";
+ interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ tcon1_p: port {
+ endpoint {
+ /* empty */
+ };
+ };
+ };
--
2.10.2
^ permalink raw reply related
* Adding a .platform_init callback to sdhci_arasan_ops
From: Doug Anderson @ 2016-11-28 18:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e32d2a01-ba3c-6127-51ba-a1fd4176e32c@laposte.net>
Hi,
On Mon, Nov 28, 2016 at 5:28 AM, Sebastian Frias <sf84@laposte.net> wrote:
> +static void sdhci_tango4_platform_init(struct sdhci_host *host)
> +{
> + printk("%s\n", __func__);
> +
> + /*
> + pad_mode[2:0]=0 must be 0
> + sel_sdio[3]=1 must be 1 for SDIO
> + inv_sdwp_pol[4]=0 if set inverts the SD write protect polarity
> + inv_sdcd_pol[5]=0 if set inverts the SD card present polarity
> + */
> + sdhci_writel(host, 0x00000008, 0x100 + 0x0);
If I were doing this, I'd probably actually add these fields to the
"sdhci_arasan_soc_ctl_map", then add a 3rd option to
sdhci_arasan_syscon_write(). Right now it has 2 modes: "hiword
update" and "non-hiword update". You could add a 3rd indicating that
you set config registers by just writing at some offset of the main
SDHCI registers space.
So you'd add 4 fields:
.tango_pad_mode = { .reg = 0x0000, .width = 3, .shift = 0 },
.sel_sdio = { .reg = 0x0000, .width = 1, .shift = 3},
.inv_sdwp_pol = { .reg = 0x0000, .width = 1, .shift = 4},
.inv_sdcd_pol = { .reg = 0x0000, .width = 1, .shift = 5},
In your platform-specific init you're proposing you could set
tango_pad_mode to 0. That seems tango-specific.
You'd want to hook into "set_ios" for setting sel_sdio or not. That's
important if anyone ever wants to plug in an external SDIO card to
your slot. This one good argument for putting this in
sdhci_arasan_soc_ctl_map, since you wouldn't want to do a
compatibility matching every time set_ios is called.
I'd have to look more into the whole SD/WP polarity issue. There are
already so many levels of inversion for these signals in Linux that
it's confusing. It seems like you might just want to hardcode them to
"0" and let users use all the existing ways to invert things... You
could either put that hardcoding into your platform init code or (if
you're using sdhci_arasan_soc_ctl_map) put it in the main init code so
that if anyone else needs to init similar signals then they can take
advantage of it.
--
One random side note is that as currently documented you need to
specify a "shift" of -1 for any sdhci_arasan_soc_ctl_map fields you
aren't using. That seems stupid--not sure why I did that. It seems
better to clue off "width = 0" so that you could just freely not init
any fields you don't need.
-Doug
^ permalink raw reply
* [PATCH] fpga zynq: Check the bitstream for validity
From: Jason Gunthorpe @ 2016-11-28 18:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <609bc971-bb6d-8cd2-8e64-23c0082a6216@topic.nl>
I seem to have missed being CC on this follow up from Mike and wanted
to respond:
> What you're creating here will require active maintenance. There
> are already 7007 and 7012 devices which aren't in your list so the
> driver will refuse to load them until someone fills in the IDs.
I don't know what list you are refering to, are you sure you are
responding to the right patch?
The patch searches for the sync word, it has nothing to do with IDs,
and does not attempt to parse any of the proprietary headers. Based on
the Xilinx documentation this will work on 7 Series, US and US+ at a
minimum. Certainly on all Zynq hardware.
> The bitstream format is actually proprietary and undocumented.
> Any "checks" in code are likely based on guesswork and reverse
> engineering.
No, this part is fully documented in UG470.
> We also use partial reprogramming a lot. Did you test
> that? On all devices?
You should read the patch, it only does the check on a full bitstream.
Jason
^ permalink raw reply
* [PATCH net-next v4 0/4] Fix OdroidC2 Gigabit Tx link issue
From: Florian Fainelli @ 2016-11-28 17:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480348229-25672-1-git-send-email-jbrunet@baylibre.com>
On 11/28/2016 07:50 AM, Jerome Brunet wrote:
> This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
> The platform seems to enter LPI on the Rx path too often while performing
> relatively high TX transfer. This eventually break the link (both Tx and
> Rx), and require to bring the interface down and up again to get the Rx
> path working again.
>
> The root cause of this issue is not fully understood yet but disabling EEE
> advertisement on the PHY prevent this feature to be negotiated.
> With this change, the link is stable and reliable, with the expected
> throughput performance.
>
> The patchset adds options in the generic phy driver to disable EEE
> advertisement, through device tree. The way it is done is very similar
> to the handling of the max-speed property.
>
> Patch 4 is provided here for testing purpose only. Please don't merge
> patch 4, this change will go through the amlogic's tree.
Sorry, but I really don't like the route this is going, and I should
have made myself clearer before on that, I really think utilizing a PHY
fixup is more appropriate here than an extremely generic DT property.
The fixup code can be in the affected PHY driver, or it can be somewhere
else, your call. There is no shortage of option on how to implement it,
and this would be something easy to enable/disable for known good
configurations (ala PCI/USB fixups).
If we start supporting generic "enable", "disable" type of properties
with values that map directly to register definitions of the HW, we
leave too much room for these properties to be utilized to implement a
specific policy, and this is not acceptable.
--
Florian
^ permalink raw reply
* Adding a .platform_init callback to sdhci_arasan_ops
From: Doug Anderson @ 2016-11-28 17:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <485a747c-47e3-bc0e-093a-4ec9ab719987@laposte.net>
Hi,
On Mon, Nov 28, 2016 at 6:39 AM, Sebastian Frias <sf84@laposte.net> wrote:
>> I will try to send another patch with what a different approach
>>
>
> Here's a different approach (I just tested that it built, because I don't have the
> rk3399 platform):
>
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 410a55b..5be6e67 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -382,22 +382,6 @@ static int sdhci_arasan_resume(struct device *dev)
> static SIMPLE_DEV_PM_OPS(sdhci_arasan_dev_pm_ops, sdhci_arasan_suspend,
> sdhci_arasan_resume);
>
> -static const struct of_device_id sdhci_arasan_of_match[] = {
> - /* SoC-specific compatible strings w/ soc_ctl_map */
> - {
> - .compatible = "rockchip,rk3399-sdhci-5.1",
> - .data = &rk3399_soc_ctl_map,
> - },
> -
> - /* Generic compatible below here */
> - { .compatible = "arasan,sdhci-8.9a" },
> - { .compatible = "arasan,sdhci-5.1" },
> - { .compatible = "arasan,sdhci-4.9a" },
> -
> - { /* sentinel */ }
> -};
> -MODULE_DEVICE_TABLE(of, sdhci_arasan_of_match);
> -
> /**
> * sdhci_arasan_sdcardclk_recalc_rate - Return the card clock rate
> *
> @@ -578,28 +562,18 @@ static void sdhci_arasan_unregister_sdclk(struct device *dev)
> of_clk_del_provider(dev->of_node);
> }
>
> -static int sdhci_arasan_probe(struct platform_device *pdev)
> +static int sdhci_rockchip_platform_init(struct sdhci_host *host,
> + struct platform_device *pdev)
> {
> int ret;
> - const struct of_device_id *match;
> struct device_node *node;
> - struct clk *clk_xin;
> - struct sdhci_host *host;
> struct sdhci_pltfm_host *pltfm_host;
> struct sdhci_arasan_data *sdhci_arasan;
> - struct device_node *np = pdev->dev.of_node;
> -
> - host = sdhci_pltfm_init(pdev, &sdhci_arasan_pdata,
> - sizeof(*sdhci_arasan));
> - if (IS_ERR(host))
> - return PTR_ERR(host);
>
> pltfm_host = sdhci_priv(host);
> sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
> - sdhci_arasan->host = host;
>
> - match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node);
> - sdhci_arasan->soc_ctl_map = match->data;
> + sdhci_arasan->soc_ctl_map = &rk3399_soc_ctl_map;
>
> node = of_parse_phandle(pdev->dev.of_node, "arasan,soc-ctl-syscon", 0);
> if (node) {
> @@ -611,10 +585,107 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
> if (ret != -EPROBE_DEFER)
> dev_err(&pdev->dev, "Can't get syscon: %d\n",
> ret);
> - goto err_pltfm_free;
> + return -1;
> }
> }
>
> + if (of_property_read_bool(pdev->dev.of_node, "xlnx,fails-without-test-cd"))
> + sdhci_arasan->quirks |= SDHCI_ARASAN_QUIRK_FORCE_CDTEST;
> +
> + return 0;
> +}
> +
> +static int sdhci_rockchip_clock_init(struct sdhci_host *host,
> + struct platform_device *pdev)
> +{
> + struct sdhci_pltfm_host *pltfm_host;
> + struct sdhci_arasan_data *sdhci_arasan;
> +
> + pltfm_host = sdhci_priv(host);
> + sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
> +
> + if (of_device_is_compatible(pdev->dev.of_node,
> + "rockchip,rk3399-sdhci-5.1"))
> + sdhci_arasan_update_clockmultiplier(host, 0x0);
This _does_ belong in a Rockchip-specific init function, for now.
Other platforms might want different values for
corecfg_clockmultiplier, I think.
If it becomes common to need to set this, it wouldn't be hard to make
it generic by putting it in the data matched by the device tree, then
generically call sdhci_arasan_update_clockmultiplier() in cases where
it is needed. sdhci_arasan_update_clockmultiplier() itself should be
generic enough to handle it.
> +
> + sdhci_arasan_update_baseclkfreq(host);
If you make soc_ctl_map always part of "struct sdhci_arasan_cs_ops"
then other platforms will be able to use it.
As argued in my original patch the field "corecfg_baseclkfreq" is
documented in the generic Arasan document
<https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf>
and thus is unlikely to be Rockchip specific. It is entirely possible
that not everyone who integrates this IP block will need this register
set, but in that case they can set an offset as "-1" and they'll be
fine.
Said another way: the concept of whether or not to set "baseclkfreq"
doesn't need to be tired to whether or not we're on Rockchip.
> +
> + return sdhci_arasan_register_sdclk(sdhci_arasan, pltfm_host->clk, &pdev->dev);
This is documented in "bindings/mmc/arasan,sdhci.txt" to be available
to all people using this driver, not just Rockchip. You should do it
always. If you don't specify "#clock-cells" in the device tree it
will be a no-op anyway.
> +}
> +
> +static int sdhci_tango_platform_init(struct sdhci_host *host,
> + struct platform_device *pdev)
> +{
> + return 0;
I'll comment in your other patch where you actually had an
implementation for this.
> +}
> +
> +/* Chip-specific ops */
> +struct sdhci_arasan_cs_ops {
> + int (*platform_init)(struct sdhci_host *host,
> + struct platform_device *pdev);
> + int (*clock_init)(struct sdhci_host *host,
> + struct platform_device *pdev);
> +};
I really think it's a good idea to add the "soc_ctl_map" into this structure.
If nothing else when the next Rockchip SoC comes out that uses this
then we won't have to do a second level of matching for Rockchip SoCs.
I'm also a firm believer that others will need "soc_ctl_map" at some
point in time, but obviously I can't predict the future.
> +
> +static const struct sdhci_arasan_cs_ops sdhci_rockchip_cs_ops = {
> + .platform_init = sdhci_rockchip_platform_init,
> + .clock_init = sdhci_rockchip_clock_init,
> +};
> +
> +static const struct sdhci_arasan_cs_ops sdhci_tango_cs_ops = {
> + .platform_init = sdhci_tango_platform_init,
> +};
> +
> +static const struct of_device_id sdhci_arasan_of_match[] = {
> + /* SoC-specific compatible strings */
> + {
> + .compatible = "rockchip,rk3399-sdhci-5.1",
> + .data = &sdhci_rockchip_cs_ops,
> + },
> + {
> + .compatible = "sigma,sdio-v1",
> + .data = &sdhci_tango_cs_ops,
> + },
> +
> + /* Generic compatible below here */
> + { .compatible = "arasan,sdhci-8.9a" },
> + { .compatible = "arasan,sdhci-5.1" },
> + { .compatible = "arasan,sdhci-4.9a" },
> +
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_arasan_of_match);
> +
> +static int sdhci_arasan_probe(struct platform_device *pdev)
> +{
> + int ret;
> + const struct of_device_id *match;
> + struct clk *clk_xin;
> + struct sdhci_host *host;
> + struct sdhci_pltfm_host *pltfm_host;
> + struct sdhci_arasan_data *sdhci_arasan;
> + const struct sdhci_arasan_cs_ops *cs_ops;
> +
> + host = sdhci_pltfm_init(pdev, &sdhci_arasan_pdata,
> + sizeof(*sdhci_arasan));
> + if (IS_ERR(host))
> + return PTR_ERR(host);
> +
> + pltfm_host = sdhci_priv(host);
> + sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
> + sdhci_arasan->host = host;
> +
> + match = of_match_device(sdhci_arasan_of_match, &pdev->dev);
> + if (match)
> + cs_ops = match->data;
> +
> + /* SoC-specific platform init */
> + if (cs_ops && cs_ops->platform_init) {
> + ret = cs_ops->platform_init(host, pdev);
> + if (ret)
> + goto err_pltfm_free;
> + }
> +
> sdhci_arasan->clk_ahb = devm_clk_get(&pdev->dev, "clk_ahb");
> if (IS_ERR(sdhci_arasan->clk_ahb)) {
> dev_err(&pdev->dev, "clk_ahb clock not found.\n");
> @@ -642,21 +713,14 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
> }
>
> sdhci_get_of_property(pdev);
> -
> - if (of_property_read_bool(np, "xlnx,fails-without-test-cd"))
> - sdhci_arasan->quirks |= SDHCI_ARASAN_QUIRK_FORCE_CDTEST;
> -
> pltfm_host->clk = clk_xin;
>
> - if (of_device_is_compatible(pdev->dev.of_node,
> - "rockchip,rk3399-sdhci-5.1"))
> - sdhci_arasan_update_clockmultiplier(host, 0x0);
> -
> - sdhci_arasan_update_baseclkfreq(host);
> -
> - ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, &pdev->dev);
> - if (ret)
> - goto clk_disable_all;
> + /* SoC-specific clock init */
> + if (cs_ops && cs_ops->clock_init) {
> + ret = cs_ops->clock_init(host, pdev);
> + if (ret)
> + goto clk_disable_all;
> + }
>
> ret = mmc_of_parse(host->mmc);
> if (ret) {
>
>
> If you are ok with it I can clean it up to submit it properly.
^ permalink raw reply
* [PATCH] KVM: arm/arm64: Access CNTHCTL_EL2 bit fields correctly
From: Marc Zyngier @ 2016-11-28 17:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480351570-11648-1-git-send-email-jintack@cs.columbia.edu>
Hi Jintack,
On 28/11/16 16:46, Jintack Lim wrote:
> Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
> EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
> are 11th and 10th bits respectively when E2H is set. Current code is
> unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set, which
> may allow guest OS to access physical timer. So, fix it.
>
> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> ---
> arch/arm/include/asm/kvm_timer.h | 33 +++++++++++++++++++
> arch/arm64/include/asm/kvm_timer.h | 62 ++++++++++++++++++++++++++++++++++++
> include/clocksource/arm_arch_timer.h | 6 ++--
> virt/kvm/arm/hyp/timer-sr.c | 8 ++---
> 4 files changed, 103 insertions(+), 6 deletions(-)
> create mode 100644 arch/arm/include/asm/kvm_timer.h
> create mode 100644 arch/arm64/include/asm/kvm_timer.h
>
> diff --git a/arch/arm/include/asm/kvm_timer.h b/arch/arm/include/asm/kvm_timer.h
> new file mode 100644
> index 0000000..d19d4b3
> --- /dev/null
> +++ b/arch/arm/include/asm/kvm_timer.h
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright (C) 2016 - Columbia University
> + * Author: Jintack Lim <jintack@cs.columbia.edu>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __ARM_KVM_TIMER_H__
> +#define __ARM_KVM_TIMER_H__
> +
> +#include <clocksource/arm_arch_timer.h>
> +
> +static inline u32 __hyp_text get_el1pcten(void)
> +{
> + return CNTHCTL_EL1PCTEN_NVHE;
> +}
> +
> +static inline u32 __hyp_text get_el1pcen(void)
> +{
> + return CNTHCTL_EL1PCEN_NVHE;
> +}
> +
> +#endif /* __ARM_KVM_TIMER_H__ */
> diff --git a/arch/arm64/include/asm/kvm_timer.h b/arch/arm64/include/asm/kvm_timer.h
> new file mode 100644
> index 0000000..153f3da
> --- /dev/null
> +++ b/arch/arm64/include/asm/kvm_timer.h
> @@ -0,0 +1,62 @@
> +/*
> + * Copyright (C) 2016 - Columbia University
> + * Author: Jintack Lim <jintack@cs.columbia.edu>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __ARM64_KVM_TIMER_H__
> +#define __ARM64_KVM_TIMER_H__
> +
> +#include <asm/kvm_hyp.h>
> +#include <clocksource/arm_arch_timer.h>
> +
> +static inline u32 __hyp_text get_el1pcten_vhe(void)
> +{
> + return CNTHCTL_EL1PCTEN_VHE;
> +}
> +
> +static inline u32 __hyp_text get_el1pcten_nvhe(void)
> +{
> + return CNTHCTL_EL1PCTEN_NVHE;
> +}
> +
> +static hyp_alternate_select(get_el1pcten_arch,
> + get_el1pcten_nvhe, get_el1pcten_vhe,
> + ARM64_HAS_VIRT_HOST_EXTN);
This is pretty horrid ;-)
First, the inline qualifier doesn't apply here, as the whole
hyp_alternate_select hack relies on thing not being inlined (it
actually creates a function pointer).
Then, this gets potentially instantiated in any compilation unit that
will include this file. GCC is probably clever enough to eliminate it
if not used, but not without a well deserved warning.
> +
> +static inline u32 __hyp_text get_el1pten_vhe(void)
> +{
> + return CNTHCTL_EL1PTEN_VHE;
> +}
> +
> +static inline u32 __hyp_text get_el1pcen_nvhe(void)
> +{
> + return CNTHCTL_EL1PCEN_NVHE;
> +}
> +
> +static hyp_alternate_select(get_el1pcen_arch,
> + get_el1pcen_nvhe, get_el1pten_vhe,
> + ARM64_HAS_VIRT_HOST_EXTN);
> +
> +static inline u32 __hyp_text get_el1pcten(void)
> +{
> + return get_el1pcten_arch()();
> +}
> +
> +static inline u32 __hyp_text get_el1pcen(void)
> +{
> + return get_el1pcen_arch()();
> +}
> +
> +#endif /* __ARM64_KVM_TIMER_H__ */
> diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
> index caedb74..4094529 100644
> --- a/include/clocksource/arm_arch_timer.h
> +++ b/include/clocksource/arm_arch_timer.h
> @@ -23,8 +23,10 @@
> #define ARCH_TIMER_CTRL_IT_MASK (1 << 1)
> #define ARCH_TIMER_CTRL_IT_STAT (1 << 2)
>
> -#define CNTHCTL_EL1PCTEN (1 << 0)
> -#define CNTHCTL_EL1PCEN (1 << 1)
> +#define CNTHCTL_EL1PCTEN_NVHE (1 << 0)
> +#define CNTHCTL_EL1PCEN_NVHE (1 << 1)
> +#define CNTHCTL_EL1PCTEN_VHE (1 << 10)
> +#define CNTHCTL_EL1PTEN_VHE (1 << 11)
> #define CNTHCTL_EVNTEN (1 << 2)
> #define CNTHCTL_EVNTDIR (1 << 3)
> #define CNTHCTL_EVNTI (0xF << 4)
> diff --git a/virt/kvm/arm/hyp/timer-sr.c b/virt/kvm/arm/hyp/timer-sr.c
> index 798866a..f3feee0 100644
> --- a/virt/kvm/arm/hyp/timer-sr.c
> +++ b/virt/kvm/arm/hyp/timer-sr.c
> @@ -15,11 +15,11 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> -#include <clocksource/arm_arch_timer.h>
> #include <linux/compiler.h>
> #include <linux/kvm_host.h>
>
> #include <asm/kvm_hyp.h>
> +#include <asm/kvm_timer.h>
>
> /* vcpu is already in the HYP VA space */
> void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
> @@ -37,7 +37,7 @@ void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
>
> /* Allow physical timer/counter access for the host */
> val = read_sysreg(cnthctl_el2);
> - val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
> + val |= get_el1pcten() | get_el1pcen();
> write_sysreg(val, cnthctl_el2);
>
> /* Clear cntvoff for the host */
> @@ -55,8 +55,8 @@ void __hyp_text __timer_restore_state(struct kvm_vcpu *vcpu)
> * Physical counter access is allowed
> */
> val = read_sysreg(cnthctl_el2);
> - val &= ~CNTHCTL_EL1PCEN;
> - val |= CNTHCTL_EL1PCTEN;
> + val &= ~get_el1pcen();
> + val |= get_el1pcten();
> write_sysreg(val, cnthctl_el2);
>
> if (timer->enabled) {
>
Trying to solve this myself, I came up with an alternative approach,
which is ugly in its own way (#define in common code, random shifts):
diff --git a/virt/kvm/arm/hyp/timer-sr.c b/virt/kvm/arm/hyp/timer-sr.c
index 798866a..5cacfa8 100644
--- a/virt/kvm/arm/hyp/timer-sr.c
+++ b/virt/kvm/arm/hyp/timer-sr.c
@@ -21,11 +21,41 @@
#include <asm/kvm_hyp.h>
+#ifdef CONFIG_ARM64
+static unsigned int __hyp_text get_cnthclt_shift_nvhe(void)
+{
+ return 0;
+}
+
+static unsigned int __hyp_text get_cnthclt_shift_vhe(void)
+{
+ return 10;
+}
+
+static hyp_alternate_select(__get_cnthclt_shift,
+ get_cnthclt_shift_nvhe, get_cnthclt_shift_vhe,
+ ARM64_HAS_VIRT_HOST_EXTN)
+
+#define cnthclt_shift() __get_cnthclt_shift()()
+#else
+#define cnthclt_shift() (0)
+#endif
+
+static inline void __hyp_text cnthctl_rmw(u32 clr, u32 set)
+{
+ u32 val;
+ int shift = cnthclt_shift();
+
+ val = read_sysreg(cnthctl_el2);
+ val &= ~clr << shift;
+ val |= set << shift;
+ write_sysreg(val, cnthctl_el2);
+}
+
/* vcpu is already in the HYP VA space */
void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
{
struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
- u64 val;
if (timer->enabled) {
timer->cntv_ctl = read_sysreg_el0(cntv_ctl);
@@ -36,9 +66,7 @@ void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
write_sysreg_el0(0, cntv_ctl);
/* Allow physical timer/counter access for the host */
- val = read_sysreg(cnthctl_el2);
- val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
- write_sysreg(val, cnthctl_el2);
+ cnthctl_rmw(0, CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN);
/* Clear cntvoff for the host */
write_sysreg(0, cntvoff_el2);
@@ -48,16 +76,12 @@ void __hyp_text __timer_restore_state(struct kvm_vcpu *vcpu)
{
struct kvm *kvm = kern_hyp_va(vcpu->kvm);
struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
- u64 val;
/*
* Disallow physical timer access for the guest
* Physical counter access is allowed
*/
- val = read_sysreg(cnthctl_el2);
- val &= ~CNTHCTL_EL1PCEN;
- val |= CNTHCTL_EL1PCTEN;
- write_sysreg(val, cnthctl_el2);
+ cnthctl_rmw(CNTHCTL_EL1PCEN, CNTHCTL_EL1PCTEN);
if (timer->enabled) {
write_sysreg(kvm->arch.timer.cntvoff, cntvoff_el2);
We could make it nicer (read "faster") by introducing a
hyp_alternate_select construct that only returns a value instead
of calling a function. I remember writing something like that
at some point, and dropping it...
Thoughts?
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply related
* [PATCH V3 0/8] IOMMU probe deferral support
From: Sricharan @ 2016-11-28 17:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f2ad6976-0f8f-d116-bc43-37337dd9f41f@arm.com>
Hi Robin,
>> <snip..>
>>
>>>
>>>>>>>> iommu_group_get_for_dev which gets called in the add_device
>>>>>>>> callback, increases the reference count of the iommu_group,
>>>>>>>> so we do an iommu_group_put after that. iommu_group_get_for_dev
>>>>>>>> inturn calls device_group callback and in the case of arm-smmu
>>>>>>>> we call generic_device_group/pci_device_group which takes
>>>>>>>> care of increasing the group's reference. But when we return
>>>>>>>> an already existing group(when multiple devices have same group)
>>>>>>>> the reference is not incremented, resulting in issues when the
>>>>>>>> remove_device callback for the devices is invoked.
>>>>>>>> Fixing the same here.
>>>>>>>
>>>>>>> Bah, yes, this does look like my fault - after flip-flopping between
>>>>>>> about 3 different ways to keep refcounts for the S2CR entries, none of
>>>>>>> which would quite work, I ripped it all out but apparently still got
>>>>>>> things wrong, oh well. Thanks for figuring it out.
>>>>>>>
>>>>>>> On the probe-deferral angle, whilst it's useful to have uncovered this
>>>>>>> bug, I don't think we should actually be calling remove_device() from
>>>>>>> DMA teardown. I think it's preferable from a user perspective if group
>>>>>>> numbering remains stable, rather than changing depending on the order in
>>>>>>> which they unbind/rebind VFIO drivers. I'm really keen to try and get
>>>>>>> this in shape for 4.10, so I've taken the liberty of hacking up my own
>>>>>>> branch (iommu/defer) based on v3 - would you mind taking a look at the
>>>>>>> two "iommu/of:" commits to see what you think? (Ignore the PCI changes
>>>>>>> to your later patches - that was an experiment which didn't really work out)
>>>>>>
>>>>>> Ok, will take a look at this now and respond more on this.
>>>>>>
>>>>> Sorry for the delayed response on this. I was OOO for the last few days.
>>>>> So i tested this branch and it worked fine. I tested it with a pci device
>>>>> for both normal and deferred probe cases. The of/iommu patches
>>>>> are the cleanup/preparation patches and it looks fine. One thing is without
>>>>> calling the remove_device callback, the resources like (smes for exmaple)
>>>>> and the group association of the device all remain allocated. That does not
>>>>> feel correct, given that the associated device does not exist. So to
>>>>> understand that, what happens with VFIO in this case which makes the
>>>>> group renumbering/rebinding a problem ?
>>>>>
>>>>
>>>> Would it be ok if i post a V4 based on your branch above ?
>>>
>>> Sure, as long as none of the hacks slip through :) - I've just pushed
>>> out a mild rework based on Lorenzo's v9, which I hope shouldn't break
>>> anything for you.
>>>
>>
>> Ok sure, i will test and just the post out the stuff from your branch then
>> mostly by tomorrow.
>
>Cool. We're rather hoping that the ACPI stuff is good to go for 4.10
>now, so it's probably worth pulling the rest of that in (beyond the one
>patch I picked) to make sure the of_dma_configure/acpi_dma_configure
>paths don't inadvertently diverge.
>
I rebased and was testing your branch with Lorenzo's series. One thing
i am still trying to get right is the acpi_dma_configure call. With your
series dma_configure calls pci_dma/of_dma configure, so i am just adding
acpi_dma_configure call there for non-pci ACPI devices as well. I see that
acpi_dma_configure right now is called from acpi_bind_one and
iort_add_smmu_platform_device, both go through the really_probe function
path, so moving acpi_dma_configure from the above the two functions
to dma_configure. I remember we discussed this on another thread, so
hopefully it is correct. I do not have an platform to test the ACPI though.
I will take some testing help on V4 for this.
>>> Having thought a bit more about the add/remove thing, I'm inclined to
>>> agree that the group numbering itself may not be that big an issue in
>>> practice - sure, it could break my little script, but it looks like QEMU
>>> and such work with the device ID rather than the group number directly,
>>> so might not even notice. However, the fact remains that the callbacks
>>> are intended to handle a device being added to/removed from its bus, and
>>> will continue to do so on other platforms, so I don't like the idea of
>>> introducing needlessly different behaviour. If you unbind a driver, the
>>> stream IDs and everything don't stop existing at the hardware level; the
>>> struct device to which the in-kernel data belongs still exists and
>>> doesn't stop being associated with its bus. There's no good reason for
>>> freeing SMEs that we'll only reallocate again (inadequately-specced
>>> hardware with not enough SMRs/contexts is not a *good* reason), and
>>
>> ok, so SMRs/contexts was the reason i was adding the remove_dev
>> callback, but if thats not good enough then there was no other
>> intention.
>>
>>> there are also some strong arguments against letting any stream IDs the
>>> kernel knows about go back to bypass after a driver has been bound - by
>>
>> ok, but not sure why is this so ?
>
>Any device the kernel is in control of, having bound a driver to it,
>definitely should not be doing DMA after that driver is unbound...
>
ok.
>>> keeping groups around as expected that's something we can implement
>>> quite easily without having to completely lock down bypass for stream
>>> IDs the kernel *doesn't* know about.
>>>
>>
>> So do you mean in this case to keep the unbound device's group/context bank
>> to bypass rather than resetting the streamids ?
>
>...which we can easily enforce by keeping the device attached to its
>default domain, in which nothing should be mapped by that point (we
>could even have a group notifier switch its S2CRs to faulting entries
>for extreme paranoia). Freeing the SMRs means those stream IDs would
>instead fall back to the default "unmatched" behaviour, which in general
>is going to be bypass, and thus allow DMA attacks.
>
>It's harder to disable unmatched bypass in general, because we may have
>devices which physically master through the SMMU but want low latency
>more than they want translation (at the moment the best we can do is
>leave the kernel unaware of those stream IDs), or there could be unknown
>devices under control of the firmware or other agents which we would
>disrupt by hitting a system-wide switch.
>
ok thanks, understand the point now. Agree that putting the previously bound
devices to fault is right than putting it to bypass. So the notifier to switch the
S2CRs to faulting entries can be added as a separate patch on top of this.
Regards,
Sricharan
^ permalink raw reply
* [PATCH net-next 1/5] net: mvneta: Use cacheable memory to store the rx buffer virtual address
From: Gregory CLEMENT @ 2016-11-28 17:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161128163548.70181560@xhacker>
Hi Jisheng,
On lun., nov. 28 2016, Jisheng Zhang <jszhang@marvell.com> wrote:
> Hi Gregory,
>
> On Fri, 25 Nov 2016 16:30:14 +0100 Gregory CLEMENT wrote:
>
>> Until now the virtual address of the received buffer were stored in the
>> cookie field of the rx descriptor. However, this field is 32-bits only
>> which prevents to use the driver on a 64-bits architecture.
>>
>> With this patch the virtual address is stored in an array not shared with
>> the hardware (no more need to use the DMA API). Thanks to this, it is
>> possible to use cache contrary to the access of the rx descriptor member.
>>
>> The change is done in the swbm path only because the hwbm uses the cookie
>> field, this also means that currently the hwbm is not usable in 64-bits.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> drivers/net/ethernet/marvell/mvneta.c | 96 ++++++++++++++++++++++++----
>> 1 file changed, 84 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
>> index 87274d4ab102..b6849f88cab7 100644
>> --- a/drivers/net/ethernet/marvell/mvneta.c
>> +++ b/drivers/net/ethernet/marvell/mvneta.c
>> @@ -561,6 +561,9 @@ struct mvneta_rx_queue {
>> u32 pkts_coal;
>> u32 time_coal;
>>
>> + /* Virtual address of the RX buffer */
>> + void **buf_virt_addr;
>
> can we store buf_phys_addr in cacheable memory as well?
Even if we store in in cacheable memory we will still need to store it
in the buffer descriptor as it is used by the hardware.
>
>> +
>> /* Virtual address of the RX DMA descriptors array */
>> struct mvneta_rx_desc *descs;
>>
>> @@ -1573,10 +1576,14 @@ static void mvneta_tx_done_pkts_coal_set(struct mvneta_port *pp,
>>
>> /* Handle rx descriptor fill by setting buf_cookie and buf_phys_addr */
>> static void mvneta_rx_desc_fill(struct mvneta_rx_desc *rx_desc,
>> - u32 phys_addr, u32 cookie)
>> + u32 phys_addr, void *virt_addr,
>> + struct mvneta_rx_queue *rxq)
>> {
>> - rx_desc->buf_cookie = cookie;
>> + int i;
>> +
>> rx_desc->buf_phys_addr = phys_addr;
>> + i = rx_desc - rxq->descs;
>> + rxq->buf_virt_addr[i] = virt_addr;
>> }
>>
>> /* Decrement sent descriptors counter */
>> @@ -1781,7 +1788,8 @@ EXPORT_SYMBOL_GPL(mvneta_frag_free);
>>
>> /* Refill processing for SW buffer management */
>> static int mvneta_rx_refill(struct mvneta_port *pp,
>> - struct mvneta_rx_desc *rx_desc)
>> + struct mvneta_rx_desc *rx_desc,
>> + struct mvneta_rx_queue *rxq)
>>
>> {
>> dma_addr_t phys_addr;
>> @@ -1799,7 +1807,7 @@ static int mvneta_rx_refill(struct mvneta_port *pp,
>> return -ENOMEM;
>> }
>>
>> - mvneta_rx_desc_fill(rx_desc, phys_addr, (u32)data);
>> + mvneta_rx_desc_fill(rx_desc, phys_addr, data, rxq);
>> return 0;
>> }
>>
>> @@ -1861,7 +1869,12 @@ static void mvneta_rxq_drop_pkts(struct mvneta_port *pp,
>>
>> for (i = 0; i < rxq->size; i++) {
>> struct mvneta_rx_desc *rx_desc = rxq->descs + i;
>> - void *data = (void *)rx_desc->buf_cookie;
>> + void *data;
>> +
>> + if (!pp->bm_priv)
>> + data = rxq->buf_virt_addr[i];
>> + else
>> + data = (void *)(uintptr_t)rx_desc->buf_cookie;
>>
>> dma_unmap_single(pp->dev->dev.parent, rx_desc->buf_phys_addr,
>> MVNETA_RX_BUF_SIZE(pp->pkt_size), DMA_FROM_DEVICE);
>> @@ -1894,12 +1907,13 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
>> unsigned char *data;
>> dma_addr_t phys_addr;
>> u32 rx_status, frag_size;
>> - int rx_bytes, err;
>> + int rx_bytes, err, index;
>>
>> rx_done++;
>> rx_status = rx_desc->status;
>> rx_bytes = rx_desc->data_size - (ETH_FCS_LEN + MVNETA_MH_SIZE);
>> - data = (unsigned char *)rx_desc->buf_cookie;
>> + index = rx_desc - rxq->descs;
>> + data = (unsigned char *)rxq->buf_virt_addr[index];
>> phys_addr = rx_desc->buf_phys_addr;
>>
>> if (!mvneta_rxq_desc_is_first_last(rx_status) ||
>> @@ -1938,7 +1952,7 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
>> }
>>
>> /* Refill processing */
>> - err = mvneta_rx_refill(pp, rx_desc);
>> + err = mvneta_rx_refill(pp, rx_desc, rxq);
>> if (err) {
>> netdev_err(dev, "Linux processing - Can't refill\n");
>> rxq->missed++;
>> @@ -2020,7 +2034,7 @@ static int mvneta_rx_hwbm(struct mvneta_port *pp, int rx_todo,
>> rx_done++;
>> rx_status = rx_desc->status;
>> rx_bytes = rx_desc->data_size - (ETH_FCS_LEN + MVNETA_MH_SIZE);
>> - data = (unsigned char *)rx_desc->buf_cookie;
>> + data = (u8 *)(uintptr_t)rx_desc->buf_cookie;
>> phys_addr = rx_desc->buf_phys_addr;
>> pool_id = MVNETA_RX_GET_BM_POOL_ID(rx_desc);
>> bm_pool = &pp->bm_priv->bm_pools[pool_id];
>> @@ -2708,6 +2722,57 @@ static int mvneta_poll(struct napi_struct *napi, int budget)
>> return rx_done;
>> }
>>
>> +/* Refill processing for HW buffer management */
>> +static int mvneta_rx_hwbm_refill(struct mvneta_port *pp,
>> + struct mvneta_rx_desc *rx_desc)
>> +
>> +{
>> + dma_addr_t phys_addr;
>> + void *data;
>> +
>> + data = mvneta_frag_alloc(pp->frag_size);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + phys_addr = dma_map_single(pp->dev->dev.parent, data,
>> + MVNETA_RX_BUF_SIZE(pp->pkt_size),
>> + DMA_FROM_DEVICE);
>> + if (unlikely(dma_mapping_error(pp->dev->dev.parent, phys_addr))) {
>> + mvneta_frag_free(pp->frag_size, data);
>> + return -ENOMEM;
>> + }
>> +
>> + phys_addr += pp->rx_offset_correction;
>> + rx_desc->buf_phys_addr = phys_addr;
>> + rx_desc->buf_cookie = (uintptr_t)data;
>> +
>> + return 0;
>> +}
>> +
>> +/* Handle rxq fill: allocates rxq skbs; called when initializing a port */
>> +static int mvneta_rxq_bm_fill(struct mvneta_port *pp,
>> + struct mvneta_rx_queue *rxq,
>> + int num)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < num; i++) {
>> + memset(rxq->descs + i, 0, sizeof(struct mvneta_rx_desc));
>> + if (mvneta_rx_hwbm_refill(pp, rxq->descs + i) != 0) {
>> + netdev_err(pp->dev, "%s:rxq %d, %d of %d buffs filled\n",
>> + __func__, rxq->id, i, num);
>> + break;
>> + }
>> + }
>> +
>> + /* Add this number of RX descriptors as non occupied (ready to
>> + * get packets)
>> + */
>> + mvneta_rxq_non_occup_desc_add(pp, rxq, i);
>> +
>> + return i;
>> +}
>> +
>> /* Handle rxq fill: allocates rxq skbs; called when initializing a port */
>> static int mvneta_rxq_fill(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
>> int num)
>> @@ -2716,7 +2781,7 @@ static int mvneta_rxq_fill(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
>>
>> for (i = 0; i < num; i++) {
>> memset(rxq->descs + i, 0, sizeof(struct mvneta_rx_desc));
>> - if (mvneta_rx_refill(pp, rxq->descs + i) != 0) {
>> + if (mvneta_rx_refill(pp, rxq->descs + i, rxq) != 0) {
>> netdev_err(pp->dev, "%s:rxq %d, %d of %d buffs filled\n",
>> __func__, rxq->id, i, num);
>> break;
>> @@ -2784,14 +2849,21 @@ static int mvneta_rxq_init(struct mvneta_port *pp,
>> mvneta_rxq_buf_size_set(pp, rxq,
>> MVNETA_RX_BUF_SIZE(pp->pkt_size));
>> mvneta_rxq_bm_disable(pp, rxq);
>> +
>> + rxq->buf_virt_addr = devm_kmalloc(pp->dev->dev.parent,
>> + rxq->size * sizeof(void *),
>> + GFP_KERNEL);
>
> I would suggest allocate this buffer during probe. Otherwise, there's
> memory leak if we either change the mtu or close then open the eth in
> a loop, e.g
>
> while true
> do
> ifconfig eth0 up
> ifconfig eth0 down
> done
Indeed, I will move it.
Thanks,
Gregory
>
> Thanks,
> Jisheng
>
>> + if (!rxq->buf_virt_addr)
>> + return -ENOMEM;
>> +
>> + mvneta_rxq_fill(pp, rxq, rxq->size);
>> } else {
>> mvneta_rxq_bm_enable(pp, rxq);
>> mvneta_rxq_long_pool_set(pp, rxq);
>> mvneta_rxq_short_pool_set(pp, rxq);
>> + mvneta_rxq_bm_fill(pp, rxq, rxq->size);
>> }
>>
>> - mvneta_rxq_fill(pp, rxq, rxq->size);
>> -
>> return 0;
>> }
>>
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH] arm64: head.S: Fix CNTHCTL_EL2 access on VHE system
From: Marc Zyngier @ 2016-11-28 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480351438-11548-1-git-send-email-jintack@cs.columbia.edu>
On 28/11/16 16:43, Jintack Lim wrote:
> From: Jintack <jintack@cs.columbia.edu>
>
> Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
> EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
> are 11th and 10th bits respectively when E2H is set. Current code is
> unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set.
>
> In fact, we don't need to set those two bits, which allow EL1 and EL0 to
> access physical timer and counter respectively, if E2H and TGE are set
> for the host kernel. They will be configured later as necessary. First,
> we don't need to configure those bits for EL1, since the host kernel
> runs in EL2. It is a hypervisor's responsibility to configure them
> before entering a VM, which runs in EL0 and EL1. Second, EL0 accesses
> are configured in the later stage of boot process.
>
> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
> ---
> arch/arm64/kernel/head.S | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 332e331..bc3d2db 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -524,10 +524,16 @@ set_hcr:
> msr hcr_el2, x0
> isb
>
> - /* Generic timers. */
> + /*
> + * Allow Non-secure EL1 and EL0 to access physical timer and counter.
> + * This is not necessary for VHE, since the host kernel runs in EL2,
> + * and EL0 accesses are configured in the later stage of boot process.
> + */
> + cbnz x2, 1f
> mrs x0, cnthctl_el2
> orr x0, x0, #3 // Enable EL1 physical timers
> msr cnthctl_el2, x0
> +1:
> msr cntvoff_el2, xzr // Clear virtual offset
>
> #ifdef CONFIG_ARM_GIC_V3
>
Nice catch. It may be worth documenting that when HCR_EL2.E2H == 1,
CNTHCTL_EL2 has the same bit layout as CNTKCTL_EL1, allowing the kernel
designed to run at EL1 to transparently mess with the EL0 bits (as
CNTHCTL_EL2 and CNTKCTL_EL1 are the same register in this configuration).
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* arasan,sdhci.txt "compatibility" DT binding
From: Mason @ 2016-11-28 16:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <14771688.Q8f97gHC2H@wuerfel>
On 28/11/2016 17:15, Arnd Bergmann wrote:
> On Monday, November 28, 2016 4:44:39 PM CET Mason wrote:
>
>> Hello,
>>
>> @Shawn Lin, could you take a look below and tell me exactly
>> which IP core(s) Rockchip is using in its SoCs?
>>
>> Based on the feedback I received, here is an updated list of
>> compatible strings and controller versions dealt with by the
>> drivers/mmc/host/sdhci-of-arasan.c code.
>>
>>
>> Xilinx Zynq:
>> "SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller"
>> "arasan,sdhci-8.9a"
>> NB: 8.9a is the documentation revision (dated 2011-10-19)
>> subsequent tweaks labeled 9.0a, 9.1a, 9.2a
>>
>> Xilinx ZynqMP:
>> "SD3.0 / SDIO3.0 / eMMC4.51 AHB Host Controller"
>> "arasan,sdhci-8.9a"
>> NB: using the same compatible string as Zynq
>>
>> Sigma SMP87xx
>> "SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller"
>> no compatible string yet, platform-specific init required
>>
>> APM:
>> "SD3.0 / SDIO3.0 / eMMC4.41 AHB Host Controller"
>> "arasan,sdhci-4.9a"
>> NB: 4.9a appears to be the documentation revision
>> no functional diff with "arasan,sdhci-8.9a"
>>
>> Rockchip
>> Exact IP unknown, waiting for Shawn's answer
>> "arasan,sdhci-5.1"
>> NB: 5.1 appears to refer to the eMMC standard supported
>>
>>
>> On a final note, there are many variations of the Arasan IP.
>> I've tracked down at least the following:
>>
>> SD_2.0_SDIO_2.0__MMC_3.31_AHB_Host_Controller.pdf
>> SD_3.0_SDIO_3.0_eMMC_4.41_OCP_Host_Controller.pdf
>> SD_3.0_SDIO_3.0_eMMC_4.4__AHB_Host_Controller.pdf
>> SD_3.0_SDIO_3.0_eMMC_4.51_Host_Controller.pdf
>> SD_3.0_SDIO_3.0_eMMC_4.5__Host_Controller.pdf
>> SD_4.1_SDIO_4.1_eMMC_4.51_Host_Controller.pdf
>> SD_4.1_SDIO_4.1_eMMC_5.1__Host_Controller.pdf
>>
>> It seems to me the compatible string should specify
>> the SD/SDIO version AND the eMMC version, since it
>> seems many combinations are allowed, e.g. eMMC 4.51
>> has two possible SD versions.
>>
>> What do you think?
>
> It seems wrong to have the eMMC or SD version in the compatible
> string. Is that the only difference between the documents you
> found? Normally there should be a version of IP block itself,
> besides the supported protocol.
But that is exactly the problem :-)
Nowhere in the documentation do they specify an "IP version".
Some documents do provide a revision number, but that's just
a *documentation* revision number, e.g.
changes in version 3.6 : fix typos
changes in version 9.1a : update company logo
That's why Xilinx used "arasan,sdhci-8.9a" and APM used
"arasan,sdhci-4.9a". These are documentation revisions.
In my opinion, that information is mostly worthless.
Looking more closely at SD_3.0_SDIO_3.0_eMMC_4.4__AHB_Host_Controller.pdf
(User Guide, which has more info than Datasheet) I see this:
Changed Host Controller Version Register value from 16'h0002 to 16'h7501
Changed Host Controller Version Register value from 16'h8301 to 16'h8401
Changed Host Controller Version Register value from 16'h8401 to 16'h8501
Changed Host Controller Version Register to 16'h9502
Changed Host Controller Version Register to 16'h9602
Changed Host Controller Version Register to 16'h9902
Host controller version register (offset 0FEh)
Vendor Version Number 15:8
HwInit=0x99
This status is reserved for the vendor version number.
The HD should not use this status.
Specification Version Number 7:0
HwInit=0x02
This status indicates the Host Controller Spec. Version.
The upper and lower 4-bits indicate the version.
Description
00 - SD Host Specification version 1.0
01 - SD Host Specification version 2.00
including only the feature of the Test Register
02 - SD Host Specification Version 3.00
others - Reserved
I'm not sure what this "Vendor Version Number" specifies, nor if is
guaranteed to be unique across controllers.
In SD_3.0_SDIO_3.0_eMMC_4.5__Host_Controller_UserGuide.pdf,
they write "The Vendor Version Number is set to 0x10 (1.0)"
I don't have a UserGuide for "arasan,sdhci-5.1".
Regards.
^ permalink raw reply
* [PATCH] KVM: arm/arm64: Access CNTHCTL_EL2 bit fields correctly
From: Jintack Lim @ 2016-11-28 16:46 UTC (permalink / raw)
To: linux-arm-kernel
Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
are 11th and 10th bits respectively when E2H is set. Current code is
unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set, which
may allow guest OS to access physical timer. So, fix it.
Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
---
arch/arm/include/asm/kvm_timer.h | 33 +++++++++++++++++++
arch/arm64/include/asm/kvm_timer.h | 62 ++++++++++++++++++++++++++++++++++++
include/clocksource/arm_arch_timer.h | 6 ++--
virt/kvm/arm/hyp/timer-sr.c | 8 ++---
4 files changed, 103 insertions(+), 6 deletions(-)
create mode 100644 arch/arm/include/asm/kvm_timer.h
create mode 100644 arch/arm64/include/asm/kvm_timer.h
diff --git a/arch/arm/include/asm/kvm_timer.h b/arch/arm/include/asm/kvm_timer.h
new file mode 100644
index 0000000..d19d4b3
--- /dev/null
+++ b/arch/arm/include/asm/kvm_timer.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2016 - Columbia University
+ * Author: Jintack Lim <jintack@cs.columbia.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ARM_KVM_TIMER_H__
+#define __ARM_KVM_TIMER_H__
+
+#include <clocksource/arm_arch_timer.h>
+
+static inline u32 __hyp_text get_el1pcten(void)
+{
+ return CNTHCTL_EL1PCTEN_NVHE;
+}
+
+static inline u32 __hyp_text get_el1pcen(void)
+{
+ return CNTHCTL_EL1PCEN_NVHE;
+}
+
+#endif /* __ARM_KVM_TIMER_H__ */
diff --git a/arch/arm64/include/asm/kvm_timer.h b/arch/arm64/include/asm/kvm_timer.h
new file mode 100644
index 0000000..153f3da
--- /dev/null
+++ b/arch/arm64/include/asm/kvm_timer.h
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2016 - Columbia University
+ * Author: Jintack Lim <jintack@cs.columbia.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ARM64_KVM_TIMER_H__
+#define __ARM64_KVM_TIMER_H__
+
+#include <asm/kvm_hyp.h>
+#include <clocksource/arm_arch_timer.h>
+
+static inline u32 __hyp_text get_el1pcten_vhe(void)
+{
+ return CNTHCTL_EL1PCTEN_VHE;
+}
+
+static inline u32 __hyp_text get_el1pcten_nvhe(void)
+{
+ return CNTHCTL_EL1PCTEN_NVHE;
+}
+
+static hyp_alternate_select(get_el1pcten_arch,
+ get_el1pcten_nvhe, get_el1pcten_vhe,
+ ARM64_HAS_VIRT_HOST_EXTN);
+
+static inline u32 __hyp_text get_el1pten_vhe(void)
+{
+ return CNTHCTL_EL1PTEN_VHE;
+}
+
+static inline u32 __hyp_text get_el1pcen_nvhe(void)
+{
+ return CNTHCTL_EL1PCEN_NVHE;
+}
+
+static hyp_alternate_select(get_el1pcen_arch,
+ get_el1pcen_nvhe, get_el1pten_vhe,
+ ARM64_HAS_VIRT_HOST_EXTN);
+
+static inline u32 __hyp_text get_el1pcten(void)
+{
+ return get_el1pcten_arch()();
+}
+
+static inline u32 __hyp_text get_el1pcen(void)
+{
+ return get_el1pcen_arch()();
+}
+
+#endif /* __ARM64_KVM_TIMER_H__ */
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index caedb74..4094529 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -23,8 +23,10 @@
#define ARCH_TIMER_CTRL_IT_MASK (1 << 1)
#define ARCH_TIMER_CTRL_IT_STAT (1 << 2)
-#define CNTHCTL_EL1PCTEN (1 << 0)
-#define CNTHCTL_EL1PCEN (1 << 1)
+#define CNTHCTL_EL1PCTEN_NVHE (1 << 0)
+#define CNTHCTL_EL1PCEN_NVHE (1 << 1)
+#define CNTHCTL_EL1PCTEN_VHE (1 << 10)
+#define CNTHCTL_EL1PTEN_VHE (1 << 11)
#define CNTHCTL_EVNTEN (1 << 2)
#define CNTHCTL_EVNTDIR (1 << 3)
#define CNTHCTL_EVNTI (0xF << 4)
diff --git a/virt/kvm/arm/hyp/timer-sr.c b/virt/kvm/arm/hyp/timer-sr.c
index 798866a..f3feee0 100644
--- a/virt/kvm/arm/hyp/timer-sr.c
+++ b/virt/kvm/arm/hyp/timer-sr.c
@@ -15,11 +15,11 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <clocksource/arm_arch_timer.h>
#include <linux/compiler.h>
#include <linux/kvm_host.h>
#include <asm/kvm_hyp.h>
+#include <asm/kvm_timer.h>
/* vcpu is already in the HYP VA space */
void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
@@ -37,7 +37,7 @@ void __hyp_text __timer_save_state(struct kvm_vcpu *vcpu)
/* Allow physical timer/counter access for the host */
val = read_sysreg(cnthctl_el2);
- val |= CNTHCTL_EL1PCTEN | CNTHCTL_EL1PCEN;
+ val |= get_el1pcten() | get_el1pcen();
write_sysreg(val, cnthctl_el2);
/* Clear cntvoff for the host */
@@ -55,8 +55,8 @@ void __hyp_text __timer_restore_state(struct kvm_vcpu *vcpu)
* Physical counter access is allowed
*/
val = read_sysreg(cnthctl_el2);
- val &= ~CNTHCTL_EL1PCEN;
- val |= CNTHCTL_EL1PCTEN;
+ val &= ~get_el1pcen();
+ val |= get_el1pcten();
write_sysreg(val, cnthctl_el2);
if (timer->enabled) {
--
1.9.1
^ permalink raw reply related
* [PATCH] arm64: head.S: Fix CNTHCTL_EL2 access on VHE system
From: Jintack Lim @ 2016-11-28 16:43 UTC (permalink / raw)
To: linux-arm-kernel
From: Jintack <jintack@cs.columbia.edu>
Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
are 11th and 10th bits respectively when E2H is set. Current code is
unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set.
In fact, we don't need to set those two bits, which allow EL1 and EL0 to
access physical timer and counter respectively, if E2H and TGE are set
for the host kernel. They will be configured later as necessary. First,
we don't need to configure those bits for EL1, since the host kernel
runs in EL2. It is a hypervisor's responsibility to configure them
before entering a VM, which runs in EL0 and EL1. Second, EL0 accesses
are configured in the later stage of boot process.
Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
---
arch/arm64/kernel/head.S | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 332e331..bc3d2db 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -524,10 +524,16 @@ set_hcr:
msr hcr_el2, x0
isb
- /* Generic timers. */
+ /*
+ * Allow Non-secure EL1 and EL0 to access physical timer and counter.
+ * This is not necessary for VHE, since the host kernel runs in EL2,
+ * and EL0 accesses are configured in the later stage of boot process.
+ */
+ cbnz x2, 1f
mrs x0, cnthctl_el2
orr x0, x0, #3 // Enable EL1 physical timers
msr cnthctl_el2, x0
+1:
msr cntvoff_el2, xzr // Clear virtual offset
#ifdef CONFIG_ARM_GIC_V3
--
1.9.1
^ permalink raw reply related
* [GIT PULL] ARM: keystone: add TI SCI protocol support for v4.10
From: Tero Kristo @ 2016-11-28 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e6b04eeb-ebfb-2da0-834b-1da440a35783@ti.com>
Gentle ping on this, is anybody going to pull this one?
-Tero
On 27/10/16 12:30, Tero Kristo wrote:
> Hi Arnd, Olof, Kevin,
>
> This pull introduces the TI SCI protocol support for keystone family of
> devices, targeted for v4.10 merge window. We discussed with Santosh
> (keystone maintainer) that it would probably be better that I'll be
> sending the pull requests for this directly, avoiding one extra step of
> merges.
>
> -Tero
>
>
> The following changes since commit
> 1001354ca34179f3db924eb66672442a173147dc:
>
> Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
>
> are available in the git repository at:
>
> https://github.com/t-kristo/linux-pm.git for-4.10-ti-sci-base
>
> for you to fetch changes up to 912cffb4ed8612dc99ee0251cc0c9785855162cd:
>
> firmware: ti_sci: Add support for reboot core service (2016-10-27
> 12:09:12 +0300)
>
> ----------------------------------------------------------------
> Nishanth Menon (5):
> Documentation: Add support for TI System Control Interface
> (TI-SCI) protocol
> firmware: Add basic support for TI System Control Interface
> (TI-SCI) protocol
> firmware: ti_sci: Add support for Device control
> firmware: ti_sci: Add support for Clock control
> firmware: ti_sci: Add support for reboot core service
>
> .../devicetree/bindings/arm/keystone/ti,sci.txt | 81 +
> MAINTAINERS | 10 +
> drivers/firmware/Kconfig | 15 +
> drivers/firmware/Makefile | 1 +
> drivers/firmware/ti_sci.c | 1991
> ++++++++++++++++++++
> drivers/firmware/ti_sci.h | 492 +++++
> include/linux/soc/ti/ti_sci_protocol.h | 249 +++
> 7 files changed, 2839 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> create mode 100644 drivers/firmware/ti_sci.c
> create mode 100644 drivers/firmware/ti_sci.h
> create mode 100644 include/linux/soc/ti/ti_sci_protocol.h
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf
From: David Lechner @ 2016-11-28 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480351226-3332-1-git-send-email-david@lechnology.com>
This SoC has a separate pin controller for configuring pullup/pulldown
bias on groups of pins.
Signed-off-by: David Lechner <david@lechnology.com>
---
v2 changes:
* Moved pin-controller at 22c00c device node after gpio at 226000 (there seem to be
more nodes in proper order here compared to the i2c at 228000 node suggested by
Sekhar)
arch/arm/boot/dts/da850.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 8945815..7a8d38e 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -442,6 +442,11 @@
ti,davinci-gpio-unbanked = <0>;
status = "disabled";
};
+ pinconf: pin-controller at 22c00c {
+ compatible = "ti,da850-pupd";
+ reg = <0x22c00c 0x8>;
+ status = "disabled";
+ };
mcasp0: mcasp at 100000 {
compatible = "ti,da830-mcasp-audio";
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/3] pinctrl: New driver for TI DA850/OMAP-L138/AM18XX pinconf
From: David Lechner @ 2016-11-28 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480351226-3332-1-git-send-email-david@lechnology.com>
This adds a new driver for pinconf on TI DA850/OMAP-L138/AM18XX. These
SoCs have a separate controller for controlling pullup/pulldown groups.
Signed-off-by: David Lechner <david@lechnology.com>
Reviewed-by: Sekhar Nori <nsekhar@ti.com>
---
v2 changes:
* s/DA8XX/DA850/ in the commit messages
* Fix `get_get` typo in two function names
drivers/pinctrl/Kconfig | 9 ++
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-da850-pupd.c | 210 +++++++++++++++++++++++++++++++++++
3 files changed, 220 insertions(+)
create mode 100644 drivers/pinctrl/pinctrl-da850-pupd.c
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 5f40ad6..54044a8 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -93,6 +93,15 @@ config PINCTRL_AMD
Requires ACPI/FDT device enumeration code to set up a platform
device.
+config PINCTRL_DA850_PUPD
+ tristate "TI DA850/OMAP-L138/AM18XX pullup/pulldown groups"
+ depends on OF && (ARCH_DAVINCI_DA850 || COMPILE_TEST)
+ select PINCONF
+ select GENERIC_PINCONF
+ help
+ Driver for TI DA850/OMAP-L138/AM18XX pinconf. Used to control
+ pullup/pulldown pin groups.
+
config PINCTRL_DIGICOLOR
bool
depends on OF && (ARCH_DIGICOLOR || COMPILE_TEST)
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 3b8e6f7..25d50a8 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_PINCTRL_BF60x) += pinctrl-adi2-bf60x.o
obj-$(CONFIG_PINCTRL_AT91) += pinctrl-at91.o
obj-$(CONFIG_PINCTRL_AT91PIO4) += pinctrl-at91-pio4.o
obj-$(CONFIG_PINCTRL_AMD) += pinctrl-amd.o
+obj-$(CONFIG_PINCTRL_DA850_PUPD) += pinctrl-da850-pupd.o
obj-$(CONFIG_PINCTRL_DIGICOLOR) += pinctrl-digicolor.o
obj-$(CONFIG_PINCTRL_FALCON) += pinctrl-falcon.o
obj-$(CONFIG_PINCTRL_MAX77620) += pinctrl-max77620.o
diff --git a/drivers/pinctrl/pinctrl-da850-pupd.c b/drivers/pinctrl/pinctrl-da850-pupd.c
new file mode 100644
index 0000000..b36a90a
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-da850-pupd.c
@@ -0,0 +1,210 @@
+/*
+ * Pinconf driver for TI DA850/OMAP-L138/AM18XX pullup/pulldown groups
+ *
+ * Copyright (C) 2016 David Lechner
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/platform_device.h>
+
+#define DA850_PUPD_ENA 0x00
+#define DA850_PUPD_SEL 0x04
+
+struct da850_pupd_data {
+ void __iomem *base;
+ struct pinctrl_desc desc;
+ struct pinctrl_dev *pinctrl;
+};
+
+static const char * const da850_pupd_group_names[] = {
+ "cp0", "cp1", "cp2", "cp3", "cp4", "cp5", "cp6", "cp7",
+ "cp8", "cp9", "cp10", "cp11", "cp12", "cp13", "cp14", "cp15",
+ "cp16", "cp17", "cp18", "cp19", "cp20", "cp21", "cp22", "cp23",
+ "cp24", "cp25", "cp26", "cp27", "cp28", "cp29", "cp30", "cp31",
+};
+
+static int da850_pupd_get_groups_count(struct pinctrl_dev *pctldev)
+{
+ return ARRAY_SIZE(da850_pupd_group_names);
+}
+
+static const char *da850_pupd_get_group_name(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ return da850_pupd_group_names[selector];
+}
+
+static int da850_pupd_get_group_pins(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ const unsigned int **pins,
+ unsigned int *num_pins)
+{
+ *num_pins = 0;
+
+ return 0;
+}
+
+static const struct pinctrl_ops da850_pupd_pctlops = {
+ .get_groups_count = da850_pupd_get_groups_count,
+ .get_group_name = da850_pupd_get_group_name,
+ .get_group_pins = da850_pupd_get_group_pins,
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_group,
+ .dt_free_map = pinconf_generic_dt_free_map,
+};
+
+static int da850_pupd_pin_config_group_get(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ unsigned long *config)
+{
+ struct da850_pupd_data *data = pinctrl_dev_get_drvdata(pctldev);
+ enum pin_config_param param = pinconf_to_config_param(*config);
+ u32 val;
+ u16 arg;
+
+ val = readl(data->base + DA850_PUPD_ENA);
+ arg = !!(~val & BIT(selector));
+
+ switch (param) {
+ case PIN_CONFIG_BIAS_DISABLE:
+ break;
+ case PIN_CONFIG_BIAS_PULL_UP:
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ if (arg) {
+ /* bias is disabled */
+ arg = 0;
+ break;
+ }
+ val = readl(data->base + DA850_PUPD_SEL);
+ if (param == PIN_CONFIG_BIAS_PULL_DOWN)
+ val = ~val;
+ arg = !!(val & BIT(selector));
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ *config = pinconf_to_config_packed(param, arg);
+
+ return 0;
+}
+
+static int da850_pupd_pin_config_group_set(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ unsigned long *configs,
+ unsigned int num_configs)
+{
+ struct da850_pupd_data *data = pinctrl_dev_get_drvdata(pctldev);
+ u32 ena, sel;
+ enum pin_config_param param;
+ u16 arg;
+ int i;
+
+ ena = readl(data->base + DA850_PUPD_ENA);
+ sel = readl(data->base + DA850_PUPD_SEL);
+
+ for (i = 0; i < num_configs; i++) {
+ param = pinconf_to_config_param(configs[i]);
+ arg = pinconf_to_config_argument(configs[i]);
+
+ switch (param) {
+ case PIN_CONFIG_BIAS_DISABLE:
+ ena &= ~BIT(selector);
+ break;
+ case PIN_CONFIG_BIAS_PULL_UP:
+ ena |= BIT(selector);
+ sel |= BIT(selector);
+ break;
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ ena |= BIT(selector);
+ sel &= ~BIT(selector);
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ writel(sel, data->base + DA850_PUPD_SEL);
+ writel(ena, data->base + DA850_PUPD_ENA);
+
+ return 0;
+}
+
+static const struct pinconf_ops da850_pupd_confops = {
+ .is_generic = true,
+ .pin_config_group_get = da850_pupd_pin_config_group_get,
+ .pin_config_group_set = da850_pupd_pin_config_group_set,
+};
+
+static int da850_pupd_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct da850_pupd_data *data;
+ struct resource *res;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ data->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(data->base)) {
+ dev_err(dev, "Could not map resource\n");
+ return PTR_ERR(data->base);
+ }
+
+ data->desc.name = dev_name(dev);
+ data->desc.pctlops = &da850_pupd_pctlops;
+ data->desc.confops = &da850_pupd_confops;
+ data->desc.owner = THIS_MODULE;
+
+ data->pinctrl = devm_pinctrl_register(dev, &data->desc, data);
+ if (IS_ERR(data->pinctrl)) {
+ dev_err(dev, "Failed to register pinctrl\n");
+ return PTR_ERR(data->pinctrl);
+ }
+
+ platform_set_drvdata(pdev, data);
+
+ return 0;
+}
+
+static int da850_pupd_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static const struct of_device_id da850_pupd_of_match[] = {
+ { .compatible = "ti,da850-pupd" },
+ { }
+};
+
+static struct platform_driver da850_pupd_driver = {
+ .driver = {
+ .name = "ti-da850-pupd",
+ .of_match_table = da850_pupd_of_match,
+ },
+ .probe = da850_pupd_probe,
+ .remove = da850_pupd_remove,
+};
+module_platform_driver(da850_pupd_driver);
+
+MODULE_AUTHOR("David Lechner <david@lechnology.com>");
+MODULE_DESCRIPTION("TI DA850/OMAP-L138/AM18XX pullup/pulldown configuration");
+MODULE_LICENSE("GPL");
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/3] devicetree: bindings: pinctrl: Add binding for ti, da850-pupd
From: David Lechner @ 2016-11-28 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480351226-3332-1-git-send-email-david@lechnology.com>
Device-tree bindings for TI DA850/OMAP-L138/AM18XX pullup/pulldown
pinconf controller.
Signed-off-by: David Lechner <david@lechnology.com>
Reviewed-by: Sekhar Nori <nsekhar@ti.com>
---
v2 changes:
* s/DA8XX/DA850/ in the commit message
.../devicetree/bindings/pinctrl/ti,da850-pupd.txt | 55 ++++++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt
diff --git a/Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt b/Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt
new file mode 100644
index 0000000..7f29805
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt
@@ -0,0 +1,55 @@
+* Pin configuration for TI DA850/OMAP-L138/AM18x
+
+These SoCs have a separate controller for setting bias (internal pullup/down).
+Bias can only be selected for groups rather than individual pins.
+
+Required Properties:
+
+ - compatible: Must be "ti,da850-pupd"
+ - reg: Base address and length of the memory resource used by the pullup/down
+ controller hardware module.
+
+The controller node also acts as a container for pin group configuration nodes.
+The names of these groups are ignored.
+
+Pin Group Node Properties:
+
+- groups: An array of strings, each string containing the name of a pin group.
+ Valid names are "cp0".."cp31".
+
+The pin configuration parameters use the generic pinconf bindings defined in
+pinctrl-bindings.txt in this directory. The supported parameters are
+bias-disable, bias-pull-up, bias-pull-down.
+
+
+Example
+-------
+
+In common dtsi file:
+
+ pinconf: pin-controller at 22c00c {
+ compatible = "ti,da850-pupd";
+ reg = <0x22c00c 0x8>;
+ };
+
+In board-specific file:
+
+ &pinconf {
+ pinctrl-0 = <&pinconf_bias_groups>;
+ pinctrl-names = "default";
+
+ pinconf_bias_groups: bias-groups {
+ pull-up {
+ groups = "cp30", "cp31";
+ bias-pull-up;
+ };
+ pull-down {
+ groups = "cp29", "cp28";
+ bias-pull-down;
+ };
+ disable {
+ groups = "cp27", "cp26";
+ bias-disable;
+ };
+ };
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH v2 0/3] TI DA850/OMAP-L138/AM18x pinconf
From: David Lechner @ 2016-11-28 16:40 UTC (permalink / raw)
To: linux-arm-kernel
This series adds a new driver and DT bindings for TI DA850/OMAP-L138/AM18x
pinconf (bias pullup/pulldown).
The motivation for this series is LEGO MINDSTORMS EV3 support. It needs most,
if not all, internal pullup/down resistors disabled in order to work correctly.
v2 changes:
* s/DA8XX/DA850/ in the commit messages
* Fix `get_get` typo in pinctrl-da850-pupd.c
* Moved pin-controller at 22c00c device node in da850.dtsi
David Lechner (3):
devicetree: bindings: pinctrl: Add binding for ti,da850-pupd
pinctrl: New driver for TI DA850/OMAP-L138/AM18XX pinconf
ARM: dts: da850: Add node for pullup/pulldown pinconf
.../devicetree/bindings/pinctrl/ti,da850-pupd.txt | 55 ++++++
arch/arm/boot/dts/da850.dtsi | 5 +
drivers/pinctrl/Kconfig | 9 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-da850-pupd.c | 210 +++++++++++++++++++++
5 files changed, 280 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/ti,da850-pupd.txt
create mode 100644 drivers/pinctrl/pinctrl-da850-pupd.c
--
2.7.4
^ permalink raw reply
* [PATCH] arm64: head.S: Fix CNTHCTL_EL2 access on VHE system
From: Jintack Lim @ 2016-11-28 16:37 UTC (permalink / raw)
To: linux-arm-kernel
From: Jintack <jintack@cs.columbia.edu>
Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
are 11th and 10th bits respectively when E2H is set. Current code is
unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set.
In fact, we don't need to set those two bits, which allow EL1 and EL0 to
access physical timer and counter respectively, if E2H and TGE are set
for the host kernel. They will be configured later as necessary. First,
we don't need to configure those bits for EL1, since the host kernel
runs in EL2. It is a hypervisor's responsibility to configure them
before entering a VM, which runs in EL0 and EL1. Second, EL0 accesses
are configured in the later stage of boot process.
Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
---
arch/arm64/kernel/head.S | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 332e331..bc3d2db 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -524,10 +524,16 @@ set_hcr:
msr hcr_el2, x0
isb
- /* Generic timers. */
+ /*
+ * Allow Non-secure EL1 and EL0 to access physical timer and counter.
+ * This is not necessary for VHE, since the host kernel runs in EL2,
+ * and EL0 accesses are configured in the later stage of boot process.
+ */
+ cbnz x2, 1f
mrs x0, cnthctl_el2
orr x0, x0, #3 // Enable EL1 physical timers
msr cnthctl_el2, x0
+1:
msr cntvoff_el2, xzr // Clear virtual offset
#ifdef CONFIG_ARM_GIC_V3
--
1.9.1
^ permalink raw reply related
* [PATCH] ARM: davinci_all_defconfig: Enable the da8xx usb otg
From: Alexandre Bailon @ 2016-11-28 16:30 UTC (permalink / raw)
To: linux-arm-kernel
The musb driver is enabled but the phy and the glue
for the da8xx are not enabled.
Enable them.
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
arch/arm/configs/davinci_all_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
index b5e978f..21c548f 100644
--- a/arch/arm/configs/davinci_all_defconfig
+++ b/arch/arm/configs/davinci_all_defconfig
@@ -159,8 +159,10 @@ CONFIG_USB=m
CONFIG_USB_MON=m
CONFIG_USB_STORAGE=m
CONFIG_USB_MUSB_HDRC=m
+CONFIG_USB_MUSB_DA8XX=m
CONFIG_MUSB_PIO_ONLY=y
CONFIG_USB_TEST=m
+CONFIG_NOP_USB_XCEIV=m
CONFIG_USB_GADGET=m
CONFIG_USB_GADGET_DEBUG_FILES=y
CONFIG_USB_GADGET_DEBUG_FS=y
--
2.7.3
^ permalink raw reply related
* [PATCH] ARM: davinci: da8xx: Fix sleeping function called from invalid context
From: Alexandre Bailon @ 2016-11-28 16:29 UTC (permalink / raw)
To: linux-arm-kernel
Everytime the usb20 phy is enabled, there is a
"sleeping function called from invalid context" BUG.
usb20_phy_clk_enable(), called with the irq disabled uses
clk_get() and clk_enable_prepapre() which may sleep.
Move clk_get() to da8xx_register_usb20_phy_clk() and
replace clk_prepare_enable() by clk_enable().
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
arch/arm/mach-davinci/usb-da8xx.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
index b010e5f..c9b5cd4 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -156,23 +156,23 @@ int __init da8xx_register_usb_refclkin(int rate)
return 0;
}
+static struct clk *usb20_clk;
+
static void usb20_phy_clk_enable(struct clk *clk)
{
- struct clk *usb20_clk;
int err;
u32 val;
u32 timeout = 500000; /* 500 msec */
val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
- usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
- if (IS_ERR(usb20_clk)) {
+ if (!usb20_clk || IS_ERR(usb20_clk)) {
pr_err("could not get usb20 clk: %ld\n", PTR_ERR(usb20_clk));
return;
}
/* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */
- err = clk_prepare_enable(usb20_clk);
+ err = clk_enable(usb20_clk);
if (err) {
pr_err("failed to enable usb20 clk: %d\n", err);
clk_put(usb20_clk);
@@ -197,8 +197,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
done:
- clk_disable_unprepare(usb20_clk);
- clk_put(usb20_clk);
+ clk_disable(usb20_clk);
}
static void usb20_phy_clk_disable(struct clk *clk)
@@ -287,6 +286,10 @@ int __init da8xx_register_usb20_phy_clk(bool use_usb_refclkin)
struct clk *parent;
int ret = 0;
+ usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
+ if (IS_ERR(usb20_clk))
+ return PTR_ERR(parent);
+
parent = clk_get(NULL, use_usb_refclkin ? "usb_refclkin" : "pll0_aux");
if (IS_ERR(parent))
return PTR_ERR(parent);
--
2.7.3
^ permalink raw reply related
* arasan,sdhci.txt "compatibility" DT binding
From: Sebastian Frias @ 2016-11-28 16:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <583C50E7.6030400@free.fr>
On 28/11/16 16:44, Mason wrote:
> Hello,
>
> @Shawn Lin, could you take a look below and tell me exactly
> which IP core(s) Rockchip is using in its SoCs?
>
> Based on the feedback I received, here is an updated list of
> compatible strings and controller versions dealt with by the
> drivers/mmc/host/sdhci-of-arasan.c code.
>
>
> Xilinx Zynq:
> "SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller"
> "arasan,sdhci-8.9a"
> NB: 8.9a is the documentation revision (dated 2011-10-19)
> subsequent tweaks labeled 9.0a, 9.1a, 9.2a
>
> Xilinx ZynqMP:
> "SD3.0 / SDIO3.0 / eMMC4.51 AHB Host Controller"
> "arasan,sdhci-8.9a"
> NB: using the same compatible string as Zynq
>
> Sigma SMP87xx
> "SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller"
> no compatible string yet, platform-specific init required
>
> APM:
> "SD3.0 / SDIO3.0 / eMMC4.41 AHB Host Controller"
> "arasan,sdhci-4.9a"
> NB: 4.9a appears to be the documentation revision
> no functional diff with "arasan,sdhci-8.9a"
>
> Rockchip
> Exact IP unknown, waiting for Shawn's answer
> "arasan,sdhci-5.1"
> NB: 5.1 appears to refer to the eMMC standard supported
>
>
> On a final note, there are many variations of the Arasan IP.
> I've tracked down at least the following:
>
> SD_2.0_SDIO_2.0__MMC_3.31_AHB_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.41_OCP_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.4__AHB_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.51_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.5__Host_Controller.pdf
> SD_4.1_SDIO_4.1_eMMC_4.51_Host_Controller.pdf
> SD_4.1_SDIO_4.1_eMMC_5.1__Host_Controller.pdf
>
> It seems to me the compatible string should specify
> the SD/SDIO version AND the eMMC version, since it
> seems many combinations are allowed, e.g. eMMC 4.51
> has two possible SD versions.
>
> What do you think?
I'm trying to picture this. Imagine:
a) SoC XYZ used two versions:
- SD_3.0_SDIO_3.0_eMMC_4.4__AHB_Host_Controller.pdf
- SD_3.0_SDIO_3.0_eMMC_4.5__Host_Controller.pdf
b) That the compatible suffixes were defined as "sd30-emmc44" and
"sd30-emmc45" respectively
c) That the chip-specific init is the same for both.
What would be the recommended way of dealing with that at DT/driver level?
1) XYZ's DT1: compatible = "arasan,sdhci-sd30-emmc44", "XYZ,sdhci"
DT2: compatible = "arasan,sdhci-sd30-emmc45", "XYZ,sdhci"
driver: match "XYZ,sdhci" for chip-specific init, and then leaves
"arasan,sdhci-sd30-emmc44" or arasan,sdhci-sd30-emmc45" for generic part
2) XYZ's DT1: compatible = "XYZ,arasan-sdhci-sd30-emmc44"
DT2: compatible = "XYZ,arasan-sdhci-sd30-emmc45"
driver: match "XYZ,arasan-sdhci-sd30-emmc44" or "XYZ,arasan-sdhci-sd30-emmc45"
for chip-specific init and generic parts
3) XYZ's DT1: compatible = "arasan,sdhci-sd30-emmc44"
DT2: compatible = "arasan,sdhci-sd30-emmc45"
driver: match "arasan,sdhci-sd30-emmc44" or "arasan,sdhci-sd30-emmc45" for
generic part; chip-specific init done somewhere else (bootloader?)
4) something else?
How would those solutions be affected if condition c) was changed to
"chip-specific init is different for both"?
>
> Regards.
>
^ permalink raw reply
* arasan,sdhci.txt "compatibility" DT binding
From: Arnd Bergmann @ 2016-11-28 16:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <583C50E7.6030400@free.fr>
On Monday, November 28, 2016 4:44:39 PM CET Mason wrote:
> Hello,
>
> @Shawn Lin, could you take a look below and tell me exactly
> which IP core(s) Rockchip is using in its SoCs?
>
> Based on the feedback I received, here is an updated list of
> compatible strings and controller versions dealt with by the
> drivers/mmc/host/sdhci-of-arasan.c code.
>
>
> Xilinx Zynq:
> "SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller"
> "arasan,sdhci-8.9a"
> NB: 8.9a is the documentation revision (dated 2011-10-19)
> subsequent tweaks labeled 9.0a, 9.1a, 9.2a
>
> Xilinx ZynqMP:
> "SD3.0 / SDIO3.0 / eMMC4.51 AHB Host Controller"
> "arasan,sdhci-8.9a"
> NB: using the same compatible string as Zynq
>
> Sigma SMP87xx
> "SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller"
> no compatible string yet, platform-specific init required
>
> APM:
> "SD3.0 / SDIO3.0 / eMMC4.41 AHB Host Controller"
> "arasan,sdhci-4.9a"
> NB: 4.9a appears to be the documentation revision
> no functional diff with "arasan,sdhci-8.9a"
>
> Rockchip
> Exact IP unknown, waiting for Shawn's answer
> "arasan,sdhci-5.1"
> NB: 5.1 appears to refer to the eMMC standard supported
>
>
> On a final note, there are many variations of the Arasan IP.
> I've tracked down at least the following:
>
> SD_2.0_SDIO_2.0__MMC_3.31_AHB_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.41_OCP_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.4__AHB_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.51_Host_Controller.pdf
> SD_3.0_SDIO_3.0_eMMC_4.5__Host_Controller.pdf
> SD_4.1_SDIO_4.1_eMMC_4.51_Host_Controller.pdf
> SD_4.1_SDIO_4.1_eMMC_5.1__Host_Controller.pdf
>
> It seems to me the compatible string should specify
> the SD/SDIO version AND the eMMC version, since it
> seems many combinations are allowed, e.g. eMMC 4.51
> has two possible SD versions.
>
> What do you think?
It seems wrong to have the eMMC or SD version in the compatible
string. Is that the only difference between the documents you
found? Normally there should be a version of IP block itself,
besides the supported protocol.
Arnd
^ permalink raw reply
* [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Andrew Lunn @ 2016-11-28 16:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2c59cc79-b6dc-9920-1725-a7785ff3b6bf@kleine-koenig.org>
On Mon, Nov 28, 2016 at 04:44:47PM +0100, Uwe Kleine-K?nig wrote:
> On 11/28/2016 02:17 PM, Andrew Lunn wrote:
> >> I still wonder (and didn't get an answer back when I asked about this)
> >> why a comment is preferred here. For other devices I know it's usual and
> >> requested by the maintainers to use:
> >>
> >> compatible = "exact name", "earlyer device to match driver";
> >>
> >> . This is more robust, documents the situation more formally and makes
> >> it better greppable. The price to pay is only a few bytes in the dtb
> >> which IMO is ok.
> >
> > We did discuss this a while back. The information is useless and
> > should to be ignored if present.
>
> Who is "we"?
Anybody on netdev a while back, but mostly Vivien and myself.
> I'd say fail to probe is the right thing to do. Of course that doesn't
> work for already supported models because it will break compatibility.
And that is the first rule of device tree, never break backwards
compatibility.
> Also it seems wrong to write "marvell,mv88e6085" (only) if I know the
> hardware is really a "marvell,mv88e6176".
Why? Remember, the property name is called "compatible", not "is".
> > Linus has said he does not like ARM devices because of all the busses
> > which are not enumerable. Here we have a device which with a little
> > bit of help we can enumerate. So we should.
>
> If you write
>
> compatible = "marvell,mv88e6176", "marvell,mv88e6085";
>
> you can still enumerate in the same way as before.
Sure it would. But if the driver decides to ignore it, it is likely to
be wrong. Developers are used to comments being wrong. We are
suspicious of comments, we don't trust them 100%. But if somebody sees
a property in a device tree, they put a higher 'trustability' value on
it. But it actually has less trustable than a comment.
> There are several more instances where the device tree specifies
> something that could be probed instead. Some examples:
>
> compatible = "ethernet-phy-id0141.0DD1", "ethernet-phy-ieee802.3-c22";
There are examples of phys which don't implemented register 2 and
3. In that case, you do need to specify the ID, if you want the
correct driver to load.
> compatible = "spansion,s25fl164k", "jedec,spi-nor";
And there was a push recently to add "jedec,spi-nor" everywhere and
deprecate a specific compatible because it is also not needed.
> compatible = "fsl,imx25-flexcan", "fsl,p1010-flexcan";
> compatible = "arm,pl011", "arm,primecell";
I don't know these hardwares, so cannot comment.
Andrew
^ 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