* Re: [PATCH net-next v6 4/4] net: dp83869: Add RGMII internal delay configuration
From: Dan Murphy @ 2020-06-04 20:27 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andrew, f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
devicetree
In-Reply-To: <20200604094829.0d7d5df7@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Jakub
On 6/4/20 11:48 AM, Jakub Kicinski wrote:
> On Thu, 4 Jun 2020 11:38:14 -0500 Dan Murphy wrote:
>> Jakub
>>
>> On 6/4/20 11:25 AM, Jakub Kicinski wrote:
>>> On Thu, 4 Jun 2020 06:14:10 -0500 Dan Murphy wrote:
>>>> Add RGMII internal delay configuration for Rx and Tx.
>>>>
>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>> Hi Dan, please make sure W=1 C=1 build is clean:
>>>
>>> drivers/net/phy/dp83869.c:103:18: warning: ‘dp83869_internal_delay’ defined but not used [-Wunused-const-variable=]
>>> 103 | static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
>>> | ^~~~~~~~~~~~~~~~~~~~~~
>> I built with W=1 and C=1 and did not see this warning.
>>
>> What defconfig are you using?
> allmodconfig with gcc-10
>
>> Can you check if CONFIG_OF_MDIO is set or not? That would be the only
>> way that warning would come up.
> Hm. I don't have the config from this particular build but just running
> allmodconfig makes it CONFIG_OF_MDIO=m
OK that makes sense then. That is an existing bug that shows up because
of this.
#ifdef CONFIG_OF_MDIO
So the addition of the array exposed an existing issue.
That bug fix can go to net then.
>>> Also net-next is closed right now, you can post RFCs but normal patches
>>> should be deferred until after net-next reopens.
>> I know net-next is closed.
>>
>> I pinged David M when it was open about what is meant by "new" patches
>> in the net-dev FAQ. So I figured I would send the patches to see what
>> the response was.
>>
>> To me these are not new they are in process patches. My understand is
>> New is v1 patchesets.
>>
>> But now I have the answer.
> Oh sorry, I may be wrong in this case, I haven't tracked this series.
>
It says v6 in $subject.
But still you may be correct I don't know
Dan
^ permalink raw reply
* Re: [PATCH 3/3] spi: bcm2835: Enable shared interrupt support
From: Florian Fainelli @ 2020-06-04 20:24 UTC (permalink / raw)
To: Mark Brown
Cc: linux-kernel, Rob Herring, Nicolas Saenz Julienne, Ray Jui,
Scott Branden,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
open list:SPI SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl, lukas
In-Reply-To: <21772111-fa1f-7a50-aa92-e44b09cff4eb@gmail.com>
On 6/4/2020 9:05 AM, Florian Fainelli wrote:
>
>
> On 6/4/2020 5:32 AM, Mark Brown wrote:
>> On Wed, Jun 03, 2020 at 08:46:55PM -0700, Florian Fainelli wrote:
>>> The SPI controller found in the BCM2711 and BCM7211 SoCs is instantiated
>>> 5 times, with all instances sharing the same interrupt line. We
>>> specifically match the two compatible strings here to determine whether
>>> it is necessary to request the interrupt with the IRQF_SHARED flag and
>>> to use an appropriate interrupt handler capable of returning IRQ_NONE.
>>
>>> For the BCM2835 case which is deemed performance critical, there is no
>>> overhead since a dedicated handler that does not assume sharing is used.
>>
>> This feels hacky - it's essentially using the compatible string to set a
>> boolean flag which isn't really about the IP but rather the platform
>> integration. It might cause problems if we do end up having to quirk
>> this version of the IP for some other reason.
>
> I am not sure why it would be a problem, when you describe a piece of
> hardware with Device Tree, even with the IP block being strictly the
> same, its very integration into a new SoC (with details like shared
> interrupt lines) do warrant a different compatible string. Maybe this is
> more of a philosophical question.
>
>> I'm also looking at the
>> code and wondering if the overhead of checking to see if the interrupt
>> is flagged is really that severe, it's just a check to see if a bit is
>> set in a register which we already read so should be a couple of
>> instructions (which disassembly seems to confirm). It *is* overhead so
>> there's some value in it, I'm just surprised that it's such a hot path
>> especially with a reasonably deep FIFO like this device has.
>
> If it was up to me, we would just add the check on BCM2835_SPI_CS_INTR
> not being set and return IRQ_NONE and be done with it. I appreciate that
> Lukas has spent some tremendous amount of time working on this
> controller driver and he has a sensitivity for performance.
>
>>
>> I guess ideally genirq would provide a way to figure out if an interrupt
>> is actually shared in the present system, and better yet we'd have a way
>> for drivers to say they aren't using the interrupt ATM, but that might
>> be more effort than it's really worth. If this is needed and there's no
>> better way of figuring out if the interrupt is really shared then I'd
>> suggest a boolean flag rather than a compatible string, it's still a
>> hack but it's less likely to store up trouble for the future.
>
> Instead of counting the number of SPI devices we culd request the
> interrupt first with flags = IRQF_PROBE_SHARED, if this works, good we
> have a single SPI master enabled, if it returns -EBUSY, try again with
> flags = IRQF_SHARED and set-up the bcm2835_spi_sh_interrupt interrupt
> handler to manage the sharing.
>
> This would not require DT changes, which is probably better anyway.
Unfortunately this does not work.. The first time we probe the driver we
need to set an interrupt handler that is capable of handling a shared
interrupt. When we probe for subsequent times, we can use the -EBUSY
return code to know that we are in a shared interrupt context, however,
it is too late to change the first controller interrupt handler.
So we do need to know for the first time we install the interrupt
handler whether we will be in a shared situation or not, I cannot think
of any solution other than counting the number of available DT nodes
with the "brcm,bcm2835-spi" compatible string.
--
Florian
^ permalink raw reply
* Re: [PATCH v8 2/6] ARM: tegra: Add device-tree for ASUS Google Nexus 7
From: Dmitry Osipenko @ 2020-06-04 20:11 UTC (permalink / raw)
To: Michał Mirosław
Cc: Rob Herring, Thierry Reding, Jonathan Hunter, David Heidelberg,
Peter Geis, Stephen Warren, Nicolas Chauvet, Pedro Ângelo,
Matt Merhar, Zack Pearsall, linux-tegra, devicetree, linux-kernel
In-Reply-To: <cf73df1a-92ed-f509-74e8-1c847945fb14@gmail.com>
16.05.2020 15:01, Dmitry Osipenko пишет:
> 15.05.2020 21:18, Michał Mirosław пишет:
>> On Fri, May 15, 2020 at 12:36:50AM +0300, Dmitry Osipenko wrote:
>>> There are few hardware variants of NVIDIA Tegra30-based Nexus 7 device:
>>>
>>> 1. WiFi-only (named Grouper)
>>> 2. GSM (named Tilapia)
>>> 3. Using Maxim PMIC (E1565 board ID)
>>> 4. Using Ti PMIC (PM269 board ID)
>>
>> Hi,
>>
>> I've briefly looked at the PM269 devicetree (PMIC part) and it looks very
>> similar, if not the same, to what I deduced from the TF300T kernel.
>
> Hello Michał,
>
> Definitely there are board parts that are reused by different devices.
> This is not surprising since most of the boards are designed by the same
> company.
>
>> Those devices don't look to differ much from original Cardhu tablet
>> devkit, so maybe the trees can base off of that?
>
> I don't think it's really possible in a case of Nexus 7 because in
> overall the used hardware components differ a bit too much. It shouldn't
> worth the effort, IMO.
>
>> I would also guess that because of this 'ram-code', memory timings would
>> be duplicated between devices. I can see small differences between
>> ram-code=1 timings of Grouper and TF300T, though they look like arbiter
>> tuning differences. I'll have to test if my TF300T works with Grouper's
>> settings. In case they work, could you split the memory timings to another
>> dtsi file?
>
> Yes, perhaps this could be done. The memory timings on Grouper and
> Tilapia are pretty much the same as well. As you noticed, there are some
> tuning differences of TF300T in comparison to the Nexus 7, the same
> applies to the Grouper and Tilapia variants.
>
>> BTW, shouldn't EMC timing set match MC? I see more frequencies listed in
>> MC than EMC nodes.
>
> The MC timings are exactly the same on Grouper and Tilapia, but EMC
> timings have a very minor differences, and thus, the common.dtsi misses
> these differentiating EMC timings, they are defined in grouper.dtsi and
> tilapia.dts.
>
> I guess we indeed could try to select the lowest common denominator
> timing and re-use it. I'll consider this change for v9, thank you very
> much for the suggestion.
Hello Michał,
I tried to investigate whether we could unify the memory timings and
it's hard to tell. The values are mostly off by one in most cases for
one or two registers, in others cases a memory-chip feature is enabled
or disabled for the same memory module. It certainly feels like we could
safely ignore all those minor differences, but I don't have enough
expertise in order to decide firmly. In my opinion it should be better
to leave the memory timings as-is for the starter, we could always get
back to the unification later on.
^ permalink raw reply
* Re: [v2] drm/bridge: ti-sn65dsi86: ensure bridge suspend happens during PM sleep
From: Sam Ravnborg @ 2020-06-04 19:33 UTC (permalink / raw)
To: Harigovindan P
Cc: dri-devel, linux-arm-msm, freedreno, devicetree, linux-kernel,
robdclark, seanpaul, hoegsberg, kalyan_t, nganji
In-Reply-To: <20200604103438.23667-1-harigovi@codeaurora.org>
Hi Harigovindan
On Thu, Jun 04, 2020 at 04:04:38PM +0530, Harigovindan P wrote:
> ti-sn65dsi86 bridge is enumerated as a runtime device.
>
> Adding sleep ops to force runtime_suspend when PM suspend is
> requested on the device.
Patch looks correct - but could you please explain why it is needed.
What is the gain compared to not having this patch.
I ask for two reasons:
1) I really do not know
2) this info should be in the changelog
Without a better changelog no ack from me - sorry.
Sam
>
> Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
> ---
> Changes in v2:
> - Include bridge name in the commit message and
> remove dependent patchwork link from the commit
> text as bridge is independent of OEM(Stephen Boyd)
>
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 6ad688b320ae..2eef755b2917 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -159,6 +159,8 @@ static int __maybe_unused ti_sn_bridge_suspend(struct device *dev)
>
> static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
> SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> };
>
> static int status_show(struct seq_file *s, void *data)
> --
> 2.27.0
^ permalink raw reply
* Re: [PATCH V2 1/2] mmc: sdhci-msm: Add interconnect bandwidth scaling support
From: Sibi Sankar @ 2020-06-04 18:34 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Pradeep P V K, bjorn.andersson, adrian.hunter, robh+dt,
ulf.hansson, vbadigan, sboyd, georgi.djakov, linux-mmc,
linux-kernel, linux-arm-msm, devicetree, linux-mmc-owner, rnayak,
matthias
In-Reply-To: <20200604170906.GP4525@google.com>
On 2020-06-04 22:39, Matthias Kaehlcke wrote:
> On Thu, Jun 04, 2020 at 04:44:42PM +0530, Pradeep P V K wrote:
>> Interconnect bandwidth scaling support is now added as a
>> part of OPP [1]. So, make sure interconnect driver is ready
>> before handling interconnect scaling.
>>
>> This change is based on
>> [1] [Patch v8] Introduce OPP bandwidth bindings
>> (https://lkml.org/lkml/2020/5/12/493)
>>
>> [2] [Patch v3] mmc: sdhci-msm: Fix error handling
>> for dev_pm_opp_of_add_table()
>> (https://lkml.org/lkml/2020/5/5/491)
>>
>> Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
>> ---
>> drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c
>> b/drivers/mmc/host/sdhci-msm.c
>> index b277dd7..a13ff1b 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -14,6 +14,7 @@
>> #include <linux/slab.h>
>> #include <linux/iopoll.h>
>> #include <linux/regulator/consumer.h>
>> +#include <linux/interconnect.h>
>>
>> #include "sdhci-pltfm.h"
>> #include "cqhci.h"
>> @@ -2070,6 +2071,18 @@ static int sdhci_msm_probe(struct
>> platform_device *pdev)
>> }
>> msm_host->bulk_clks[0].clk = clk;
>>
>> + /* Make sure that ICC driver is ready for interconnect bandwdith
>> + * scaling before registering the device for OPP.
>> + */
>> + ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
>> + if (ret) {
>> + if (ret == -EPROBE_DEFER)
>> + dev_info(&pdev->dev, "defer icc path: %d\n", ret);
>
> I already commented on this on v1:
>
> This log seems to add little more than noise, or are there particular
> reasons
> why it is useful in this driver? Most drivers just return silently in
> case of
> deferred probing.
>
> If you think the log is really needed please explain why.
Both the err logs seem redundant.
EPROBE_DEFERS are rather readily
noticeable through the return val.
dev_.._find_icc_paths already prints
err messages when we fail to get icc
paths.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [V9, 2/2] media: i2c: ov02a10: Add OV02A10 image sensor driver
From: Tomasz Figa @ 2020-06-04 18:05 UTC (permalink / raw)
To: Sakari Ailus
Cc: Dongchun Zhu, Linus Walleij, Bartosz Golaszewski,
Mauro Carvalho Chehab, Andy Shevchenko, Rob Herring, Mark Rutland,
Nicolas Boichat, Matthias Brugger, Cao Bing Bu, srv_heupstream,
moderated list:ARM/Mediatek SoC support,
list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>,,
Sj Huang, Linux Media Mailing List, linux-devicetree, Louis Kuo,
Shengnan Wang (王圣男)
In-Reply-To: <20200604092616.GJ16711@paasikivi.fi.intel.com>
On Thu, Jun 4, 2020 at 11:26 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Dongchun,
>
> On Thu, Jun 04, 2020 at 10:14:05AM +0800, Dongchun Zhu wrote:
> > Hi Tomasz, Sakari, and sirs,
> >
> > Could anyone help to review this patch?
> >
> > On Sat, 2020-05-23 at 16:41 +0800, Dongchun Zhu wrote:
> > > Add a V4L2 sub-device driver for OV02A10 image sensor.
> > >
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > ---
> > > MAINTAINERS | 1 +
> > > drivers/media/i2c/Kconfig | 13 +
> > > drivers/media/i2c/Makefile | 1 +
> > > drivers/media/i2c/ov02a10.c | 1025 +++++++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 1040 insertions(+)
> > > create mode 100644 drivers/media/i2c/ov02a10.c
> > >
> >
> > [snip]
> >
> > > +static int ov02a10_probe(struct i2c_client *client)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct ov02a10 *ov02a10;
> > > + unsigned int rotation;
> > > + unsigned int clock_lane_tx_speed;
> > > + unsigned int i;
> > > + int ret;
> > > +
> > > + ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> > > + if (!ov02a10)
> > > + return -ENOMEM;
> > > +
> > > + ret = ov02a10_check_hwcfg(dev, ov02a10);
> > > + if (ret) {
> > > + dev_err(dev, "failed to check HW configuration: %d", ret);
> > > + return ret;
> > > + }
> > > +
> > > + v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> > > + ov02a10->mipi_clock_tx_speed = OV02A10_MIPI_TX_SPEED_DEFAULT;
> > > + ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> > > +
> > > + /* Optional indication of physical rotation of sensor */
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation", &rotation);
> > > + if (!ret && rotation == 180) {
> > > + ov02a10->upside_down = true;
> > > + ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> > > + }
> > > +
> > > + /* Optional indication of mipi TX speed */
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "ovti,mipi-tx-speed",
> > > + &clock_lane_tx_speed);
> > > +
> > > + if (!ret)
> > > + ov02a10->mipi_clock_tx_speed = clock_lane_tx_speed;
> > > +
> > > + /* Get system clock (eclk) */
> > > + ov02a10->eclk = devm_clk_get(dev, "eclk");
> > > + if (IS_ERR(ov02a10->eclk)) {
> > > + ret = PTR_ERR(ov02a10->eclk);
> > > + dev_err(dev, "failed to get eclk %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
> > > + &ov02a10->eclk_freq);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get eclk frequency\n");
> > > + return ret;
> > > + }
> > > +
> > > + ret = clk_set_rate(ov02a10->eclk, ov02a10->eclk_freq);
> > > + if (ret) {
> > > + dev_err(dev, "failed to set eclk frequency (24MHz)\n");
> > > + return ret;
> > > + }
> > > +
> > > + if (clk_get_rate(ov02a10->eclk) != OV02A10_ECLK_FREQ) {
> > > + dev_warn(dev, "wrong eclk frequency %d Hz, expected: %d Hz\n",
> > > + ov02a10->eclk_freq, OV02A10_ECLK_FREQ);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ov02a10->pd_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_HIGH);
> > > + if (IS_ERR(ov02a10->pd_gpio)) {
> > > + ret = PTR_ERR(ov02a10->pd_gpio);
> > > + dev_err(dev, "failed to get powerdown-gpios %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ov02a10->n_rst_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > + if (IS_ERR(ov02a10->n_rst_gpio)) {
> > > + ret = PTR_ERR(ov02a10->n_rst_gpio);
> > > + dev_err(dev, "failed to get reset-gpios %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(ov02a10_supply_names); i++)
> > > + ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> > > +
> > > + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ov02a10_supply_names),
> > > + ov02a10->supplies);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get regulators\n");
> > > + return ret;
> > > + }
> > > +
> > > + mutex_init(&ov02a10->mutex);
> > > + ov02a10->cur_mode = &supported_modes[0];
> > > + ret = ov02a10_initialize_controls(ov02a10);
> > > + if (ret) {
> > > + dev_err(dev, "failed to initialize controls\n");
> > > + goto err_destroy_mutex;
> > > + }
> > > +
> > > + ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > + ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> > > + ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > + ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> > > + ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to init entity pads: %d", ret);
> > > + goto err_free_handler;
> > > + }
> > > +
> > > + pm_runtime_enable(dev);
> > > + if (!pm_runtime_enabled(dev)) {
> > > + ret = ov02a10_power_on(dev);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to power on: %d\n", ret);
> > > + goto err_free_handler;
> > > + }
> > > + }
> > > +
> > > + ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > > + if (ret) {
> > > + dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > > + if (!pm_runtime_enabled(dev))
> > > + ov02a10_power_off(dev);
>
> This should be moved to error handling section below.
>
> > > + goto err_clean_entity;
> > > + }
> >
> > Tomasz, Sakari, is this ok?
> > or coding like this:
> >
> > ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > if (!pm_runtime_enabled(dev))
> > ov02a10_power_off(dev);
> > if (ret) {
> > dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > goto err_clean_entity;
> > }
> >
> > What's your opinions about the change?
>
> This turns power off if runtime PM is disabled. I'd keep it as-is, as it'd
> require re-implementing what runtime PM is used for now --- and that's not
> a sensor driver's job.
That and in general I believe the expectations are:
- runtime PM enabled - powered on only when it has something to do
- runtime PM disabled - powered on when the driver is bound (probe
succeeded), powered off when the driver unbinds (remove or probe
error)
Best regards,
Tomasz
>
> >
> > > +
> > > + return 0;
> > > +
> > > +err_clean_entity:
> > > + media_entity_cleanup(&ov02a10->subdev.entity);
> > > +err_free_handler:
> > > + v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> > > +err_destroy_mutex:
> > > + mutex_destroy(&ov02a10->mutex);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int ov02a10_remove(struct i2c_client *client)
> > > +{
> > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > > +
> > > + v4l2_async_unregister_subdev(sd);
> > > + media_entity_cleanup(&sd->entity);
> > > + v4l2_ctrl_handler_free(sd->ctrl_handler);
> > > + pm_runtime_disable(&client->dev);
> > > + if (!pm_runtime_status_suspended(&client->dev))
> > > + ov02a10_power_off(&client->dev);
> > > + pm_runtime_set_suspended(&client->dev);
> > > + mutex_destroy(&ov02a10->mutex);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id ov02a10_of_match[] = {
> > > + { .compatible = "ovti,ov02a10" },
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ov02a10_of_match);
> > > +
> > > +static struct i2c_driver ov02a10_i2c_driver = {
> > > + .driver = {
> > > + .name = "ov02a10",
> > > + .pm = &ov02a10_pm_ops,
> > > + .of_match_table = ov02a10_of_match,
> > > + },
> > > + .probe_new = &ov02a10_probe,
> > > + .remove = &ov02a10_remove,
> > > +};
> > > +
> > > +module_i2c_driver(ov02a10_i2c_driver);
> > > +
> > > +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> > > +MODULE_DESCRIPTION("OmniVision OV02A10 sensor driver");
> > > +MODULE_LICENSE("GPL v2");
> > > +
> >
>
> --
> Sakari Ailus
^ permalink raw reply
* Re: [PATCH V1 1/2] mmc: sdhci-msm: Add interconnect bandwidth scaling support
From: Sibi Sankar @ 2020-06-04 17:55 UTC (permalink / raw)
To: ppvk
Cc: bjorn.andersson, adrian.hunter, robh+dt, ulf.hansson, vbadigan,
sboyd, georgi.djakov, mka, linux-mmc, linux-kernel, linux-arm-msm,
devicetree, linux-mmc-owner, rnayak, matthias,
linux-arm-msm-owner, linux-kernel-owner
In-Reply-To: <8865e3b00fce4f28264b60cd498fcf02@codeaurora.org>
On 2020-06-04 16:43, ppvk@codeaurora.org wrote:
> Hi Sibi,
>
> Thanks for the review!!
>
> On 2020-06-03 17:22, Sibi Sankar wrote:
>> Hey Pradeep,
>> Thanks for the patch.
>>
>> On 2020-06-03 14:39, Pradeep P V K wrote:
>>> Interconnect bandwidth scaling support is now added as a
>>> part of OPP [1]. So, make sure interconnect driver is ready
>>> before handling interconnect scaling.
>>>
>>> This change is based on
>>> [1] [Patch v8] Introduce OPP bandwidth bindings
>>> (https://lkml.org/lkml/2020/5/12/493)
>>>
>>> [2] [Patch v3] mmc: sdhci-msm: Fix error handling
>>> for dev_pm_opp_of_add_table()
>>> (https://lkml.org/lkml/2020/5/5/491)
>>>
>>> Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
>>> ---
>>> drivers/mmc/host/sdhci-msm.c | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-msm.c
>>> b/drivers/mmc/host/sdhci-msm.c
>>> index b277dd7..bf95484 100644
>>> --- a/drivers/mmc/host/sdhci-msm.c
>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>> @@ -14,6 +14,7 @@
>>> #include <linux/slab.h>
>>> #include <linux/iopoll.h>
>>> #include <linux/regulator/consumer.h>
>>> +#include <linux/interconnect.h>
>>>
>>> #include "sdhci-pltfm.h"
>>> #include "cqhci.h"
>>> @@ -1999,6 +2000,7 @@ static int sdhci_msm_probe(struct
>>> platform_device *pdev)
>>> struct sdhci_pltfm_host *pltfm_host;
>>> struct sdhci_msm_host *msm_host;
>>> struct clk *clk;
>>> + struct icc_path *sdhc_path;
>>> int ret;
>>> u16 host_version, core_minor;
>>> u32 core_version, config;
>>> @@ -2070,6 +2072,20 @@ static int sdhci_msm_probe(struct
>>> platform_device *pdev)
>>> }
>>> msm_host->bulk_clks[0].clk = clk;
>>>
>>> + /* Make sure that ICC driver is ready for interconnect bandwdith
>>> + * scaling before registering the device for OPP.
>>> + */
>>> + sdhc_path = of_icc_get(&pdev->dev, NULL);
>>> + ret = PTR_ERR_OR_ZERO(sdhc_path);
>>> + if (ret) {
>>> + if (ret == -EPROBE_DEFER)
>>> + dev_info(&pdev->dev, "defer icc path: %d\n", ret);
>>> + else
>>> + dev_err(&pdev->dev, "failed to get icc path:%d\n", ret);
>>> + goto bus_clk_disable;
>>> + }
>>> + icc_put(sdhc_path);
>>
>> ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
>>
>> since there are multiple paths
>> described in the bindings you
>> should use ^^ instead and you
>> can drop temporary path as well.
>>
> Ok. of_icc_get() used above is only to test if ICC driver is ready or
> not. I'm not
> really using the multiple paths here. Anyhow i will use
> dev_pm_opp_of_find_icc_paths()
> to get rid of some extra lines of code.
Using dev_pm_opp_of_find_icc_paths
with NULL passed acts as a way of
validating all the paths specified
in the device and also validates if
the opp-table has bw related bindings
as well.
>
>>> +
>>> msm_host->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "core");
>>> if (IS_ERR(msm_host->opp_table)) {
>>> ret = PTR_ERR(msm_host->opp_table);
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH v3 004/105] clk: bcm: Add BCM2711 DVP driver
From: Nicolas Saenz Julienne @ 2020-06-04 17:26 UTC (permalink / raw)
To: Maxime Ripard, Eric Anholt
Cc: dri-devel, linux-rpi-kernel, bcm-kernel-feedback-list,
linux-arm-kernel, linux-kernel, Dave Stevenson, Tim Gover,
Phil Elwell, Michael Turquette, Stephen Boyd, Rob Herring,
linux-clk, devicetree
In-Reply-To: <6615a61b8af240e3d10f8890e4b2462ccdaac9b9.1590594512.git-series.maxime@cerno.tech>
[-- Attachment #1: Type: text/plain, Size: 6101 bytes --]
Hi Maxime,
On Wed, 2020-05-27 at 17:47 +0200, Maxime Ripard wrote:
> The HDMI block has a block that controls clocks and reset signals to the
> HDMI0 and HDMI1 controllers.
Why not having two separate drivers?
> Let's expose that through a clock driver implementing a clock and reset
> provider.
>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-clk@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
> drivers/clk/bcm/Kconfig | 11 +++-
> drivers/clk/bcm/Makefile | 1 +-
> drivers/clk/bcm/clk-bcm2711-dvp.c | 127 +++++++++++++++++++++++++++++++-
> 3 files changed, 139 insertions(+)
> create mode 100644 drivers/clk/bcm/clk-bcm2711-dvp.c
>
> diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
> index 8c83977a7dc4..784f12c72365 100644
> --- a/drivers/clk/bcm/Kconfig
> +++ b/drivers/clk/bcm/Kconfig
> @@ -1,4 +1,15 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +
> +config CLK_BCM2711_DVP
> + tristate "Broadcom BCM2711 DVP support"
> + depends on ARCH_BCM2835 ||COMPILE_TEST
> + depends on COMMON_CLK
> + default ARCH_BCM2835
> + select RESET_SIMPLE
> + help
> + Enable common clock framework support for the Broadcom BCM2711
> + DVP Controller.
> +
> config CLK_BCM2835
> bool "Broadcom BCM2835 clock support"
> depends on ARCH_BCM2835 || ARCH_BRCMSTB || COMPILE_TEST
> diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
> index 0070ddf6cdd2..2c1349062147 100644
> --- a/drivers/clk/bcm/Makefile
> +++ b/drivers/clk/bcm/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_CLK_BCM_KONA) += clk-kona-setup.o
> obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm281xx.o
> obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm21664.o
> obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o
> clk-iproc-asiu.o
> +obj-$(CONFIG_CLK_BCM2835) += clk-bcm2711-dvp.o
> obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835.o
> obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835-aux.o
> obj-$(CONFIG_CLK_RASPBERRYPI) += clk-raspberrypi.o
> diff --git a/drivers/clk/bcm/clk-bcm2711-dvp.c b/drivers/clk/bcm/clk-bcm2711-
> dvp.c
> new file mode 100644
> index 000000000000..c1c4b5857d32
> --- /dev/null
> +++ b/drivers/clk/bcm/clk-bcm2711-dvp.c
> @@ -0,0 +1,127 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// Copyright 2020 Cerno
> +
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/reset/reset-simple.h>
> +
> +#define DVP_HT_RPI_SW_INIT 0x04
> +#define DVP_HT_RPI_MISC_CONFIG 0x08
> +
> +#define NR_CLOCKS 2
> +#define NR_RESETS 6
> +
> +struct clk_dvp {
> + struct clk_hw_onecell_data *data;
> + struct reset_simple_data reset;
> +};
> +
> +static const struct clk_parent_data clk_dvp_parent = {
> + .index = 0,
> +};
> +
> +static int clk_dvp_probe(struct platform_device *pdev)
> +{
> + struct clk_hw_onecell_data *data;
> + struct resource *res;
> + struct clk_dvp *dvp;
> + void __iomem *base;
> + int ret;
> +
> + dvp = devm_kzalloc(&pdev->dev, sizeof(*dvp), GFP_KERNEL);
> + if (!dvp)
> + return -ENOMEM;
> + platform_set_drvdata(pdev, dvp);
> +
> + dvp->data = devm_kzalloc(&pdev->dev,
> + struct_size(dvp->data, hws, NR_CLOCKS),
> + GFP_KERNEL);
> + if (!dvp->data)
> + return -ENOMEM;
> + data = dvp->data;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
I think the cool function to use these days is
devm_platform_get_and_ioremap_resource().
Regards,
Nicolas
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + dvp->reset.rcdev.owner = THIS_MODULE;
> + dvp->reset.rcdev.nr_resets = NR_RESETS;
> + dvp->reset.rcdev.ops = &reset_simple_ops;
> + dvp->reset.rcdev.of_node = pdev->dev.of_node;
> + dvp->reset.membase = base + DVP_HT_RPI_SW_INIT;
> + spin_lock_init(&dvp->reset.lock);
> +
> + ret = reset_controller_register(&dvp->reset.rcdev);
> + if (ret)
> + return ret;
> +
> + data->hws[0] = clk_hw_register_gate_parent_data(&pdev->dev,
> + "hdmi0-108MHz",
> + &clk_dvp_parent, 0,
> + base +
> DVP_HT_RPI_MISC_CONFIG, 3,
> + CLK_GATE_SET_TO_DISABLE,
> + &dvp->reset.lock);
> + if (IS_ERR(data->hws[0])) {
> + ret = PTR_ERR(data->hws[0]);
> + goto unregister_reset;
> + }
> +
> + data->hws[1] = clk_hw_register_gate_parent_data(&pdev->dev,
> + "hdmi1-108MHz",
> + &clk_dvp_parent, 0,
> + base +
> DVP_HT_RPI_MISC_CONFIG, 4,
> + CLK_GATE_SET_TO_DISABLE,
> + &dvp->reset.lock);
> + if (IS_ERR(data->hws[1])) {
> + ret = PTR_ERR(data->hws[1]);
> + goto unregister_clk0;
> + }
> +
> + data->num = NR_CLOCKS;
> + ret = of_clk_add_hw_provider(pdev->dev.of_node, of_clk_hw_onecell_get,
> + data);
> + if (ret)
> + goto unregister_clk1;
> +
> + return 0;
> +
> +unregister_clk1:
> + clk_hw_unregister_gate(data->hws[1]);
> +
> +unregister_clk0:
> + clk_hw_unregister_gate(data->hws[0]);
> +
> +unregister_reset:
> + reset_controller_unregister(&dvp->reset.rcdev);
> + return ret;
> +};
> +
> +static int clk_dvp_remove(struct platform_device *pdev)
> +{
> + struct clk_dvp *dvp = platform_get_drvdata(pdev);
> + struct clk_hw_onecell_data *data = dvp->data;
> +
> + clk_hw_unregister_gate(data->hws[1]);
> + clk_hw_unregister_gate(data->hws[0]);
> + reset_controller_unregister(&dvp->reset.rcdev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id clk_dvp_dt_ids[] = {
> + { .compatible = "brcm,brcm2711-dvp", },
> + { /* sentinel */ }
> +};
> +
> +static struct platform_driver clk_dvp_driver = {
> + .probe = clk_dvp_probe,
> + .remove = clk_dvp_remove,
> + .driver = {
> + .name = "brcm2711-dvp",
> + .of_match_table = clk_dvp_dt_ids,
> + },
> +};
> +module_platform_driver(clk_dvp_driver);
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH V2 1/2] mmc: sdhci-msm: Add interconnect bandwidth scaling support
From: Matthias Kaehlcke @ 2020-06-04 17:09 UTC (permalink / raw)
To: Pradeep P V K
Cc: bjorn.andersson, adrian.hunter, robh+dt, ulf.hansson, vbadigan,
sboyd, georgi.djakov, linux-mmc, linux-kernel, linux-arm-msm,
devicetree, linux-mmc-owner, rnayak, sibis, matthias
In-Reply-To: <1591269283-24084-2-git-send-email-ppvk@codeaurora.org>
On Thu, Jun 04, 2020 at 04:44:42PM +0530, Pradeep P V K wrote:
> Interconnect bandwidth scaling support is now added as a
> part of OPP [1]. So, make sure interconnect driver is ready
> before handling interconnect scaling.
>
> This change is based on
> [1] [Patch v8] Introduce OPP bandwidth bindings
> (https://lkml.org/lkml/2020/5/12/493)
>
> [2] [Patch v3] mmc: sdhci-msm: Fix error handling
> for dev_pm_opp_of_add_table()
> (https://lkml.org/lkml/2020/5/5/491)
>
> Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
> ---
> drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index b277dd7..a13ff1b 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -14,6 +14,7 @@
> #include <linux/slab.h>
> #include <linux/iopoll.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/interconnect.h>
>
> #include "sdhci-pltfm.h"
> #include "cqhci.h"
> @@ -2070,6 +2071,18 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> }
> msm_host->bulk_clks[0].clk = clk;
>
> + /* Make sure that ICC driver is ready for interconnect bandwdith
> + * scaling before registering the device for OPP.
> + */
> + ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
> + if (ret) {
> + if (ret == -EPROBE_DEFER)
> + dev_info(&pdev->dev, "defer icc path: %d\n", ret);
I already commented on this on v1:
This log seems to add little more than noise, or are there particular reasons
why it is useful in this driver? Most drivers just return silently in case of
deferred probing.
If you think the log is really needed please explain why.
^ permalink raw reply
* Re: [PATCH v2 0/3] media: rockchip: Introduce driver for the camera interface on PX30
From: Tomasz Figa @ 2020-06-04 17:04 UTC (permalink / raw)
To: Heiko Stübner
Cc: Maxime Chevallier, Helen Koike, Dafna Hirschfeld,
Mauro Carvalho Chehab, Robin Murphy, Rob Herring, Mark Rutland,
Hans Verkuil, Linux Media Mailing List, linux-devicetree
In-Reply-To: <1779471.kMuJgyiE6z@diego>
On Mon, Jun 1, 2020 at 11:38 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi Tomasz,
>
> Am Montag, 1. Juni 2020, 20:45:14 CEST schrieb Tomasz Figa:
> > On Fri, May 29, 2020 at 3:04 PM Maxime Chevallier
> > <maxime.chevallier@bootlin.com> wrote:
> > >
> > > Hello everyone,
> > >
> > > Here's a V2 of the series adding very basic support for the camera interface on
> > > the Rockchip PX30 SoC.
> > >
> > > Thanks to everyone that commented on the first series, your reviews were
> > > very helpful :)
> > >
> > > This Camera Interface is also supported on other Rockchip SoC such as
> > > the RK1808, RK3128, RK3288 and RK3288, but for now I've only been able to
> > > test it on the PX30, using a PAL format.
> >
> > How does this hardware relate to the one handled by the rkisp1 driver
> > that is available under staging/media/rkisp1? It was written with
> > RK3399 in mind, but I have a loose recollection that the hardware in
> > RK3288 was roughly the same.
>
> (un-)educated guess would be that the rk3288 has both.
>
> When introducing new IPs Rockchip often keeps the previous incarnation
> around - probably as a fallback.
>
> From a bit of digging around manuals and vendor-dtsi [0] I found:
>
> in rk3288.dtsi both:
> - isp: isp@ff910000
> - cif_isp0: cif_isp@ff910000
>
> - grf_con_disable_isp in GRF_SOC_CON6
> - dphy_rx1_src_sel (1: isp, 0: csi host) in GRF_SOC_CON14
>
Makes sense. Thanks!
Right now the rkisp1 driver doesn't support rk3288, because we didn't
have any way to test it, but it shouldn't require much changes to do
so. If that happens, I wonder how one would select between the two
hardware blocks?
Best regards,
Tomasz
>
> Heiko
>
>
> [0] https://github.com/rockchip-linux/kernel/blob/develop-4.4/arch/arm/boot/dts/rk3288.dtsi
>
>
> > +Helen Koike +Dafna Hirschfeld working on the rkisp1 driver.
> >
> > Best regards,
> > Tomasz
> >
> > >
> > > This driver is mostly based on the driver found in Rockchip's BSP, that
> > > has been trimmed down to support the set of features that I was able to test,
> > > that is pretty much a very basic one-frame capture and video streaming
> > > with GStreamer.
> > >
> > > This first draft only supports the Parallel interface, although the
> > > controller has support for BT656 and CSI2.
> > >
> > > Finally, this controller has an iommu that could be used in this driver,
> > > but as of today I've not been able to get it to work.
> > >
> > > Any review is welcome.
> > >
> > > Thanks,
> > >
> > > Maxime
> > >
> > > --- Changes since V1 ---
> > >
> > > - Took reviews from Rob, Hans, Robin and Heiko into account :
> > > - Renamed the clocks in the binding
> > > - Fixed the DT schema compiling
> > > - Fixed a few typos
> > > - Used the clk bulk API
> > > - Used the reset array API
> > > - Changed a few helpers for more suitable ones
> > > - Rebased on 5.7-rc7
> > >
> > >
> > >
> > > Maxime Chevallier (3):
> > > media: dt-bindings: media: Document Rockchip CIF bindings
> > > media: rockchip: Introduce driver for Rockhip's camera interface
> > > arm64: dts: rockchip: Add the camera interface description of the PX30
> > >
> > > .../bindings/media/rockchip-cif.yaml | 100 ++
> > > arch/arm64/boot/dts/rockchip/px30.dtsi | 12 +
> > > drivers/media/platform/Kconfig | 13 +
> > > drivers/media/platform/Makefile | 1 +
> > > drivers/media/platform/rockchip/cif/Makefile | 3 +
> > > drivers/media/platform/rockchip/cif/capture.c | 1170 +++++++++++++++++
> > > drivers/media/platform/rockchip/cif/dev.c | 358 +++++
> > > drivers/media/platform/rockchip/cif/dev.h | 213 +++
> > > drivers/media/platform/rockchip/cif/regs.h | 256 ++++
> > > 9 files changed, 2126 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/media/rockchip-cif.yaml
> > > create mode 100644 drivers/media/platform/rockchip/cif/Makefile
> > > create mode 100644 drivers/media/platform/rockchip/cif/capture.c
> > > create mode 100644 drivers/media/platform/rockchip/cif/dev.c
> > > create mode 100644 drivers/media/platform/rockchip/cif/dev.h
> > > create mode 100644 drivers/media/platform/rockchip/cif/regs.h
> > >
> > > --
> > > 2.25.4
> > >
> >
>
>
>
>
^ permalink raw reply
* Re: [PATCH v8 0/5] support reserving crashkernel above 4G on arm64 kdump
From: Nicolas Saenz Julienne @ 2020-06-04 17:01 UTC (permalink / raw)
To: Bhupesh Sharma, John Donnelly
Cc: chenzhou, Simon Horman, Devicetree List, Arnd Bergmann,
Baoquan He, Linux Doc Mailing List, Catalin Marinas, guohanjun,
kexec mailing list, Linux Kernel Mailing List, Will Deacon,
Rob Herring, James Morse, Prabhakar Kushwaha, Thomas Gleixner,
Prabhakar Kushwaha, RuiRui Yang, Ingo Molnar, linux-arm-kernel
In-Reply-To: <CACi5LpN-+NRnaDoWWWidbzma8BNzmofA5FQBV=cPF1Mc84FpFg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 16696 bytes --]
On Thu, 2020-06-04 at 01:17 +0530, Bhupesh Sharma wrote:
> Hi All,
>
> On Wed, Jun 3, 2020 at 9:03 PM John Donnelly <john.p.donnelly@oracle.com>
> wrote:
> >
> >
> > > On Jun 3, 2020, at 8:20 AM, chenzhou <chenzhou10@huawei.com> wrote:
> > >
> > > Hi,
> > >
> > >
> > > On 2020/6/3 19:47, Prabhakar Kushwaha wrote:
> > > > Hi Chen,
> > > >
> > > > On Tue, Jun 2, 2020 at 8:12 PM John Donnelly <john.p.donnelly@oracle.com
> > > > > wrote:
> > > > >
> > > > > > On Jun 2, 2020, at 12:38 AM, Prabhakar Kushwaha <
> > > > > > prabhakar.pkin@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, Jun 2, 2020 at 3:29 AM John Donnelly <
> > > > > > john.p.donnelly@oracle.com> wrote:
> > > > > > > Hi . See below !
> > > > > > >
> > > > > > > > On Jun 1, 2020, at 4:02 PM, Bhupesh Sharma <bhsharma@redhat.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi John,
> > > > > > > >
> > > > > > > > On Tue, Jun 2, 2020 at 1:01 AM John Donnelly <
> > > > > > > > John.P.donnelly@oracle.com> wrote:
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On 6/1/20 7:02 AM, Prabhakar Kushwaha wrote:
> > > > > > > > > > Hi Chen,
> > > > > > > > > >
> > > > > > > > > > On Thu, May 21, 2020 at 3:05 PM Chen Zhou <
> > > > > > > > > > chenzhou10@huawei.com> wrote:
> > > > > > > > > > > This patch series enable reserving crashkernel above 4G in
> > > > > > > > > > > arm64.
> > > > > > > > > > >
> > > > > > > > > > > There are following issues in arm64 kdump:
> > > > > > > > > > > 1. We use crashkernel=X to reserve crashkernel below 4G,
> > > > > > > > > > > which will fail
> > > > > > > > > > > when there is no enough low memory.
> > > > > > > > > > > 2. Currently, crashkernel=Y@X can be used to reserve
> > > > > > > > > > > crashkernel above 4G,
> > > > > > > > > > > in this case, if swiotlb or DMA buffers are required,
> > > > > > > > > > > crash dump kernel
> > > > > > > > > > > will boot failure because there is no low memory available
> > > > > > > > > > > for allocation.
> > > > > > > > > > >
> > > > > > > > > > We are getting "warn_alloc" [1] warning during boot of kdump
> > > > > > > > > > kernel
> > > > > > > > > > with bootargs as [2] of primary kernel.
> > > > > > > > > > This error observed on ThunderX2 ARM64 platform.
> > > > > > > > > >
> > > > > > > > > > It is observed with latest upstream tag (v5.7-rc3) with this
> > > > > > > > > > patch set
> > > > > > > > > > and
> > > > > > > > > >
https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbiIAAlzu$
> > > > > > > > > > Also **without** this patch-set
> > > > > > > > > > "
> > > > > > > > > >
https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$
> > > > > > > > > > "
> > > > > > > > > >
> > > > > > > > > > This issue comes whenever crashkernel memory is reserved
> > > > > > > > > > after 0xc000_0000.
> > > > > > > > > > More details discussed earlier in
> > > > > > > > > >
https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$
without
> > > > > > > > > > any
> > > > > > > > > > solution
> > > > > > > > > >
> > > > > > > > > > This patch-set is expected to solve similar kind of issue.
> > > > > > > > > > i.e. low memory is only targeted for DMA, swiotlb; So above
> > > > > > > > > > mentioned
> > > > > > > > > > observation should be considered/fixed. .
> > > > > > > > > >
> > > > > > > > > > --pk
> > > > > > > > > >
> > > > > > > > > > [1]
> > > > > > > > > > [ 30.366695] DMI: Cavium Inc. Saber/Saber, BIOS
> > > > > > > > > > TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
> > > > > > > > > > [ 30.367696] NET: Registered protocol family 16
> > > > > > > > > > [ 30.369973] swapper/0: page allocation failure: order:6,
> > > > > > > > > > mode:0x1(GFP_DMA), nodemask=(null),cpuset=/,mems_allowed=0
> > > > > > > > > > [ 30.369980] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> > > > > > > > > > 5.7.0-rc3+ #121
> > > > > > > > > > [ 30.369981] Hardware name: Cavium Inc. Saber/Saber, BIOS
> > > > > > > > > > TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
> > > > > > > > > > [ 30.369984] Call trace:
> > > > > > > > > > [ 30.369989] dump_backtrace+0x0/0x1f8
> > > > > > > > > > [ 30.369991] show_stack+0x20/0x30
> > > > > > > > > > [ 30.369997] dump_stack+0xc0/0x10c
> > > > > > > > > > [ 30.370001] warn_alloc+0x10c/0x178
> > > > > > > > > > [ 30.370004] __alloc_pages_slowpath.constprop.111+0xb10/0
> > > > > > > > > > xb50
> > > > > > > > > > [ 30.370006] __alloc_pages_nodemask+0x2b4/0x300
> > > > > > > > > > [ 30.370008] alloc_page_interleave+0x24/0x98
> > > > > > > > > > [ 30.370011] alloc_pages_current+0xe4/0x108
> > > > > > > > > > [ 30.370017] dma_atomic_pool_init+0x44/0x1a4
> > > > > > > > > > [ 30.370020] do_one_initcall+0x54/0x228
> > > > > > > > > > [ 30.370027] kernel_init_freeable+0x228/0x2cc
> > > > > > > > > > [ 30.370031] kernel_init+0x1c/0x110
> > > > > > > > > > [ 30.370034] ret_from_fork+0x10/0x18
> > > > > > > > > > [ 30.370036] Mem-Info:
> > > > > > > > > > [ 30.370064] active_anon:0 inactive_anon:0 isolated_anon:0
> > > > > > > > > > [ 30.370064] active_file:0 inactive_file:0
> > > > > > > > > > isolated_file:0
> > > > > > > > > > [ 30.370064] unevictable:0 dirty:0 writeback:0 unstable:0
> > > > > > > > > > [ 30.370064] slab_reclaimable:34 slab_unreclaimable:4438
> > > > > > > > > > [ 30.370064] mapped:0 shmem:0 pagetables:14 bounce:0
> > > > > > > > > > [ 30.370064] free:1537719 free_pcp:219 free_cma:0
> > > > > > > > > > [ 30.370070] Node 0 active_anon:0kB inactive_anon:0kB
> > > > > > > > > > active_file:0kB inactive_file:0kB unevictable:0kB
> > > > > > > > > > isolated(anon):0kB
> > > > > > > > > > isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB
> > > > > > > > > > shmem:0kB
> > > > > > > > > > shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB
> > > > > > > > > > writeback_tmp:0kB
> > > > > > > > > > unstable:0kB all_unreclaimable? no
> > > > > > > > > > [ 30.370073] Node 1 active_anon:0kB inactive_anon:0kB
> > > > > > > > > > active_file:0kB inactive_file:0kB unevictable:0kB
> > > > > > > > > > isolated(anon):0kB
> > > > > > > > > > isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB
> > > > > > > > > > shmem:0kB
> > > > > > > > > > shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB
> > > > > > > > > > writeback_tmp:0kB
> > > > > > > > > > unstable:0kB all_unreclaimable? no
> > > > > > > > > > [ 30.370079] Node 0 DMA free:0kB min:0kB low:0kB high:0kB
> > > > > > > > > > reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
> > > > > > > > > > active_file:0kB inactive_file:0kB unevictable:0kB
> > > > > > > > > > writepending:0kB
> > > > > > > > > > present:128kB managed:0kB mlocked:0kB kernel_stack:0kB
> > > > > > > > > > pagetables:0kB
> > > > > > > > > > bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
> > > > > > > > > > [ 30.370084] lowmem_reserve[]: 0 250 6063 6063
> > > > > > > > > > [ 30.370090] Node 0 DMA32 free:256000kB min:408kB
> > > > > > > > > > low:664kB
> > > > > > > > > > high:920kB reserved_highatomic:0KB active_anon:0kB
> > > > > > > > > > inactive_anon:0kB
> > > > > > > > > > active_file:0kB inactive_file:0kB unevictable:0kB
> > > > > > > > > > writepending:0kB
> > > > > > > > > > present:269700kB managed:256000kB mlocked:0kB
> > > > > > > > > > kernel_stack:0kB
> > > > > > > > > > pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB
> > > > > > > > > > free_cma:0kB
> > > > > > > > > > [ 30.370094] lowmem_reserve[]: 0 0 5813 5813
> > > > > > > > > > [ 30.370100] Node 0 Normal free:5894876kB min:9552kB
> > > > > > > > > > low:15504kB
> > > > > > > > > > high:21456kB reserved_highatomic:0KB active_anon:0kB
> > > > > > > > > > inactive_anon:0kB
> > > > > > > > > > active_file:0kB inactive_file:0kB unevictable:0kB
> > > > > > > > > > writepending:0kB
> > > > > > > > > > present:8388608kB managed:5953112kB mlocked:0kB
> > > > > > > > > > kernel_stack:21672kB
> > > > > > > > > > pagetables:56kB bounce:0kB free_pcp:876kB local_pcp:176kB
> > > > > > > > > > free_cma:0kB
> > > > > > > > > > [ 30.370104] lowmem_reserve[]: 0 0 0 0
> > > > > > > > > > [ 30.370107] Node 0 DMA: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB
> > > > > > > > > > 0*128kB
> > > > > > > > > > 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 0kB
> > > > > > > > > > [ 30.370113] Node 0 DMA32: 0*4kB 0*8kB 0*16kB 0*32kB
> > > > > > > > > > 0*64kB 0*128kB
> > > > > > > > > > 0*256kB 0*512kB 0*1024kB 1*2048kB (M) 62*4096kB (M) =
> > > > > > > > > > 256000kB
> > > > > > > > > > [ 30.370119] Node 0 Normal: 2*4kB (M) 3*8kB (ME) 2*16kB
> > > > > > > > > > (UE) 3*32kB
> > > > > > > > > > (UM) 1*64kB (U) 2*128kB (M) 2*256kB (ME) 3*512kB (ME)
> > > > > > > > > > 3*1024kB (ME)
> > > > > > > > > > 3*2048kB (UME) 1436*4096kB (M) = 5893600kB
> > > > > > > > > > [ 30.370129] Node 0 hugepages_total=0 hugepages_free=0
> > > > > > > > > > hugepages_surp=0 hugepages_size=1048576kB
> > > > > > > > > > [ 30.370130] 0 total pagecache pages
> > > > > > > > > > [ 30.370132] 0 pages in swap cache
> > > > > > > > > > [ 30.370134] Swap cache stats: add 0, delete 0, find 0/0
> > > > > > > > > > [ 30.370135] Free swap = 0kB
> > > > > > > > > > [ 30.370136] Total swap = 0kB
> > > > > > > > > > [ 30.370137] 2164609 pages RAM
> > > > > > > > > > [ 30.370139] 0 pages HighMem/MovableOnly
> > > > > > > > > > [ 30.370140] 612331 pages reserved
> > > > > > > > > > [ 30.370141] 0 pages hwpoisoned
> > > > > > > > > > [ 30.370143] DMA: failed to allocate 256 KiB pool for
> > > > > > > > > > atomic
> > > > > > > > > > coherent allocation
> > > > > > > > >
> > > > > > > > > During my testing I saw the same error and Chen's solution
> > > > > > > > > corrected it .
> > > > > > > > Which combination you are using on your side? I am using
> > > > > > > > Prabhakar's
> > > > > > > > suggested environment and can reproduce the issue
> > > > > > > > with or without Chen's crashkernel support above 4G patchset.
> > > > > > > >
> > > > > > > > I am also using a ThunderX2 platform with latest makedumpfile
> > > > > > > > code and
> > > > > > > > kexec-tools (with the suggested patch
> > > > > > > > <
> > > > > > > >
https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!J6lUig58-Gw6TKZnEEYzEeSU36T-1SqlB1kImU00xtX_lss5Tx-JbUmLE9TJC3foXBLg$
> > > > > > > > >).
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Bhupesh
> > > > > > >
> > > > > > > I did this activity 5 months ago and I have moved on to other
> > > > > > > activities. My DMA failures were related to PCI devices that could
> > > > > > > not be enumerated because low-DMA space was not available when
> > > > > > > crashkernel was moved above 4G; I don’t recall the exact platform.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > For this failure ,
> > > > > > >
> > > > > > > > > > DMA: failed to allocate 256 KiB pool for atomic
> > > > > > > > > > coherent allocation
> > > > > > >
> > > > > > > Is due to :
> > > > > > >
> > > > > > >
> > > > > > > 3618082c
> > > > > > > ("arm64 use both ZONE_DMA and ZONE_DMA32")
> > > > > > >
> > > > > > > With the introduction of ZONE_DMA to support the Raspberry DMA
> > > > > > > region below 1G, the crashkernel is placed in the upper 4G
> > > > > > > ZONE_DMA_32 region. Since the crashkernel does not have access
> > > > > > > to the ZONE_DMA region, it prints out call trace during bootup.
> > > > > > >
> > > > > > > It is due to having this CONFIG item ON :
> > > > > > >
> > > > > > >
> > > > > > > CONFIG_ZONE_DMA=y
> > > > > > >
> > > > > > > Turning off ZONE_DMA fixes a issue and Raspberry PI 4 will
> > > > > > > use the device tree to specify memory below 1G.
> > > > > > >
> > > > > > >
> > > > > > Disabling ZONE_DMA is temporary solution. We may need proper
> > > > > > solution
> > > > >
> > > > > Perhaps the Raspberry platform configuration dependencies need
> > > > > separated from “server class” Arm equipment ? Or auto-configured on
> > > > > boot ? Consult an expert ;-)
> > > > >
> > > > >
> > > > >
> > > > > > > I would like to see Chen’s feature added , perhaps as
> > > > > > > EXPERIMENTAL, so we can get some configuration testing done on
> > > > > > > it. It corrects having a DMA zone in low memory while crash-
> > > > > > > kernel is above 4GB. This has been going on for a year now.
> > > > > > I will also like this patch to be added in Linux as early as
> > > > > > possible.
> > > > > >
> > > > > > Issue mentioned by me happens with or without this patch.
> > > > > >
> > > > > > This patch-set can consider fixing because it uses low memory for
> > > > > > DMA
> > > > > > & swiotlb only.
> > > > > > We can consider restricting crashkernel within the required range
> > > > > > like below
> > > > > >
> > > > > > diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> > > > > > index 7f9e5a6dc48c..bd67b90d35bd 100644
> > > > > > --- a/kernel/crash_core.c
> > > > > > +++ b/kernel/crash_core.c
> > > > > > @@ -354,7 +354,7 @@ int __init reserve_crashkernel_low(void)
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > - low_base = memblock_find_in_range(0, 1ULL << 32, low_size,
> > > > > > CRASH_ALIGN);
> > > > > > + low_base = memblock_find_in_range(0,0xc0000000, low_size,
> > > > > > CRASH_ALIGN);
> > > > > > if (!low_base) {
> > > > > > pr_err("Cannot reserve %ldMB crashkernel low memory,
> > > > > > please try smaller size.\n",
> > > > > > (unsigned long)(low_size >> 20));
> > > > > >
> > > > > >
> > > > > I suspect 0xc0000000 would need to be a CONFIG item and not
> > > > > hard-coded.
> > > > >
> > > > if you consider this as valid change, can you please incorporate as
> > > > part of your patch-set.
> > >
> > > After commit 1a8e1cef7 ("arm64: use both ZONE_DMA and ZONE_DMA32")，the 0-
> > > 4G memory is splited
> > > to DMA [mem 0x0000000000000000-0x000000003fffffff] and DMA32 [mem
> > > 0x0000000040000000-0x00000000ffffffff] on arm64.
> > >
> > > From the above discussion, on your platform, the low crashkernel fall in
> > > DMA32 region, but your environment needs to access DMA
> > > region, so there is the call trace.
> > >
> > > I have a question, why do you choose 0xc0000000 here?
> > >
> > > Besides, this is common code, we also need to consider about x86.
> > >
> >
> > + nsaenzjulienne@suse.de
Thanks for adding me to the conversation, and sorry for the headaches.
> >
> > Exactly . This is why it needs to be a CONFIG option for Raspberry
> > .., or device tree option.
> >
> >
> > We could revert 1a8e1cef7 since it broke Arm kdump too.
>
> Well, unfortunately the patch for commit 1a8e1cef7603 ("arm64: use
> both ZONE_DMA and ZONE_DMA32") was not Cc'ed to the kexec mailing
> list, thus we couldn't get many eyes on it for a thorough review from
> kexec/kdump p-o-v.
>
> Also we historically never had distinction in common arch code on the
> basis of the intended end use-case: embedded, server or automotive, so
> I am not sure introducing a Raspberry specific CONFIG option would be
> a good idea.
+1
From the distros perspective it's very important to keep a single kernel image.
> So, rather than reverting the patch, we can look at addressing the
> same properly this time - especially from a kdump p-o-v.
> This issue has been reported by some Red Hat arm64 partners with
> upstream kernel also and as we have noticed in the past as well,
> hardcoding the placement of the crashkernel base address (unless the
> base address is specified by a crashkernel=X@Y like bootargs) is also
> not a portable suggestion.
>
> I am working on a possible fix and will have more updates on the same
> in a day-or-two.
Please keep me in the loop, we've also had issues pointing to this reported by
SUSE partners. I can do some testing both on the RPi4 and on big servers that
need huge crashkernel sizes.
Regards,
Nicolas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] ARM: dts: bcm2711: Update SPI nodes compatible strings
From: Florian Fainelli @ 2020-06-04 16:56 UTC (permalink / raw)
To: Stefan Wahren, Lukas Wunner
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Scott Branden, Ray Jui, linux-kernel, Rob Herring,
open list:SPI SUBSYSTEM, Mark Brown,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl, Nicolas Saenz Julienne
In-Reply-To: <2978874a-fe1e-3b07-381d-55dcb00ecca7@i2se.com>
On 6/4/2020 9:54 AM, Stefan Wahren wrote:
> Am 04.06.20 um 18:40 schrieb Florian Fainelli:
>>
>> On 6/3/2020 9:20 PM, Lukas Wunner wrote:
>>> On Wed, Jun 03, 2020 at 08:46:54PM -0700, Florian Fainelli wrote:
>>>> The BCM2711 SoC features 5 SPI controllers which all share the same
>>>> interrupt line, the SPI driver needs to support interrupt sharing,
>>>> therefore use the chip specific compatible string to help with that.
>>> You're saying above that the 5 controllers all share the interrupt
>>> but below you're only changing the compatible string of 4 controllers.
>>>
>>> So I assume spi0 still has its own interrupt and only the additional
>>> 4 controllers present on the BCM2711/BCM7211 share their interrupt?
>> Correct, there are 5 instances, but only the 4 that were added for 2711
>> actually share the interrupt line, I will correct that in the next patch
>> version.
>
> No, all 5 instances uses the same interrupt line. Please see my comment
> before.
OK, but this is not going to be needed, I have another solution that
does not involve device tree changes.
--
Florian
^ permalink raw reply
* Re: [PATCH 2/3] ARM: dts: bcm2711: Update SPI nodes compatible strings
From: Stefan Wahren @ 2020-06-04 16:54 UTC (permalink / raw)
To: Florian Fainelli, Lukas Wunner
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Scott Branden, Ray Jui, linux-kernel, Rob Herring,
open list:SPI SUBSYSTEM, Mark Brown,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl, Nicolas Saenz Julienne
In-Reply-To: <15c3995e-87de-0f2b-3424-5dd698b181d3@gmail.com>
Am 04.06.20 um 18:40 schrieb Florian Fainelli:
>
> On 6/3/2020 9:20 PM, Lukas Wunner wrote:
>> On Wed, Jun 03, 2020 at 08:46:54PM -0700, Florian Fainelli wrote:
>>> The BCM2711 SoC features 5 SPI controllers which all share the same
>>> interrupt line, the SPI driver needs to support interrupt sharing,
>>> therefore use the chip specific compatible string to help with that.
>> You're saying above that the 5 controllers all share the interrupt
>> but below you're only changing the compatible string of 4 controllers.
>>
>> So I assume spi0 still has its own interrupt and only the additional
>> 4 controllers present on the BCM2711/BCM7211 share their interrupt?
> Correct, there are 5 instances, but only the 4 that were added for 2711
> actually share the interrupt line, I will correct that in the next patch
> version.
No, all 5 instances uses the same interrupt line. Please see my comment
before.
Regards
^ permalink raw reply
* Re: [PATCH net-next v6 4/4] net: dp83869: Add RGMII internal delay configuration
From: Jakub Kicinski @ 2020-06-04 16:48 UTC (permalink / raw)
To: Dan Murphy
Cc: andrew, f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
devicetree
In-Reply-To: <63a53dad-4f0a-31ca-ad1a-361b633c28bf@ti.com>
On Thu, 4 Jun 2020 11:38:14 -0500 Dan Murphy wrote:
> Jakub
>
> On 6/4/20 11:25 AM, Jakub Kicinski wrote:
> > On Thu, 4 Jun 2020 06:14:10 -0500 Dan Murphy wrote:
> >> Add RGMII internal delay configuration for Rx and Tx.
> >>
> >> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> > Hi Dan, please make sure W=1 C=1 build is clean:
> >
> > drivers/net/phy/dp83869.c:103:18: warning: ‘dp83869_internal_delay’ defined but not used [-Wunused-const-variable=]
> > 103 | static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
> > | ^~~~~~~~~~~~~~~~~~~~~~
>
> I built with W=1 and C=1 and did not see this warning.
>
> What defconfig are you using?
allmodconfig with gcc-10
> Can you check if CONFIG_OF_MDIO is set or not? That would be the only
> way that warning would come up.
Hm. I don't have the config from this particular build but just running
allmodconfig makes it CONFIG_OF_MDIO=m
> > Also net-next is closed right now, you can post RFCs but normal patches
> > should be deferred until after net-next reopens.
>
> I know net-next is closed.
>
> I pinged David M when it was open about what is meant by "new" patches
> in the net-dev FAQ. So I figured I would send the patches to see what
> the response was.
>
> To me these are not new they are in process patches. My understand is
> New is v1 patchesets.
>
> But now I have the answer.
Oh sorry, I may be wrong in this case, I haven't tracked this series.
^ permalink raw reply
* Re: [PATCH 2/3] ARM: dts: bcm2711: Update SPI nodes compatible strings
From: Stefan Wahren @ 2020-06-04 16:46 UTC (permalink / raw)
To: Florian Fainelli, linux-kernel
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Scott Branden, lukas, Ray Jui, Rob Herring,
open list:SPI SUBSYSTEM, Mark Brown,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl, Nicolas Saenz Julienne
In-Reply-To: <20200604034655.15930-3-f.fainelli@gmail.com>
Hi Florian,
Am 04.06.20 um 05:46 schrieb Florian Fainelli:
> The BCM2711 SoC features 5 SPI controllers which all share the same
> interrupt line, the SPI driver needs to support interrupt sharing,
> therefore use the chip specific compatible string to help with that.
the commit message is correct about 5 SPI controllers, but the patch
only changes 4 ones.
Please add the new compatibles also for &spi (included from
bcm283x.dtsi) below in this file, which also share interrupt 118.
Thanks
^ permalink raw reply
* Re: [PATCH 2/3] ARM: dts: bcm2711: Update SPI nodes compatible strings
From: Florian Fainelli @ 2020-06-04 16:40 UTC (permalink / raw)
To: Lukas Wunner, Florian Fainelli
Cc: linux-kernel, Mark Brown, Rob Herring, Nicolas Saenz Julienne,
Ray Jui, Scott Branden,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
open list:SPI SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl
In-Reply-To: <20200604042038.jzolu6k7q3d6bsvq@wunner.de>
On 6/3/2020 9:20 PM, Lukas Wunner wrote:
> On Wed, Jun 03, 2020 at 08:46:54PM -0700, Florian Fainelli wrote:
>> The BCM2711 SoC features 5 SPI controllers which all share the same
>> interrupt line, the SPI driver needs to support interrupt sharing,
>> therefore use the chip specific compatible string to help with that.
>
> You're saying above that the 5 controllers all share the interrupt
> but below you're only changing the compatible string of 4 controllers.
>
> So I assume spi0 still has its own interrupt and only the additional
> 4 controllers present on the BCM2711/BCM7211 share their interrupt?
Correct, there are 5 instances, but only the 4 that were added for 2711
actually share the interrupt line, I will correct that in the next patch
version.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v6 4/4] net: dp83869: Add RGMII internal delay configuration
From: Dan Murphy @ 2020-06-04 16:38 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andrew, f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
devicetree
In-Reply-To: <20200604092545.40c85fce@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Jakub
On 6/4/20 11:25 AM, Jakub Kicinski wrote:
> On Thu, 4 Jun 2020 06:14:10 -0500 Dan Murphy wrote:
>> Add RGMII internal delay configuration for Rx and Tx.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> Hi Dan, please make sure W=1 C=1 build is clean:
>
> drivers/net/phy/dp83869.c:103:18: warning: ‘dp83869_internal_delay’ defined but not used [-Wunused-const-variable=]
> 103 | static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
> | ^~~~~~~~~~~~~~~~~~~~~~
I built with W=1 and C=1 and did not see this warning.
What defconfig are you using?
Can you check if CONFIG_OF_MDIO is set or not? That would be the only
way that warning would come up.
> Also net-next is closed right now, you can post RFCs but normal patches
> should be deferred until after net-next reopens.
I know net-next is closed.
I pinged David M when it was open about what is meant by "new" patches
in the net-dev FAQ. So I figured I would send the patches to see what
the response was.
To me these are not new they are in process patches. My understand is
New is v1 patchesets.
But now I have the answer.
Dan
^ permalink raw reply
* Re: [PATCH v25 03/16] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
From: Dan Murphy @ 2020-06-04 16:28 UTC (permalink / raw)
To: Jacek Anaszewski, Pavel Machek; +Cc: robh, devicetree, linux-leds, linux-kernel
In-Reply-To: <c03ce8da-0895-2e1f-0a4c-2b3d9fae8d4d@gmail.com>
Jacek
On 6/1/20 4:34 AM, Jacek Anaszewski wrote:
> Hi Pavel and Dan,
>
> On 5/31/20 9:06 PM, Pavel Machek wrote:
>> Hi!
>>
>>>>> + There can only be one instance of the ti,led-bank
>>>>> + property for each device node. This is a required node
>>>>> is the LED
>>>>> + modules are to be backed.
>>>> I don't understand the second sentence. Pretty sure it is not valid
>>>> english.
>>>
>>>
>>> If I make these changes is this still viable for 5.8 or would you
>>> then go
>>> into 5.9?
>>
>> It really depends if we get -rc8 or not, and if you'll need to do any
>> changes to C code or not...
>
> I think that we need to simmer such a big extension of the LED
> subsystem for a whole cycle in linux-next, especially taking into
> account addition of new sysfs interface, that is bit quirky.
>
> Effectively 5.8 seems to not have been viable since few weeks.
>
After thinking about this for a while I would actually think to have
this in 5.9.
Either 5.7 or 5.8 will be the 2020 LTS and such a new interface would be
best suited for intermediate stable releases that get EOL'd faster.
This way we don't have to back port bug fixes for 2 years.
Dan
^ permalink raw reply
* Re: [PATCH net-next v6 4/4] net: dp83869: Add RGMII internal delay configuration
From: Jakub Kicinski @ 2020-06-04 16:25 UTC (permalink / raw)
To: Dan Murphy
Cc: andrew, f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
devicetree
In-Reply-To: <20200604111410.17918-5-dmurphy@ti.com>
On Thu, 4 Jun 2020 06:14:10 -0500 Dan Murphy wrote:
> Add RGMII internal delay configuration for Rx and Tx.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
Hi Dan, please make sure W=1 C=1 build is clean:
drivers/net/phy/dp83869.c:103:18: warning: ‘dp83869_internal_delay’ defined but not used [-Wunused-const-variable=]
103 | static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
| ^~~~~~~~~~~~~~~~~~~~~~
Also net-next is closed right now, you can post RFCs but normal patches
should be deferred until after net-next reopens.
^ permalink raw reply
* Re: [PATCH v6 1/2] phy: qualcomm: add qcom ipq806x dwc usb phy driver
From: Jonathan McDowell @ 2020-06-04 16:19 UTC (permalink / raw)
To: Ansuel Smith
Cc: Bjorn Andersson, Andy Gross, Andy Gross, Kishon Vijay Abraham I,
Rob Herring, Mark Rutland, linux-arm-msm, linux-kernel,
devicetree
In-Reply-To: <20200603132237.6036-1-ansuelsmth@gmail.com>
On Wed, Jun 03, 2020 at 03:22:34PM +0200, Ansuel Smith wrote:
> This has lost in the original push for the dwc3 qcom driver.
> This is needed for ipq806x SoC as without this the usb ports
> doesn't work at all.
FWIW I tested this on my RB3011 so feel free to add:
Tested-by: Jonathan McDowell <noodles@earth.li>
One minor comment; would PHY_QCOM_USB_IPQ806X not be a better choice
than PHY_QCOM_IPQ806X_USB given the existing naming?
> Signed-off-by: Andy Gross <agross@codeaurora.org>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
> v6:
> * Use GENMASK instead of hex value
> v4:
> * Add qcom to specific bindings
> v3:
> * Use reg instead of regmap phandle
> v2:
> * Renamed config from PHY_QCOM_DWC3 to PHY_QCOM_IPQ806X_USB
> * Rename inline function to generic name to reduce length
> * Fix check reported by checkpatch --strict
> * Rename compatible to qcom,ipq806x-usb-phy-(hs/ss)
>
> drivers/phy/qualcomm/Kconfig | 12 +
> drivers/phy/qualcomm/Makefile | 1 +
> drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c | 593 ++++++++++++++++++++
> 3 files changed, 606 insertions(+)
> create mode 100644 drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
>
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index e46824da29f6..9d41c3d12800 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -91,3 +91,15 @@ config PHY_QCOM_USB_HSIC
> select GENERIC_PHY
> help
> Support for the USB HSIC ULPI compliant PHY on QCOM chipsets.
> +
> +config PHY_QCOM_IPQ806X_USB
> + tristate "Qualcomm IPQ806x DWC3 USB PHY driver"
> + depends on ARCH_QCOM
> + depends on HAS_IOMEM
> + depends on OF
> + select GENERIC_PHY
> + help
> + This option enables support for the Synopsis PHYs present inside the
> + Qualcomm USB3.0 DWC3 controller on ipq806x SoC. This driver supports
> + both HS and SS PHY controllers.
> +
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index 283251d6a5d9..8629299c1495 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_PHY_QCOM_UFS_14NM) += phy-qcom-ufs-qmp-14nm.o
> obj-$(CONFIG_PHY_QCOM_UFS_20NM) += phy-qcom-ufs-qmp-20nm.o
> obj-$(CONFIG_PHY_QCOM_USB_HS) += phy-qcom-usb-hs.o
> obj-$(CONFIG_PHY_QCOM_USB_HSIC) += phy-qcom-usb-hsic.o
> +obj-$(CONFIG_PHY_QCOM_IPQ806X_USB) += phy-qcom-ipq806x-usb.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> new file mode 100644
> index 000000000000..f37cd8760118
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> @@ -0,0 +1,593 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2014-2015, Code Aurora Forum. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only 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.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +
> +/* USB QSCRATCH Hardware registers */
> +#define QSCRATCH_GENERAL_CFG (0x08)
> +#define HSUSB_PHY_CTRL_REG (0x10)
> +
> +/* PHY_CTRL_REG */
> +#define HSUSB_CTRL_DMSEHV_CLAMP BIT(24)
> +#define HSUSB_CTRL_USB2_SUSPEND BIT(23)
> +#define HSUSB_CTRL_UTMI_CLK_EN BIT(21)
> +#define HSUSB_CTRL_UTMI_OTG_VBUS_VALID BIT(20)
> +#define HSUSB_CTRL_USE_CLKCORE BIT(18)
> +#define HSUSB_CTRL_DPSEHV_CLAMP BIT(17)
> +#define HSUSB_CTRL_COMMONONN BIT(11)
> +#define HSUSB_CTRL_ID_HV_CLAMP BIT(9)
> +#define HSUSB_CTRL_OTGSESSVLD_CLAMP BIT(8)
> +#define HSUSB_CTRL_CLAMP_EN BIT(7)
> +#define HSUSB_CTRL_RETENABLEN BIT(1)
> +#define HSUSB_CTRL_POR BIT(0)
> +
> +/* QSCRATCH_GENERAL_CFG */
> +#define HSUSB_GCFG_XHCI_REV BIT(2)
> +
> +/* USB QSCRATCH Hardware registers */
> +#define SSUSB_PHY_CTRL_REG (0x00)
> +#define SSUSB_PHY_PARAM_CTRL_1 (0x04)
> +#define SSUSB_PHY_PARAM_CTRL_2 (0x08)
> +#define CR_PROTOCOL_DATA_IN_REG (0x0c)
> +#define CR_PROTOCOL_DATA_OUT_REG (0x10)
> +#define CR_PROTOCOL_CAP_ADDR_REG (0x14)
> +#define CR_PROTOCOL_CAP_DATA_REG (0x18)
> +#define CR_PROTOCOL_READ_REG (0x1c)
> +#define CR_PROTOCOL_WRITE_REG (0x20)
> +
> +/* PHY_CTRL_REG */
> +#define SSUSB_CTRL_REF_USE_PAD BIT(28)
> +#define SSUSB_CTRL_TEST_POWERDOWN BIT(27)
> +#define SSUSB_CTRL_LANE0_PWR_PRESENT BIT(24)
> +#define SSUSB_CTRL_SS_PHY_EN BIT(8)
> +#define SSUSB_CTRL_SS_PHY_RESET BIT(7)
> +
> +/* SSPHY control registers - Does this need 0x30? */
> +#define SSPHY_CTRL_RX_OVRD_IN_HI(lane) (0x1006 + 0x100 * (lane))
> +#define SSPHY_CTRL_TX_OVRD_DRV_LO(lane) (0x1002 + 0x100 * (lane))
> +
> +/* SSPHY SoC version specific values */
> +#define SSPHY_RX_EQ_VALUE 4 /* Override value for rx_eq */
> +/* Override value for transmit preemphasis */
> +#define SSPHY_TX_DEEMPH_3_5DB 23
> +/* Override value for mpll */
> +#define SSPHY_MPLL_VALUE 0
> +
> +/* QSCRATCH PHY_PARAM_CTRL1 fields */
> +#define PHY_PARAM_CTRL1_TX_FULL_SWING_MASK GENMASK(26, 19)
> +#define PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK GENMASK(19, 13)
> +#define PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK GENMASK(13, 7)
> +#define PHY_PARAM_CTRL1_LOS_BIAS_MASK GENMASK(7, 2)
> +
> +#define PHY_PARAM_CTRL1_MASK \
> + (PHY_PARAM_CTRL1_TX_FULL_SWING_MASK | \
> + PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK | \
> + PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK | \
> + PHY_PARAM_CTRL1_LOS_BIAS_MASK)
> +
> +#define PHY_PARAM_CTRL1_TX_FULL_SWING(x) \
> + (((x) << 20) & PHY_PARAM_CTRL1_TX_FULL_SWING_MASK)
> +#define PHY_PARAM_CTRL1_TX_DEEMPH_6DB(x) \
> + (((x) << 14) & PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK)
> +#define PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB(x) \
> + (((x) << 8) & PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK)
> +#define PHY_PARAM_CTRL1_LOS_BIAS(x) \
> + (((x) << 3) & PHY_PARAM_CTRL1_LOS_BIAS_MASK)
> +
> +/* RX OVRD IN HI bits */
> +#define RX_OVRD_IN_HI_RX_RESET_OVRD BIT(13)
> +#define RX_OVRD_IN_HI_RX_RX_RESET BIT(12)
> +#define RX_OVRD_IN_HI_RX_EQ_OVRD BIT(11)
> +#define RX_OVRD_IN_HI_RX_EQ_MASK GENMASK(10, 7)
> +#define RX_OVRD_IN_HI_RX_EQ(x) ((x) << 8)
> +#define RX_OVRD_IN_HI_RX_EQ_EN_OVRD BIT(7)
> +#define RX_OVRD_IN_HI_RX_EQ_EN BIT(6)
> +#define RX_OVRD_IN_HI_RX_LOS_FILTER_OVRD BIT(5)
> +#define RX_OVRD_IN_HI_RX_LOS_FILTER_MASK GENMASK(4, 2)
> +#define RX_OVRD_IN_HI_RX_RATE_OVRD BIT(2)
> +#define RX_OVRD_IN_HI_RX_RATE_MASK GENMASK(2, 0)
> +
> +/* TX OVRD DRV LO register bits */
> +#define TX_OVRD_DRV_LO_AMPLITUDE_MASK GENMASK(6, 0)
> +#define TX_OVRD_DRV_LO_PREEMPH_MASK GENMASK(13, 6)
> +#define TX_OVRD_DRV_LO_PREEMPH(x) ((x) << 7)
> +#define TX_OVRD_DRV_LO_EN BIT(14)
> +
> +/* MPLL bits */
> +#define SSPHY_MPLL_MASK GENMASK(8, 5)
> +#define SSPHY_MPLL(x) ((x) << 5)
> +
> +/* SS CAP register bits */
> +#define SS_CR_CAP_ADDR_REG BIT(0)
> +#define SS_CR_CAP_DATA_REG BIT(0)
> +#define SS_CR_READ_REG BIT(0)
> +#define SS_CR_WRITE_REG BIT(0)
> +
> +struct usb_phy {
> + void __iomem *base;
> + struct device *dev;
> + struct clk *xo_clk;
> + struct clk *ref_clk;
> + u32 rx_eq;
> + u32 tx_deamp_3_5db;
> + u32 mpll;
> +};
> +
> +struct phy_drvdata {
> + struct phy_ops ops;
> + u32 clk_rate;
> +};
> +
> +/**
> + * Write register and read back masked value to confirm it is written
> + *
> + * @base - QCOM DWC3 PHY base virtual address.
> + * @offset - register offset.
> + * @mask - register bitmask specifying what should be updated
> + * @val - value to write.
> + */
> +static inline void usb_phy_write_readback(struct usb_phy *phy_dwc3,
> + u32 offset,
> + const u32 mask, u32 val)
> +{
> + u32 write_val, tmp = readl(phy_dwc3->base + offset);
> +
> + tmp &= ~mask; /* retain other bits */
> + write_val = tmp | val;
> +
> + writel(write_val, phy_dwc3->base + offset);
> +
> + /* Read back to see if val was written */
> + tmp = readl(phy_dwc3->base + offset);
> + tmp &= mask; /* clear other bits */
> +
> + if (tmp != val)
> + dev_err(phy_dwc3->dev, "write: %x to QSCRATCH: %x FAILED\n",
> + val, offset);
> +}
> +
> +static int wait_for_latch(void __iomem *addr)
> +{
> + u32 retry = 10;
> +
> + while (true) {
> + if (!readl(addr))
> + break;
> +
> + if (--retry == 0)
> + return -ETIMEDOUT;
> +
> + usleep_range(10, 20);
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * Write SSPHY register
> + *
> + * @base - QCOM DWC3 PHY base virtual address.
> + * @addr - SSPHY address to write.
> + * @val - value to write.
> + */
> +static int usb_ss_write_phycreg(struct usb_phy *phy_dwc3,
> + u32 addr, u32 val)
> +{
> + int ret;
> +
> + writel(addr, phy_dwc3->base + CR_PROTOCOL_DATA_IN_REG);
> + writel(SS_CR_CAP_ADDR_REG,
> + phy_dwc3->base + CR_PROTOCOL_CAP_ADDR_REG);
> +
> + ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_CAP_ADDR_REG);
> + if (ret)
> + goto err_wait;
> +
> + writel(val, phy_dwc3->base + CR_PROTOCOL_DATA_IN_REG);
> + writel(SS_CR_CAP_DATA_REG,
> + phy_dwc3->base + CR_PROTOCOL_CAP_DATA_REG);
> +
> + ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_CAP_DATA_REG);
> + if (ret)
> + goto err_wait;
> +
> + writel(SS_CR_WRITE_REG, phy_dwc3->base + CR_PROTOCOL_WRITE_REG);
> +
> + ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_WRITE_REG);
> +
> +err_wait:
> + if (ret)
> + dev_err(phy_dwc3->dev, "timeout waiting for latch\n");
> + return ret;
> +}
> +
> +/**
> + * Read SSPHY register.
> + *
> + * @base - QCOM DWC3 PHY base virtual address.
> + * @addr - SSPHY address to read.
> + */
> +static int usb_ss_read_phycreg(struct usb_phy *phy_dwc3,
> + u32 addr, u32 *val)
> +{
> + int ret;
> +
> + writel(addr, phy_dwc3->base + CR_PROTOCOL_DATA_IN_REG);
> + writel(SS_CR_CAP_ADDR_REG,
> + phy_dwc3->base + CR_PROTOCOL_CAP_ADDR_REG);
> +
> + ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_CAP_ADDR_REG);
> + if (ret)
> + goto err_wait;
> +
> + /*
> + * Due to hardware bug, first read of SSPHY register might be
> + * incorrect. Hence as workaround, SW should perform SSPHY register
> + * read twice, but use only second read and ignore first read.
> + */
> + writel(SS_CR_READ_REG, phy_dwc3->base + CR_PROTOCOL_READ_REG);
> +
> + ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_READ_REG);
> + if (ret)
> + goto err_wait;
> +
> + /* throwaway read */
> + readl(phy_dwc3->base + CR_PROTOCOL_DATA_OUT_REG);
> +
> + writel(SS_CR_READ_REG, phy_dwc3->base + CR_PROTOCOL_READ_REG);
> +
> + ret = wait_for_latch(phy_dwc3->base + CR_PROTOCOL_READ_REG);
> + if (ret)
> + goto err_wait;
> +
> + *val = readl(phy_dwc3->base + CR_PROTOCOL_DATA_OUT_REG);
> +
> +err_wait:
> + return ret;
> +}
> +
> +static int qcom_ipq806x_usb_hs_phy_init(struct phy *phy)
> +{
> + struct usb_phy *phy_dwc3 = phy_get_drvdata(phy);
> + int ret;
> + u32 val;
> +
> + ret = clk_prepare_enable(phy_dwc3->xo_clk);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(phy_dwc3->ref_clk);
> + if (ret) {
> + clk_disable_unprepare(phy_dwc3->xo_clk);
> + return ret;
> + }
> +
> + /*
> + * HSPHY Initialization: Enable UTMI clock, select 19.2MHz fsel
> + * enable clamping, and disable RETENTION (power-on default is ENABLED)
> + */
> + val = HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_DMSEHV_CLAMP |
> + HSUSB_CTRL_RETENABLEN | HSUSB_CTRL_COMMONONN |
> + HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
> + HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_UTMI_OTG_VBUS_VALID |
> + HSUSB_CTRL_UTMI_CLK_EN | HSUSB_CTRL_CLAMP_EN | 0x70;
> +
> + /* use core clock if external reference is not present */
> + if (!phy_dwc3->xo_clk)
> + val |= HSUSB_CTRL_USE_CLKCORE;
> +
> + writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);
> + usleep_range(2000, 2200);
> +
> + /* Disable (bypass) VBUS and ID filters */
> + writel(HSUSB_GCFG_XHCI_REV, phy_dwc3->base + QSCRATCH_GENERAL_CFG);
> +
> + return 0;
> +}
> +
> +static int qcom_ipq806x_usb_hs_phy_exit(struct phy *phy)
> +{
> + struct usb_phy *phy_dwc3 = phy_get_drvdata(phy);
> +
> + clk_disable_unprepare(phy_dwc3->ref_clk);
> + clk_disable_unprepare(phy_dwc3->xo_clk);
> +
> + return 0;
> +}
> +
> +static int qcom_ipq806x_usb_ss_phy_init(struct phy *phy)
> +{
> + struct usb_phy *phy_dwc3 = phy_get_drvdata(phy);
> + int ret;
> + u32 data = 0;
> +
> + ret = clk_prepare_enable(phy_dwc3->xo_clk);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(phy_dwc3->ref_clk);
> + if (ret) {
> + clk_disable_unprepare(phy_dwc3->xo_clk);
> + return ret;
> + }
> +
> + /* reset phy */
> + data = readl(phy_dwc3->base + SSUSB_PHY_CTRL_REG);
> + writel(data | SSUSB_CTRL_SS_PHY_RESET,
> + phy_dwc3->base + SSUSB_PHY_CTRL_REG);
> + usleep_range(2000, 2200);
> + writel(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
> +
> + /* clear REF_PAD if we don't have XO clk */
> + if (!phy_dwc3->xo_clk)
> + data &= ~SSUSB_CTRL_REF_USE_PAD;
> + else
> + data |= SSUSB_CTRL_REF_USE_PAD;
> +
> + writel(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
> +
> + /* wait for ref clk to become stable, this can take up to 30ms */
> + msleep(30);
> +
> + data |= SSUSB_CTRL_SS_PHY_EN | SSUSB_CTRL_LANE0_PWR_PRESENT;
> + writel(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
> +
> + /*
> + * WORKAROUND: There is SSPHY suspend bug due to which USB enumerates
> + * in HS mode instead of SS mode. Workaround it by asserting
> + * LANE0.TX_ALT_BLOCK.EN_ALT_BUS to enable TX to use alt bus mode
> + */
> + ret = usb_ss_read_phycreg(phy_dwc3, 0x102D, &data);
> + if (ret)
> + goto err_phy_trans;
> +
> + data |= (1 << 7);
> + ret = usb_ss_write_phycreg(phy_dwc3, 0x102D, data);
> + if (ret)
> + goto err_phy_trans;
> +
> + ret = usb_ss_read_phycreg(phy_dwc3, 0x1010, &data);
> + if (ret)
> + goto err_phy_trans;
> +
> + data &= ~0xff0;
> + data |= 0x20;
> + ret = usb_ss_write_phycreg(phy_dwc3, 0x1010, data);
> + if (ret)
> + goto err_phy_trans;
> +
> + /*
> + * Fix RX Equalization setting as follows
> + * LANE0.RX_OVRD_IN_HI. RX_EQ_EN set to 0
> + * LANE0.RX_OVRD_IN_HI.RX_EQ_EN_OVRD set to 1
> + * LANE0.RX_OVRD_IN_HI.RX_EQ set based on SoC version
> + * LANE0.RX_OVRD_IN_HI.RX_EQ_OVRD set to 1
> + */
> + ret = usb_ss_read_phycreg(phy_dwc3,
> + SSPHY_CTRL_RX_OVRD_IN_HI(0), &data);
> + if (ret)
> + goto err_phy_trans;
> +
> + data &= ~RX_OVRD_IN_HI_RX_EQ_EN;
> + data |= RX_OVRD_IN_HI_RX_EQ_EN_OVRD;
> + data &= ~RX_OVRD_IN_HI_RX_EQ_MASK;
> + data |= RX_OVRD_IN_HI_RX_EQ(phy_dwc3->rx_eq);
> + data |= RX_OVRD_IN_HI_RX_EQ_OVRD;
> + ret = usb_ss_write_phycreg(phy_dwc3,
> + SSPHY_CTRL_RX_OVRD_IN_HI(0), data);
> + if (ret)
> + goto err_phy_trans;
> +
> + /*
> + * Set EQ and TX launch amplitudes as follows
> + * LANE0.TX_OVRD_DRV_LO.PREEMPH set based on SoC version
> + * LANE0.TX_OVRD_DRV_LO.AMPLITUDE set to 110
> + * LANE0.TX_OVRD_DRV_LO.EN set to 1.
> + */
> + ret = usb_ss_read_phycreg(phy_dwc3,
> + SSPHY_CTRL_TX_OVRD_DRV_LO(0), &data);
> + if (ret)
> + goto err_phy_trans;
> +
> + data &= ~TX_OVRD_DRV_LO_PREEMPH_MASK;
> + data |= TX_OVRD_DRV_LO_PREEMPH(phy_dwc3->tx_deamp_3_5db);
> + data &= ~TX_OVRD_DRV_LO_AMPLITUDE_MASK;
> + data |= 0x6E;
> + data |= TX_OVRD_DRV_LO_EN;
> + ret = usb_ss_write_phycreg(phy_dwc3,
> + SSPHY_CTRL_TX_OVRD_DRV_LO(0), data);
> + if (ret)
> + goto err_phy_trans;
> +
> + data = 0;
> + data &= ~SSPHY_MPLL_MASK;
> + data |= SSPHY_MPLL(phy_dwc3->mpll);
> + usb_ss_write_phycreg(phy_dwc3, 0x30, data);
> +
> + /*
> + * Set the QSCRATCH PHY_PARAM_CTRL1 parameters as follows
> + * TX_FULL_SWING [26:20] amplitude to 110
> + * TX_DEEMPH_6DB [19:14] to 32
> + * TX_DEEMPH_3_5DB [13:8] set based on SoC version
> + * LOS_BIAS [7:3] to 9
> + */
> + data = readl(phy_dwc3->base + SSUSB_PHY_PARAM_CTRL_1);
> +
> + data &= ~PHY_PARAM_CTRL1_MASK;
> +
> + data |= PHY_PARAM_CTRL1_TX_FULL_SWING(0x6e) |
> + PHY_PARAM_CTRL1_TX_DEEMPH_6DB(0x20) |
> + PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB(phy_dwc3->tx_deamp_3_5db) |
> + PHY_PARAM_CTRL1_LOS_BIAS(0x9);
> +
> + usb_phy_write_readback(phy_dwc3, SSUSB_PHY_PARAM_CTRL_1,
> + PHY_PARAM_CTRL1_MASK, data);
> +
> +err_phy_trans:
> + return ret;
> +}
> +
> +static int qcom_ipq806x_usb_ss_phy_exit(struct phy *phy)
> +{
> + struct usb_phy *phy_dwc3 = phy_get_drvdata(phy);
> +
> + /* Sequence to put SSPHY in low power state:
> + * 1. Clear REF_PHY_EN in PHY_CTRL_REG
> + * 2. Clear REF_USE_PAD in PHY_CTRL_REG
> + * 3. Set TEST_POWERED_DOWN in PHY_CTRL_REG to enable PHY retention
> + */
> + usb_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
> + SSUSB_CTRL_SS_PHY_EN, 0x0);
> + usb_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
> + SSUSB_CTRL_REF_USE_PAD, 0x0);
> + usb_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
> + SSUSB_CTRL_TEST_POWERDOWN, 0x0);
> +
> + clk_disable_unprepare(phy_dwc3->ref_clk);
> + clk_disable_unprepare(phy_dwc3->xo_clk);
> +
> + return 0;
> +}
> +
> +static const struct phy_drvdata qcom_ipq806x_usb_hs_drvdata = {
> + .ops = {
> + .init = qcom_ipq806x_usb_hs_phy_init,
> + .exit = qcom_ipq806x_usb_hs_phy_exit,
> + .owner = THIS_MODULE,
> + },
> + .clk_rate = 60000000,
> +};
> +
> +static const struct phy_drvdata qcom_ipq806x_usb_ss_drvdata = {
> + .ops = {
> + .init = qcom_ipq806x_usb_ss_phy_init,
> + .exit = qcom_ipq806x_usb_ss_phy_exit,
> + .owner = THIS_MODULE,
> + },
> + .clk_rate = 125000000,
> +};
> +
> +static const struct of_device_id qcom_ipq806x_usb_phy_table[] = {
> + { .compatible = "qcom,ipq806x-usb-phy-hs",
> + .data = &qcom_ipq806x_usb_hs_drvdata },
> + { .compatible = "qcom,ipq806x-usb-phy-ss",
> + .data = &qcom_ipq806x_usb_ss_drvdata },
> + { /* Sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, qcom_ipq806x_usb_phy_table);
> +
> +static int qcom_ipq806x_usb_phy_probe(struct platform_device *pdev)
> +{
> + struct usb_phy *phy_dwc3;
> + struct phy_provider *phy_provider;
> + struct phy *generic_phy;
> + const struct of_device_id *match;
> + const struct phy_drvdata *data;
> + struct resource *res;
> + resource_size_t size;
> + struct device_node *np;
> +
> + phy_dwc3 = devm_kzalloc(&pdev->dev, sizeof(*phy_dwc3), GFP_KERNEL);
> + if (!phy_dwc3)
> + return -ENOMEM;
> +
> + match = of_match_node(qcom_ipq806x_usb_phy_table, pdev->dev.of_node);
> + data = match->data;
> +
> + phy_dwc3->dev = &pdev->dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -EINVAL;
> + size = resource_size(res);
> + phy_dwc3->base = devm_ioremap(phy_dwc3->dev, res->start, size);
> +
> + if (IS_ERR(phy_dwc3->base)) {
> + dev_err(phy_dwc3->dev, "failed to map reg\n");
> + return PTR_ERR(phy_dwc3->base);
> + }
> +
> + phy_dwc3->ref_clk = devm_clk_get(phy_dwc3->dev, "ref");
> + if (IS_ERR(phy_dwc3->ref_clk)) {
> + dev_dbg(phy_dwc3->dev, "cannot get reference clock\n");
> + return PTR_ERR(phy_dwc3->ref_clk);
> + }
> +
> + clk_set_rate(phy_dwc3->ref_clk, data->clk_rate);
> +
> + phy_dwc3->xo_clk = devm_clk_get(phy_dwc3->dev, "xo");
> + if (IS_ERR(phy_dwc3->xo_clk)) {
> + dev_dbg(phy_dwc3->dev, "cannot get TCXO clock\n");
> + phy_dwc3->xo_clk = NULL;
> + }
> +
> + /* Parse device node to probe HSIO settings */
> + np = of_node_get(pdev->dev.of_node);
> + if (!of_compat_cmp(match->compatible, "qcom,ipq806x-usb-phy-ss",
> + strlen(match->compatible))) {
> + if (of_property_read_u32(np, "qcom,rx-eq", &phy_dwc3->rx_eq) ||
> + of_property_read_u32(np, "qcom,tx-deamp_3_5db",
> + &phy_dwc3->tx_deamp_3_5db) ||
> + of_property_read_u32(np, "qcom,mpll", &phy_dwc3->mpll)) {
> + dev_err(phy_dwc3->dev, "cannot get HSIO settings from device node, using default values\n");
> +
> + /* Default HSIO settings */
> + phy_dwc3->rx_eq = SSPHY_RX_EQ_VALUE;
> + phy_dwc3->tx_deamp_3_5db = SSPHY_TX_DEEMPH_3_5DB;
> + phy_dwc3->mpll = SSPHY_MPLL_VALUE;
> + }
> + }
> +
> + generic_phy = devm_phy_create(phy_dwc3->dev, pdev->dev.of_node,
> + &data->ops);
> +
> + if (IS_ERR(generic_phy))
> + return PTR_ERR(generic_phy);
> +
> + phy_set_drvdata(generic_phy, phy_dwc3);
> + platform_set_drvdata(pdev, phy_dwc3);
> +
> + phy_provider = devm_of_phy_provider_register(phy_dwc3->dev,
> + of_phy_simple_xlate);
> +
> + if (IS_ERR(phy_provider))
> + return PTR_ERR(phy_provider);
> +
> + return 0;
> +}
> +
> +static struct platform_driver qcom_ipq806x_usb_phy_driver = {
> + .probe = qcom_ipq806x_usb_phy_probe,
> + .driver = {
> + .name = "qcom-ipq806x-usb-phy",
> + .owner = THIS_MODULE,
> + .of_match_table = qcom_ipq806x_usb_phy_table,
> + },
> +};
> +
> +module_platform_driver(qcom_ipq806x_usb_phy_driver);
> +
> +MODULE_ALIAS("platform:phy-qcom-ipq806x-usb");
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Andy Gross <agross@codeaurora.org>");
> +MODULE_AUTHOR("Ivan T. Ivanov <iivanov@mm-sol.com>");
> +MODULE_DESCRIPTION("DesignWare USB3 QCOM PHY driver");
> --
> 2.25.1
J.
--
Hell is other people.
^ permalink raw reply
* Re: [PATCH 0/6] arm64: dts: qcom: smmu/USB nodes and HDK855/HDK865 dts
From: Jonathan Marek @ 2020-06-04 16:09 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: linux-arm-msm, Andy Gross, Bjorn Andersson,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, Rob Herring
In-Reply-To: <20200604155825.GI16719@Mani-XPS-13-9360>
On 6/4/20 11:58 AM, Manivannan Sadhasivam wrote:
> On Thu, Jun 04, 2020 at 10:06:19AM -0400, Jonathan Marek wrote:
>> On 6/4/20 9:52 AM, Manivannan Sadhasivam wrote:
>>> Hi,
>>>
>>> On Sat, May 23, 2020 at 10:38:06PM -0400, Jonathan Marek wrote:
>>>> Add dts nodes for apps_smmu and USB for both sm8150 and sm8250.
>>>>
>>>
>>> I've tested this series on an SM8250 based board and able to get Type C (USB0)
>>> working. There are also couple of Type A ports (USB1) on that board behind a
>>> USB hub. It is probing fine but I don't see any activity while connecting a
>>> USB device. Will continue to debug and once I get them working, I'll add my
>>> Tested-by tag.
>>>
>>
>> HDK865 also has a couple Type A ports, I am using them with devices already
>> plugged in during boot and I haven't hit a problem like that, but I think
>> I've seen the same issue when hotplugging. IIRC the behavior was a bit
>> weird, like plugging a device in the Type A port (USB1) nothing would
>> happen, but unplugging/replugging the type C port (USB0) would cause the
>> Type A port device to start working..
>>
>> Have you tried with the devices already plugged in before booting?
>>
>
> Tried it but no luck :/ Also plugging and removing Type C doesn't make any
> difference.
>
This one might be obvious, but do you have 5V power coming out of the
type A ports?
> Thanks,
> Mani
>
>>> Thanks,
>>> Mani
>>>
>>>> Also add initial dts files for HDK855 and HDK865, based on mtp dts, with a
>>>> few changes. Notably, the HDK865 dts has regulator config changed a bit based
>>>> on downstream (I think sm8250-mtp.dts is wrong and copied too much from sm8150).
>>>>
>>>> Jonathan Marek (6):
>>>> arm64: dts: qcom: sm8150: add apps_smmu node
>>>> arm64: dts: qcom: sm8250: add apps_smmu node
>>>> arm64: dts: qcom: sm8150: Add secondary USB and PHY nodes
>>>> arm64: dts: qcom: sm8250: Add USB and PHY device nodes
>>>> arm64: dts: qcom: add sm8150 hdk dts
>>>> arm64: dts: qcom: add sm8250 hdk dts
>>>>
>>>> arch/arm64/boot/dts/qcom/Makefile | 2 +
>>>> arch/arm64/boot/dts/qcom/sm8150-hdk.dts | 461 ++++++++++++++++++++++++
>>>> arch/arm64/boot/dts/qcom/sm8150.dtsi | 180 +++++++++
>>>> arch/arm64/boot/dts/qcom/sm8250-hdk.dts | 454 +++++++++++++++++++++++
>>>> arch/arm64/boot/dts/qcom/sm8250.dtsi | 287 +++++++++++++++
>>>> 5 files changed, 1384 insertions(+)
>>>> create mode 100644 arch/arm64/boot/dts/qcom/sm8150-hdk.dts
>>>> create mode 100644 arch/arm64/boot/dts/qcom/sm8250-hdk.dts
>>>>
>>>> --
>>>> 2.26.1
>>>>
^ permalink raw reply
* Re: [PATCH 3/3] spi: bcm2835: Enable shared interrupt support
From: Florian Fainelli @ 2020-06-04 16:05 UTC (permalink / raw)
To: Mark Brown, Florian Fainelli
Cc: linux-kernel, Rob Herring, Nicolas Saenz Julienne, Ray Jui,
Scott Branden,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
open list:SPI SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Martin Sperl, lukas
In-Reply-To: <20200604123220.GD6644@sirena.org.uk>
On 6/4/2020 5:32 AM, Mark Brown wrote:
> On Wed, Jun 03, 2020 at 08:46:55PM -0700, Florian Fainelli wrote:
>> The SPI controller found in the BCM2711 and BCM7211 SoCs is instantiated
>> 5 times, with all instances sharing the same interrupt line. We
>> specifically match the two compatible strings here to determine whether
>> it is necessary to request the interrupt with the IRQF_SHARED flag and
>> to use an appropriate interrupt handler capable of returning IRQ_NONE.
>
>> For the BCM2835 case which is deemed performance critical, there is no
>> overhead since a dedicated handler that does not assume sharing is used.
>
> This feels hacky - it's essentially using the compatible string to set a
> boolean flag which isn't really about the IP but rather the platform
> integration. It might cause problems if we do end up having to quirk
> this version of the IP for some other reason.
I am not sure why it would be a problem, when you describe a piece of
hardware with Device Tree, even with the IP block being strictly the
same, its very integration into a new SoC (with details like shared
interrupt lines) do warrant a different compatible string. Maybe this is
more of a philosophical question.
> I'm also looking at the
> code and wondering if the overhead of checking to see if the interrupt
> is flagged is really that severe, it's just a check to see if a bit is
> set in a register which we already read so should be a couple of
> instructions (which disassembly seems to confirm). It *is* overhead so
> there's some value in it, I'm just surprised that it's such a hot path
> especially with a reasonably deep FIFO like this device has.
If it was up to me, we would just add the check on BCM2835_SPI_CS_INTR
not being set and return IRQ_NONE and be done with it. I appreciate that
Lukas has spent some tremendous amount of time working on this
controller driver and he has a sensitivity for performance.
>
> I guess ideally genirq would provide a way to figure out if an interrupt
> is actually shared in the present system, and better yet we'd have a way
> for drivers to say they aren't using the interrupt ATM, but that might
> be more effort than it's really worth. If this is needed and there's no
> better way of figuring out if the interrupt is really shared then I'd
> suggest a boolean flag rather than a compatible string, it's still a
> hack but it's less likely to store up trouble for the future.
Instead of counting the number of SPI devices we culd request the
interrupt first with flags = IRQF_PROBE_SHARED, if this works, good we
have a single SPI master enabled, if it returns -EBUSY, try again with
flags = IRQF_SHARED and set-up the bcm2835_spi_sh_interrupt interrupt
handler to manage the sharing.
This would not require DT changes, which is probably better anyway.
--
Florian
^ permalink raw reply
* Re: [PATCH 0/6] arm64: dts: qcom: smmu/USB nodes and HDK855/HDK865 dts
From: Manivannan Sadhasivam @ 2020-06-04 15:58 UTC (permalink / raw)
To: Jonathan Marek
Cc: linux-arm-msm, Andy Gross, Bjorn Andersson,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, Rob Herring
In-Reply-To: <200d1f60-781b-51c7-1a38-c955f59919de@marek.ca>
On Thu, Jun 04, 2020 at 10:06:19AM -0400, Jonathan Marek wrote:
> On 6/4/20 9:52 AM, Manivannan Sadhasivam wrote:
> > Hi,
> >
> > On Sat, May 23, 2020 at 10:38:06PM -0400, Jonathan Marek wrote:
> > > Add dts nodes for apps_smmu and USB for both sm8150 and sm8250.
> > >
> >
> > I've tested this series on an SM8250 based board and able to get Type C (USB0)
> > working. There are also couple of Type A ports (USB1) on that board behind a
> > USB hub. It is probing fine but I don't see any activity while connecting a
> > USB device. Will continue to debug and once I get them working, I'll add my
> > Tested-by tag.
> >
>
> HDK865 also has a couple Type A ports, I am using them with devices already
> plugged in during boot and I haven't hit a problem like that, but I think
> I've seen the same issue when hotplugging. IIRC the behavior was a bit
> weird, like plugging a device in the Type A port (USB1) nothing would
> happen, but unplugging/replugging the type C port (USB0) would cause the
> Type A port device to start working..
>
> Have you tried with the devices already plugged in before booting?
>
Tried it but no luck :/ Also plugging and removing Type C doesn't make any
difference.
Thanks,
Mani
> > Thanks,
> > Mani
> >
> > > Also add initial dts files for HDK855 and HDK865, based on mtp dts, with a
> > > few changes. Notably, the HDK865 dts has regulator config changed a bit based
> > > on downstream (I think sm8250-mtp.dts is wrong and copied too much from sm8150).
> > >
> > > Jonathan Marek (6):
> > > arm64: dts: qcom: sm8150: add apps_smmu node
> > > arm64: dts: qcom: sm8250: add apps_smmu node
> > > arm64: dts: qcom: sm8150: Add secondary USB and PHY nodes
> > > arm64: dts: qcom: sm8250: Add USB and PHY device nodes
> > > arm64: dts: qcom: add sm8150 hdk dts
> > > arm64: dts: qcom: add sm8250 hdk dts
> > >
> > > arch/arm64/boot/dts/qcom/Makefile | 2 +
> > > arch/arm64/boot/dts/qcom/sm8150-hdk.dts | 461 ++++++++++++++++++++++++
> > > arch/arm64/boot/dts/qcom/sm8150.dtsi | 180 +++++++++
> > > arch/arm64/boot/dts/qcom/sm8250-hdk.dts | 454 +++++++++++++++++++++++
> > > arch/arm64/boot/dts/qcom/sm8250.dtsi | 287 +++++++++++++++
> > > 5 files changed, 1384 insertions(+)
> > > create mode 100644 arch/arm64/boot/dts/qcom/sm8150-hdk.dts
> > > create mode 100644 arch/arm64/boot/dts/qcom/sm8250-hdk.dts
> > >
> > > --
> > > 2.26.1
> > >
^ permalink raw reply
* Re: [PATCH v2 1/2] arm64: dts: Add a device tree for the Librem 5 phone
From: Mark Brown @ 2020-06-04 15:48 UTC (permalink / raw)
To: Martin Kepplinger
Cc: robh, kernel, shawnguo, s.hauer, kernel, festevam, linux-imx,
mchehab, Anson.Huang, agx, angus, devicetree, Daniel Baluta,
linux-kernel, linux-arm-kernel
In-Reply-To: <20200604084756.586-1-martin.kepplinger@puri.sm>
[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]
On Thu, Jun 04, 2020 at 10:47:55AM +0200, Martin Kepplinger wrote:
> + reg_audio_pwr_en: regulator-audio-pwr-en {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_audiopwr>;
> + regulator-name = "AUDIO_PWR_EN";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + gpio = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + reg_aud_1v8: regulator-audio-1v8 {
> + compatible = "regulator-fixed";
> + regulator-name = "AUD_1V8";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + vin-supply = <®_audio_pwr_en>;
> + };
This looks broken - a combination of the structure, lack of any
references to reg_audio_pwr_en and the naming suggests that you have one
regulator here for a supply called AUD_1V8 which has a GPIO controlling
the enable. I can't figure out any reason for reg_audio_pwr_en.
> + sound {
> + compatible = "simple-audio-card";
> + simple-audio-card,name = "wm8962";
You might want to put a more user friendly display name here (eg, one
mentioning Librem 5) - some UIs will show this string to users.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC] dt-bindings: mailbox: add doorbell support to ARM MHU
From: Jassi Brar @ 2020-06-04 15:15 UTC (permalink / raw)
To: Sudeep Holla
Cc: Viresh Kumar, Rob Herring, Arnd Bergmann, Frank Rowand,
Bjorn Andersson, Vincent Guittot, linux-arm-kernel,
Devicetree List, Linux Kernel Mailing List
In-Reply-To: <20200604092052.GD8814@bogus>
On Thu, Jun 4, 2020 at 4:20 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Wed, Jun 03, 2020 at 01:32:42PM -0500, Jassi Brar wrote:
> > On Wed, Jun 3, 2020 at 1:04 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > On Fri, May 29, 2020 at 09:37:58AM +0530, Viresh Kumar wrote:
> > > > On 28-05-20, 13:20, Rob Herring wrote:
> > > > > Whether Linux
> > > > > requires serializing mailbox accesses is a separate issue. On that side,
> > > > > it seems silly to not allow driving the h/w in the most efficient way
> > > > > possible.
> > > >
> > > > That's exactly what we are trying to say. The hardware allows us to
> > > > write all 32 bits in parallel, without any hardware issues, why
> > > > shouldn't we do that ? The delay (which Sudeep will find out, he is
> > > > facing issues with hardware access because of lockdown right now)
> > >
> > > OK, I was able to access the setup today. I couldn't reach a point
> > > where I can do measurements as the system just became unusable with
> > > one physical channel instead of 2 virtual channels as in my patches.
> > >
> > > My test was simple. Switch to schedutil and read sensors periodically
> > > via sysfs.
> > >
> > > arm-scmi firmware:scmi: message for 1 is not expected!
> > >
> > This sounds like you are not serialising requests on a shared channel.
> > Can you please also share the patch?
>
> OK, I did try with a small patch initially and then realised we must hit
> issue with mainline as is. Tried and the behaviour is exact same. All
> I did is removed my patches and use bit[0] as the signal. It doesn't
> matter as writes to the register are now serialised. Oh, the above
> message comes when OS times out in advance while firmware continues to
> process the old request and respond.
>
> The trace I sent gives much better view of what's going on.
>
BTW, you didn't even share 'good' vs 'bad' log for me to actually see
if the api lacks.
Let us look closely ...
>> bash-1019 [005] 1149.452340: scmi_xfer_begin:
transfer_id=1537 msg_id=7 protocol_id=19 seq=0 poll=1
>> bash-1019 [005] 1149.452407: scmi_xfer_end:
transfer_id=1537 msg_id=7 protocol_id=19 seq=0 status=0
>
This round trip took 67usecs. (log shows some at even 3usecs)
That includes mailbox api overhead, memcpy and the time taken by
remote to respond.
So the api is definitely capable of much faster transfers than you require.
>> bash-1526 [000] 1149.472553: scmi_xfer_begin: transfer_id=1538 msg_id=6 protocol_id=21 seq=0 poll=0
>> <idle>-0 [001] 1149.472733: scmi_xfer_begin: transfer_id=1539 msg_id=7 protocol_id=19 seq=1 poll=1
>
Here another request is started before the first is finished.
If you want this to work you have to serialise access to the single
physical channel and/or run the remote firmware in asynchronous mode -
that is, the firmware ack the bit as soon as it sees and starts
working in the background, so that we return in ~3usec always, while
the data returns whenever it is ready. Again, I don't have the code
or the 'good' run log to compare.
PS: I feel it is probably less effort for me to simply let the patch
through, than use my reiki powers to imagine how your test code and
firmware looks like.
^ 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