* Re: [PATCH 2/3] arm64: dts: renesas: r8a77995: Add VIN4
From: jacopo mondi @ 2018-05-13 18:30 UTC (permalink / raw)
To: Simon Horman
Cc: Niklas Söderlund, Jacopo Mondi, laurent.pinchart, geert,
magnus.damm, robh+dt, linux-renesas-soc, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <20180511134516.omsv25i2wi4cxypc@verge.net.au>
Hi Simon,
On Fri, May 11, 2018 at 03:45:16PM +0200, Simon Horman wrote:
> On Fri, May 11, 2018 at 01:25:23PM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2018-05-11 12:00:01 +0200, Jacopo Mondi wrote:
> > > Describe VIN4 interface for R-Car D3 R8A77995 SoC.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >
> > > ---
> > > arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
> > > 1 file changed, 11 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > > index 82aed7e..bdf7017 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > > @@ -783,6 +783,17 @@
> > > };
> > > };
> > > };
> > > +
> > > + vin4: video@e6ef4000 {
> > > + compatible = "renesas,vin-r8a77995";
> > > + reg = <0 0xe6ef4000 0 0x1000>;
> > > + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > > + clocks = <&cpg CPG_MOD 807>;
> > > + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> > > + resets = <&cpg 807>;
> > > + renesas,id = <4>;
> > > + status = "disabled";
> > > + };
> > > };
>
> Thanks, I have moved the new node to preserve sorting of nodes by bus
> address and applied the result. It is as follows:
Great, thanks for doing this, I should have take care of sorting nodes
opprtunely.
Thanks
j
>
> From: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Subject: [PATCH] arm64: dts: renesas: r8a77995: Add VIN4
>
> Describe VIN4 interface for R-Car D3 R8A77995 SoC.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> [simon: sorted node by bus address]
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> index ba98865b0c9b..2506f46293e8 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> @@ -610,6 +610,17 @@
> status = "disabled";
> };
s
> + vin4: video@e6ef4000 {
> + compatible = "renesas,vin-r8a77995";
> + reg = <0 0xe6ef4000 0 0x1000>;
> + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD 807>;
> + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> + resets = <&cpg 807>;
> + renesas,id = <4>;
> + status = "disabled";
> + };
> +
> ohci0: usb@ee080000 {
> compatible = "generic-ohci";
> reg = <0 0xee080000 0 0x100>;
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH 6/8] serial: Add Tegra Combined UART driver
From: Mikko Perttunen @ 2018-05-13 18:06 UTC (permalink / raw)
To: Jassi Brar, Mikko Perttunen
Cc: Rob Herring, Mark Rutland, Greg KH, Thierry Reding, Jon Hunter,
araza, Devicetree List, linux-serial, linux-tegra,
", linux-arm-kernel", linux-mediatek, srv_heupstream,
Linux Kernel Mailing List
In-Reply-To: <CABb+yY3snB9o4e14by4xui+o_Vhqpe2zh6Pp_e0t-DCTKvMHWA@mail.gmail.com>
On 05/13/2018 06:36 PM, Jassi Brar wrote:
> On Tue, May 8, 2018 at 5:14 PM, Mikko Perttunen <mperttunen@nvidia.com> wrote:
>
> ....
>>
>> +config SERIAL_TEGRA_TCU
>> + tristate "NVIDIA Tegra Combined UART"
>> + depends on ARCH_TEGRA && MAILBOX
>> + select SERIAL_CORE
>> + help
>> + Support for the mailbox-based TCU (Tegra Combined UART) serial port.
>> + TCU is a virtual serial port that allows multiplexing multiple data
>> + streams into a single hardware serial port.
>> +
> Maybe make it depend upon TEGRA_HSP_MBOX ?
Yeah, that probably makes more sense. MAILBOX is enough to build it but
it won't be of any use without TEGRA_HSP_MBOX.
>
> ......
>
>> +
>> +static void tegra_tcu_write(const char *s, unsigned int count)
>> +{
>> + struct tegra_tcu *tcu = tegra_tcu_uart_port.private_data;
>> + unsigned int written = 0, i = 0;
>> + bool insert_nl = false;
>> + uint32_t value = 0;
>> +
>> + while (i < count) {
>> + if (insert_nl) {
>> + value |= '\n' << (written++ * 8);
>> + insert_nl = false;
>> + i++;
>> + } else if (s[i] == '\n') {
>> + value |= '\r' << (written++ * 8);
>> + insert_nl = true;
>> + } else {
>> + value |= s[i++] << (written++ * 8);
>> + }
>> +
>> + if (written == 3) {
>> + value |= 3 << 24;
>> + value |= BIT(26);
>> + mbox_send_message(tcu->tx, &value);
>>
> How is this supposed to work? tegra_hsp_doorbell_send_data() ignores
> the second argument.
The previous patch in the series adds support for what are called
"shared mailboxes" to the tegra-hsp driver. For these the second
argument is used.
Thanks,
Mikko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 6/8] serial: Add Tegra Combined UART driver
From: Mikko Perttunen @ 2018-05-13 18:04 UTC (permalink / raw)
To: Andy Shevchenko, Mikko Perttunen
Cc: Rob Herring, Mark Rutland, Jassi Brar, Greg Kroah-Hartman,
Thierry Reding, Jon Hunter, araza, devicetree,
open list:SERIAL DRIVERS, linux-tegra, linux-arm Mailing List,
Linux Kernel Mailing List
In-Reply-To: <CAHp75VfYF_u2bZ+temGbReEB9K-t3A75aUNYwffyv9-=zEi0Lw@mail.gmail.com>
On 05/13/2018 05:16 PM, Andy Shevchenko wrote:
> On Tue, May 8, 2018 at 2:44 PM, Mikko Perttunen <mperttunen@nvidia.com> wrote:
>> The Tegra Combined UART (TCU) is a mailbox-based mechanism that allows
>> multiplexing multiple "virtual UARTs" into a single hardware serial
>> port. The TCU is the primary serial port on Tegra194 devices.
>>
>> Add a TCU driver utilizing the mailbox framework, as the used mailboxes
>> are part of Tegra HSP blocks that are already controlled by the Tegra
>> HSP mailbox driver.
>
> First question, can it be done utilizing SERDEV framework?
Based on some brief research, the SERDEV framework is for devices that
are behind some UART interface. In this case, this driver implements the
UART interface itself, so by my understanding SERDEV is not appropriate.
Please correct me if I'm wrong.
>
>> +static void tegra_tcu_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
>> +{
>
>> + (void)port;
>> + (void)mctrl;
>
> Huh?
The serial core calls these callbacks without checking if they are set.
They don't make sense for this driver so they are stubbed out.
>
>> +}
>
>> +static void tegra_tcu_uart_stop_tx(struct uart_port *port)
>> +{
>> + (void)port;
>> +}
>
> Ditto.
>
>> + if (written == 3) {
>> + value |= 3 << 24;
>> + value |= BIT(26);
>> + mbox_send_message(tcu->tx, &value);
>
>> + }
>
> (1)
>
>> + }
>> +
>> + if (written) {
>> + value |= written << 24;
>> + value |= BIT(26);
>> + mbox_send_message(tcu->tx, &value);
>> + }
>
> (2)
>
> These are code duplications.
Indeed - the length of the duplicated code is so short, and the
instances are so close to each other, that I don't find it necessary (or
clearer) to have an extra function.
>
>> +static void tegra_tcu_uart_stop_rx(struct uart_port *port)
>> +{
>> + (void)port;
>> +}
>> +
>> +static void tegra_tcu_uart_break_ctl(struct uart_port *port, int ctl)
>> +{
>> + (void)port;
>> + (void)ctl;
>> +}
>> +
>> +static int tegra_tcu_uart_startup(struct uart_port *port)
>> +{
>> + (void)port;
>> +
>> + return 0;
>> +}
>> +
>> +static void tegra_tcu_uart_shutdown(struct uart_port *port)
>> +{
>> + (void)port;
>> +}
>> +
>> +static void tegra_tcu_uart_set_termios(struct uart_port *port,
>> + struct ktermios *new,
>> + struct ktermios *old)
>> +{
>> + (void)port;
>> + (void)new;
>> + (void)old;
>> +}
>
> Remove those unused stub contents.
Sure. I had these here so that we don't get unused parameter warnings,
but I can just as well remove the parameter names.
>
>> + return uart_set_options(&tegra_tcu_uart_port, cons,
>> + 115200, 'n', 8, 'n');
>
> Can't it be one line?
It would be a total of 81 characters in length on one line, so no.
>
>> +static void tegra_tcu_receive(struct mbox_client *client, void *msg_p)
>> +{
>> + struct tty_port *port = &tegra_tcu_uart_port.state->port;
>
>> + uint32_t msg = *(uint32_t *)msg_p;
>
> Redundant casting.
Will remove.
>
>> + unsigned int num_bytes;
>> + int i;
>> +
>
>> + num_bytes = (msg >> 24) & 0x3;
>
> Two magic numbers.
Sure, will add defines.
>
>> + for (i = 0; i < num_bytes; i++)
>> + tty_insert_flip_char(port, (msg >> (i*8)) & 0xff, TTY_NORMAL);
>> +
>> + tty_flip_buffer_push(port);
>> +}
>
>> +MODULE_AUTHOR("Mikko Perttunen <mperttunen@nvidia.com>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("NVIDIA Tegra Combined UART driver");
>> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
>> index dce5f9dae121..eaf3c303cba6 100644
>> --- a/include/uapi/linux/serial_core.h
>> +++ b/include/uapi/linux/serial_core.h
>> @@ -281,4 +281,7 @@
>> /* MediaTek BTIF */
>> #define PORT_MTK_BTIF 117
>>
>> +/* NVIDIA Tegra Combined UART */
>> +#define PORT_TEGRA_TCU 118
>
> Check if there is an unused gap. IIRC we still have one near to 40ish.
>
Correct, looks like 41-43 are unused. I'll change this 41.
Thanks for reviewing!
Mikko
^ permalink raw reply
* Re: [PATCH 6/8] serial: Add Tegra Combined UART driver
From: Jassi Brar @ 2018-05-13 15:36 UTC (permalink / raw)
To: Mikko Perttunen
Cc: Rob Herring, Mark Rutland, Greg KH, Thierry Reding, Jon Hunter,
araza, Devicetree List, linux-serial, linux-tegra,
, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, srv_heupstream,
Linux Kernel Mailing List
In-Reply-To: <20180508114403.14499-7-mperttunen@nvidia.com>
On Tue, May 8, 2018 at 5:14 PM, Mikko Perttunen <mperttunen@nvidia.com> wrote:
....
>
> +config SERIAL_TEGRA_TCU
> + tristate "NVIDIA Tegra Combined UART"
> + depends on ARCH_TEGRA && MAILBOX
> + select SERIAL_CORE
> + help
> + Support for the mailbox-based TCU (Tegra Combined UART) serial port.
> + TCU is a virtual serial port that allows multiplexing multiple data
> + streams into a single hardware serial port.
> +
Maybe make it depend upon TEGRA_HSP_MBOX ?
......
> +
> +static void tegra_tcu_write(const char *s, unsigned int count)
> +{
> + struct tegra_tcu *tcu = tegra_tcu_uart_port.private_data;
> + unsigned int written = 0, i = 0;
> + bool insert_nl = false;
> + uint32_t value = 0;
> +
> + while (i < count) {
> + if (insert_nl) {
> + value |= '\n' << (written++ * 8);
> + insert_nl = false;
> + i++;
> + } else if (s[i] == '\n') {
> + value |= '\r' << (written++ * 8);
> + insert_nl = true;
> + } else {
> + value |= s[i++] << (written++ * 8);
> + }
> +
> + if (written == 3) {
> + value |= 3 << 24;
> + value |= BIT(26);
> + mbox_send_message(tcu->tx, &value);
>
How is this supposed to work? tegra_hsp_doorbell_send_data() ignores
the second argument.
^ permalink raw reply
* Re: [PATCH v1 11/13] dt-bindings: power: add PX30 SoCs header for power-domain
From: Tao Huang @ 2018-05-13 15:18 UTC (permalink / raw)
To: Heiko Stuebner, Elaine Zhang
Cc: mark.rutland, devicetree, ulf.hansson, khilman, xxx, linux-pm,
rjw, linux-kernel, xf, linux-rockchip, robh+dt, Finley Xiao,
linux-arm-kernel, wxt
In-Reply-To: <508694354.7btLQf7nsA@phil>
Hi Heiko:
On 2018年05月12日 06:11, Heiko Stuebner wrote:
> Here I have a naming question. When looking at the vendor kernel
> it looks like the px30 is largely related to the rk3326.
> (rk3326.dtsi includeing the px30.dtsi)
>
> What is the reason for basing the naming on the px30 this time? And could
> we possibly keep to rkXXXX names for the basic things in the kernel, thus
> keeping the pxXX as second name, like with the other px-variants before?
>
PX30 and RK3326 are different chips. PX30 has more features. You can simply think that RK3326 is a subset of PX30. The RK3326 is more like a PX30 derivative chip. This is not the same as the previous chips.
So we use PX30 instead of RK3326 for name, and the opening document is only for PX30, we think this is more convenient for developers.
Best Regards,
Tao Huang
^ permalink raw reply
* Re: [PATCH v3] Input: add bu21029 touch driver
From: Andy Shevchenko @ 2018-05-13 14:56 UTC (permalink / raw)
To: Mark Jonas
Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, linux-input,
devicetree, Linux Kernel Mailing List, Heiko Schocher, Zhu Yi
In-Reply-To: <1526048528-3613-1-git-send-email-mark.jonas@de.bosch.com>
On Fri, May 11, 2018 at 5:22 PM, Mark Jonas <mark.jonas@de.bosch.com> wrote:
> Add Rohm BU21029 resistive touch panel controller support with I2C
> interface.
> +#include <linux/of.h>
This becomes redundant (see below).
> +#define STOP_DELAY_US 50L
> +#define START_DELAY_MS 2L
> +#define BUF_LEN 8L
No need to use L for such small numbers. Integer promotion is a part
of C standard.
> +#define SCALE_12BIT (1 << 12)
> +#define MAX_12BIT ((1 << 12) - 1)
BIT(12)
GENMASK(11, 0)
> +static int bu21029_touch_report(struct bu21029_ts_data *bu21029)
> +{
> + struct i2c_client *i2c = bu21029->client;
> + u8 buf[BUF_LEN];
> + int error = bu21029_touch_report(bu21029);
> +
Redundant empty line.
> + if (error) {
> + dev_err(&i2c->dev, "failed to report (error: %d)\n", error);
Potential spamming case.
> + return IRQ_NONE;
> + }
> +static void bu21029_stop_chip(struct input_dev *dev)
> +{
> + struct bu21029_ts_data *bu21029 = input_get_drvdata(dev);
> +
> + disable_irq(bu21029->client->irq);
> + del_timer_sync(&bu21029->timer);
> +
> + /* put chip into reset */
> + gpiod_set_value_cansleep(bu21029->reset_gpios, 1);
> + udelay(STOP_DELAY_US);
udelay() ?!
> +}
> +
> +static int bu21029_start_chip(struct input_dev *dev)
> +{
> + u16 hwid;
> +
> + /* take chip out of reset */
> + gpiod_set_value_cansleep(bu21029->reset_gpios, 0);
> + mdelay(START_DELAY_MS);
mdelay()?!
> +
> + error = i2c_smbus_read_i2c_block_data(i2c,
> + BU21029_HWID_REG,
> + 2,
> + (u8 *)&hwid);
> + if (error < 0) {
> + dev_err(&i2c->dev, "failed to read HW ID\n");
> + goto out;
> + }
> +
> + if (cpu_to_be16(hwid) != SUPPORTED_HWID) {
Hmm... Why cpu_to_be16() is required?
> + dev_err(&i2c->dev, "unsupported HW ID 0x%x\n", hwid);
> + error = -ENODEV;
> + goto out;
> + }
> +}
> +static int bu21029_parse_dt(struct bu21029_ts_data *bu21029)
You can get rid of DT requirement by...
> +{
> + struct device *dev = &bu21029->client->dev;
> + struct device_node *np = dev->of_node;
> + u32 val32;
> + int error;
> + if (!np) {
> + dev_err(dev, "no device tree data\n");
> + return -EINVAL;
> + }
(this becomes redundant)
> +
> + bu21029->reset_gpios = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(bu21029->reset_gpios)) {
> + error = PTR_ERR(bu21029->reset_gpios);
> + if (error != -EPROBE_DEFER)
> + dev_err(dev, "invalid 'reset-gpios':%d\n", error);
> + return error;
> + }
> +
> + if (of_property_read_u32(np, "rohm,x-plate-ohms", &val32)) {
...simple calling device_property_read_u32() instead.
> + dev_err(dev, "invalid 'x-plate-ohms' supplied\n");
> + return -EINVAL;
> + }
> + bu21029->x_plate_ohms = val32;
> +
> + touchscreen_parse_properties(bu21029->in_dev, false, &bu21029->prop);
> +
> + return 0;
> +}
> +#ifdef CONFIG_PM_SLEEP
Instead...
> +static int bu21029_suspend(struct device *dev)
...use __maby_unused annotation.
> +static int bu21029_resume(struct device *dev)
Ditto.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 6/8] serial: Add Tegra Combined UART driver
From: Andy Shevchenko @ 2018-05-13 14:16 UTC (permalink / raw)
To: Mikko Perttunen
Cc: Rob Herring, Mark Rutland, Jassi Brar, Greg Kroah-Hartman,
Thierry Reding, Jon Hunter, araza, devicetree,
open list:SERIAL DRIVERS, linux-tegra, linux-arm Mailing List,
Linux Kernel Mailing List
In-Reply-To: <20180508114403.14499-7-mperttunen@nvidia.com>
On Tue, May 8, 2018 at 2:44 PM, Mikko Perttunen <mperttunen@nvidia.com> wrote:
> The Tegra Combined UART (TCU) is a mailbox-based mechanism that allows
> multiplexing multiple "virtual UARTs" into a single hardware serial
> port. The TCU is the primary serial port on Tegra194 devices.
>
> Add a TCU driver utilizing the mailbox framework, as the used mailboxes
> are part of Tegra HSP blocks that are already controlled by the Tegra
> HSP mailbox driver.
First question, can it be done utilizing SERDEV framework?
> +static void tegra_tcu_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> + (void)port;
> + (void)mctrl;
Huh?
> +}
> +static void tegra_tcu_uart_stop_tx(struct uart_port *port)
> +{
> + (void)port;
> +}
Ditto.
> + if (written == 3) {
> + value |= 3 << 24;
> + value |= BIT(26);
> + mbox_send_message(tcu->tx, &value);
> + }
(1)
> + }
> +
> + if (written) {
> + value |= written << 24;
> + value |= BIT(26);
> + mbox_send_message(tcu->tx, &value);
> + }
(2)
These are code duplications.
> +static void tegra_tcu_uart_stop_rx(struct uart_port *port)
> +{
> + (void)port;
> +}
> +
> +static void tegra_tcu_uart_break_ctl(struct uart_port *port, int ctl)
> +{
> + (void)port;
> + (void)ctl;
> +}
> +
> +static int tegra_tcu_uart_startup(struct uart_port *port)
> +{
> + (void)port;
> +
> + return 0;
> +}
> +
> +static void tegra_tcu_uart_shutdown(struct uart_port *port)
> +{
> + (void)port;
> +}
> +
> +static void tegra_tcu_uart_set_termios(struct uart_port *port,
> + struct ktermios *new,
> + struct ktermios *old)
> +{
> + (void)port;
> + (void)new;
> + (void)old;
> +}
Remove those unused stub contents.
> + return uart_set_options(&tegra_tcu_uart_port, cons,
> + 115200, 'n', 8, 'n');
Can't it be one line?
> +static void tegra_tcu_receive(struct mbox_client *client, void *msg_p)
> +{
> + struct tty_port *port = &tegra_tcu_uart_port.state->port;
> + uint32_t msg = *(uint32_t *)msg_p;
Redundant casting.
> + unsigned int num_bytes;
> + int i;
> +
> + num_bytes = (msg >> 24) & 0x3;
Two magic numbers.
> + for (i = 0; i < num_bytes; i++)
> + tty_insert_flip_char(port, (msg >> (i*8)) & 0xff, TTY_NORMAL);
> +
> + tty_flip_buffer_push(port);
> +}
> +MODULE_AUTHOR("Mikko Perttunen <mperttunen@nvidia.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("NVIDIA Tegra Combined UART driver");
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index dce5f9dae121..eaf3c303cba6 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -281,4 +281,7 @@
> /* MediaTek BTIF */
> #define PORT_MTK_BTIF 117
>
> +/* NVIDIA Tegra Combined UART */
> +#define PORT_TEGRA_TCU 118
Check if there is an unused gap. IIRC we still have one near to 40ish.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi
From: Andy Shevchenko @ 2018-05-13 13:35 UTC (permalink / raw)
To: Radu Pirea
Cc: devicetree, open list:SERIAL DRIVERS, Linux Kernel Mailing List,
linux-arm Mailing List, linux-spi, Mark Rutland, Rob Herring,
Lee Jones, Greg Kroah-Hartman, Jiri Slaby, Richard Genoud,
alexandre.belloni, Nicolas Ferre, Mark Brown
In-Reply-To: <CAHp75Ve3Ugnjjm8EZkPQTZSvH1qad1e5SqjOn8zz5syHSQea_g@mail.gmail.com>
> I will refer to above as (1) later on.
> The question is, why you didn't utilize what SPI core provides you?
Here I should have referred to (1).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi
From: Andy Shevchenko @ 2018-05-13 13:33 UTC (permalink / raw)
To: Radu Pirea
Cc: devicetree, open list:SERIAL DRIVERS, Linux Kernel Mailing List,
linux-arm Mailing List, linux-spi, Mark Rutland, Rob Herring,
Lee Jones, Greg Kroah-Hartman, Jiri Slaby, Richard Genoud,
alexandre.belloni, Nicolas Ferre, Mark Brown
In-Reply-To: <20180511103822.31698-6-radu.pirea@microchip.com>
On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com> wrote:
> This is the driver for at91-usart in spi mode. The USART IP can be configured
> to work in many modes and one of them is SPI.
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
Here is something wrong. You need to use latter one in new code.
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
Hmm... Do you need all of them?
> +static inline void at91_usart_spi_cs_activate(struct spi_device *spi)
> +{
...
> + gpiod_set_value(ausd->npcs_pin, active);
> + aus->cs_active = true;
> +}
> +
> +static inline void at91_usart_spi_cs_deactivate(struct spi_device *spi)
> +{
...
> + gpiod_set_value(ausd->npcs_pin, !active);
> + aus->cs_active = false;
> +}
...
> + if (!ausd) {
> + if (gpio_is_valid(spi->cs_gpio)) {
> + npcs_pin = gpio_to_desc(spi->cs_gpio);
...
> + }
...
> + gpiod_direction_output(npcs_pin, !(spi->mode & SPI_CS_HIGH));
> +
> + ausd->npcs_pin = npcs_pin;
...
> + }
I will refer to above as (1) later on.
> + dev_dbg(&spi->dev, "new message %p submitted for %s\n",
> + msg, dev_name(&spi->dev));
%p does make a very little sense.
> + list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> + ret = at91_usart_spi_one_transfer(controller, msg, xfer);
> + if (ret)
> + goto msg_done;
> + }
Cant SPI core do this for your?
> +static void at91_usart_spi_cleanup(struct spi_device *spi)
> +{
> + struct at91_usart_spi_device *ausd = spi->controller_state;
> +
> + if (!ausd)
> + return;
Is it even possible?
Anyway the code below will work fine even if it's the case.
> +
> + spi->controller_state = NULL;
> + kfree(ausd);
> +}
> +static int at91_usart_spi_gpio_cs(struct platform_device *pdev)
> +{
> + struct spi_controller *controller = platform_get_drvdata(pdev);
> + struct device_node *np = controller->dev.parent->of_node;
> + struct gpio_desc *cs_gpio;
> + int nb;
> + int i;
> +
> + if (!np)
> + return 0;
> +
> + nb = of_gpio_named_count(np, "cs-gpios");
> + for (i = 0; i < nb; i++) {
> + cs_gpio = devm_gpiod_get_from_of_node(&pdev->dev,
> + pdev->dev.parent->of_node,
> + "cs-gpios",
> + i, GPIOD_OUT_HIGH,
> + dev_name(&pdev->dev));
> + if (IS_ERR(cs_gpio))
> + return PTR_ERR(cs_gpio);
> + }
> +
> + controller->num_chipselect = nb;
> +
> + return 0;
> +}
The question is, why you didn't utilize what SPI core provides you?
> + spi_writel(aus, MR, US_MR_SPI_MASTER | US_MR_CHRL | US_MR_CLKO |
> + US_MR_WRDBT);
> + spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS | US_CR_RSTRX |
> + US_CR_RSTTX);
I didn't check over, but it seems like you might have duplication in
these bitwise ORs. Consider to unify them into another (shorter)
definitions and reuse all over the code.
> + regs = platform_get_resource(to_platform_device(pdev->dev.parent),
> + IORESOURCE_MEM, 0);
> + if (!regs)
> + return -ENXIO;
Strange error code for getting MMIO resource. ENOMEM sounds better.
> + dev_info(&pdev->dev,
> + "Atmel USART SPI Controller version 0x%x at 0x%08lx (irq %d)\n",
> + spi_readl(aus, VERSION),
> + (unsigned long)regs->start, irq);
If you do explicit casting when printing something you are doing wrong.
Please use %pR or %pr in this case.
> +static struct platform_driver at91_usart_spi_driver = {
> + .driver = {
> + .name = "at91_usart_spi",
> + .of_match_table = of_match_ptr(at91_usart_spi_dt_ids),
Can it work as pure platform driver? If no, of_match_ptr() is redundant.
> + },
> + .probe = at91_usart_spi_probe,
> + .remove = at91_usart_spi_remove, };
Two lines at one. Split.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 3/6] MAINTAINERS: add at91 usart spi driver
From: Andy Shevchenko @ 2018-05-13 13:14 UTC (permalink / raw)
To: Radu Pirea
Cc: devicetree, open list:SERIAL DRIVERS, Linux Kernel Mailing List,
linux-arm Mailing List, linux-spi, Mark Rutland, Rob Herring,
Lee Jones, Greg Kroah-Hartman, Jiri Slaby, Richard Genoud,
alexandre.belloni, Nicolas Ferre, Mark Brown
In-Reply-To: <20180511103822.31698-4-radu.pirea@microchip.com>
On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com> wrote:
> Added entry for at91 usart mfd driver.
>
You are adding a record for not existing file?
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
> MAINTAINERS | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ca06c6f58299..9243b9007966 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9199,6 +9199,13 @@ S: Supported
> F: drivers/mfd/at91-usart.c
> F: include/dt-bindings/mfd/at91-usart.h
>
> +MICROCHIP AT91 USART SPI DRIVER
> +M: Radu Pirea <radu.pirea@microchip.com>
> +L: linux-spi@vger.kernel.org
> +S: Supported
> +F: drivers/spi/spi-at91-usart.c
> +F: Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
> +
> MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
> M: Woojung Huh <Woojung.Huh@microchip.com>
> M: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> --
> 2.17.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] media: rcar-vin: Drop unnecessary register properties from example vin port
From: Niklas Söderlund @ 2018-05-13 13:11 UTC (permalink / raw)
To: Simon Horman
Cc: Mark Rutland, devicetree, Magnus Damm, linux-renesas-soc,
Rob Herring, Geert Uytterhoeven, Mauro Carvalho Chehab,
linux-arm-kernel, linux-media
In-Reply-To: <20180509184558.14960-1-horms+renesas@verge.net.au>
Hi Simon,
Thanks for your patch.
On 2018-05-09 20:45:58 +0200, Simon Horman wrote:
> The example vin port node does not have an address and thus does not
> need address-cells or address size-properties.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> Documentation/devicetree/bindings/media/rcar_vin.txt | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1c669..2a0c59e97f40 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -107,9 +107,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> status = "okay";
>
> port {
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> vin1ep0: endpoint {
> remote-endpoint = <&adv7180>;
> bus-width = <8>;
> --
> 2.11.0
>
--
Regards,
Niklas Söderlund
^ permalink raw reply
* Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input
From: Niklas Söderlund @ 2018-05-13 12:57 UTC (permalink / raw)
To: Jacopo Mondi
Cc: laurent.pinchart, horms, geert, magnus.damm, robh+dt,
linux-renesas-soc, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1526032802-14376-4-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thanks for your patch.
On 2018-05-11 12:00:02 +0200, Jacopo Mondi wrote:
> Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> development board.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 ++++++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> index d03f194..e0ce462 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> @@ -59,6 +59,17 @@
> };
> };
>
> + hdmi-in {
> + compatible = "hdmi-connector";
> + type = "a";
> +
> + port {
> + hdmi_con_in: endpoint {
> + remote-endpoint = <&adv7612_in>;
> + };
> + };
> + };
> +
> memory@48000000 {
> device_type = "memory";
> /* first 128MB is reserved for secure area. */
> @@ -142,6 +153,11 @@
> groups = "usb0";
> function = "usb0";
> };
> +
> + vin4_pins: vin4 {
> + groups = "vin4_data24", "vin4_sync", "vin4_clk", "vin4_clkenb";
> + function = "vin4";
> + };
> };
>
> &i2c0 {
> @@ -154,6 +170,35 @@
> reg = <0x50>;
> pagesize = <8>;
> };
> +
> + hdmi-decoder@4c {
> + compatible = "adi,adv7612";
> + reg = <0x4c>;
> + default-input = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + adv7612_in: endpoint {
> + remote-endpoint = <&hdmi_con_in>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> + adv7612_out: endpoint {
> + pclk-sample = <0>;
> + hsync-active = <0>;
> + vsync-active = <0>;
This differs from the Gen2 DT bindings which is a very similar hardware
setup using the same components. Defining these properties will make the
bus marked as V4L2_MBUS_PARALLEL instead of V4L2_MBUS_BT656.
This will change how the hardware is configured for capture if the media
bus is in a UYVY format, see VNMC_INF register in rvin_setup(). Maybe
this it not an issue here but still I'm curious to why this differ
between Gen2 and Gen3 :-)
> +
> + remote-endpoint = <&vin4_in>;
> + };
> + };
> + };
> + };
> };
>
> &i2c1 {
> @@ -246,3 +291,26 @@
> timeout-sec = <60>;
> status = "okay";
> };
> +
> +&vin4 {
> + pinctrl-0 = <&vin4_pins>;
> + pinctrl-names = "default";
> +
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + vin4_in: endpoint {
> + hsync-active = <0>;
> + vsync-active = <0>;
Comparing this to the Gen2 bindings some properties are missing,
bus-width = <24>;
pclk-sample = <1>;
data-active = <1>;
This is not a big deal as the VIN driver don't use these properties so
no functional change should come of this but still a difference.
Over all I'm happy with this change but before I add my tag I would like
to understand why it differs from the Gen2 configuration for the adv7612
properties.
Also on a side not it is possible with hardware switches on the board
switch the VIN4 source to a completely different pipeline CVBS connector
-> adv7180 -> VIN4. But I think it's best we keep the HDMI as default as
this seems to be how the boards are shipped. But maybe mentioning this
in the commit message would not hurt if you end-up resending the patch.
> +
> + remote-endpoint = <&adv7612_out>;
> + };
> + };
> + };
> +};
> --
> 2.7.4
>
--
Regards,
Niklas Söderlund
^ permalink raw reply
* Re: [PATCH] ARM: dts: exynos: Use dedicated DT bindings for Odroid X/X2, U3
From: Krzysztof Kozlowski @ 2018-05-13 12:24 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: robh+dt, devicetree, linux-samsung-soc, linux-arm-kernel,
linux-kernel, b.zolnierkie, m.szyprowski
In-Reply-To: <20180511080947.3057-1-s.nawrocki@samsung.com>
On Fri, May 11, 2018 at 10:09:47AM +0200, Sylwester Nawrocki wrote:
> Use dedicated Odroid audio subsystem DT bindings instead of the simple-card.
> This adds support for audio on the HDMI interface.
How about changing the subject to "Add suppor for audio over HDMI for
Odroid X/X2/U3"?
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 33 ++++++++++++++-----------
> arch/arm/boot/dts/exynos4412-odroidu3.dts | 7 +++---
> arch/arm/boot/dts/exynos4412-odroidx.dts | 7 +++---
> 3 files changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> index d7ad07fd48f9..bad08e70358a 100644
> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> @@ -37,20 +37,12 @@
> };
>
> sound: sound {
> - compatible = "simple-audio-card";
> -
> - simple-audio-card,format = "i2s";
> - simple-audio-card,bitclock-master = <&link0_codec>;
> - simple-audio-card,frame-master = <&link0_codec>;
> -
> - simple-audio-card,cpu {
> + cpu {
> sound-dai = <&i2s0 0>;
> - system-clock-frequency = <19200000>;
> };
>
> - link0_codec: simple-audio-card,codec {
> - sound-dai = <&max98090>;
> - clocks = <&i2s0 CLK_I2S_CDCLK>;
> + codec {
> + sound-dai = <&hdmi>, <&max98090>;
> };
> };
>
> @@ -142,14 +134,25 @@
> pinctrl-0 = <>;
> };
>
> +&clock {
> + assigned-clocks = <&clock CLK_FOUT_EPLL>;
> + assigned-clock-rates = <45158401>;
> +};
> +
> &clock_audss {
> assigned-clocks = <&clock_audss EXYNOS_MOUT_AUDSS>,
> <&clock_audss EXYNOS_MOUT_I2S>,
> <&clock_audss EXYNOS_DOUT_SRP>,
> - <&clock_audss EXYNOS_DOUT_AUD_BUS>;
> + <&clock_audss EXYNOS_DOUT_AUD_BUS>,
> + <&clock_audss EXYNOS_DOUT_I2S>;
> +
> assigned-clock-parents = <&clock CLK_FOUT_EPLL>,
> - <&clock_audss EXYNOS_MOUT_AUDSS>;
> - assigned-clock-rates = <0>, <0>, <192000000>, <19200000>;
> + <&clock_audss EXYNOS_MOUT_AUDSS>;
> +
> + assigned-clock-rates = <0>, <0>,
> + <196608001>,
> + <(196608001 / 2)>,
> + <(196608001 / 8)>;
> };
>
> &cpu0 {
> @@ -498,6 +501,8 @@
> pinctrl-0 = <&i2s0_bus>;
> pinctrl-names = "default";
> status = "okay";
> + assigned-clocks = <&i2s0 CLK_I2S_RCLK_SRC>;
> + assigned-clock-parents = <&clock_audss EXYNOS_SCLK_I2S>;
> };
>
> &mixer {
> diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts
> index bdcd4523cc1c..7a94a423097d 100644
> --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
> +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
> @@ -113,11 +113,12 @@
> };
>
> &sound {
> - simple-audio-card,name = "Odroid-U3";
> - simple-audio-card,widgets =
> + compatible = "hardkernel,odroid-xu4-audio";
The compatible is the same for all users using DTSI so it could be moved
to the exynos4412-odroid-common.dtsi itself.
Best regards,
Krzysztof
> + model = "Odroid-U3";
> + samsung,audio-widgets =
> "Headphone", "Headphone Jack",
> "Speakers", "Speakers";
> - simple-audio-card,routing =
> + samsung,audio-routing =
> "Headphone Jack", "HPL",
> "Headphone Jack", "HPR",
> "Headphone Jack", "MICBIAS",
> diff --git a/arch/arm/boot/dts/exynos4412-odroidx.dts b/arch/arm/boot/dts/exynos4412-odroidx.dts
> index 2dff129bc2ad..f0d5037f3a5d 100644
> --- a/arch/arm/boot/dts/exynos4412-odroidx.dts
> +++ b/arch/arm/boot/dts/exynos4412-odroidx.dts
> @@ -97,12 +97,13 @@
> };
>
> &sound {
> - simple-audio-card,name = "Odroid-X";
> - simple-audio-card,widgets =
> + compatible = "hardkernel,odroid-xu4-audio";
> + model = "Odroid-X";
> + samsung,audio-widgets =
> "Headphone", "Headphone Jack",
> "Microphone", "Mic Jack",
> "Microphone", "DMIC";
> - simple-audio-card,routing =
> + samsung,audio-routing =
> "Headphone Jack", "HPL",
> "Headphone Jack", "HPR",
> "IN1", "Mic Jack",
> --
> 2.14.2
>
^ permalink raw reply
* Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input
From: Niklas Söderlund @ 2018-05-13 11:56 UTC (permalink / raw)
To: Simon Horman
Cc: Jacopo Mondi, laurent.pinchart, geert, magnus.damm, robh+dt,
linux-renesas-soc, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20180513081750.xcpc5ya3yl7ezkjd@verge.net.au>
Hi Simon,
On 2018-05-13 10:17:50 +0200, Simon Horman wrote:
> On Fri, May 11, 2018 at 12:00:02PM +0200, Jacopo Mondi wrote:
> > Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> > development board.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> Hi Niklas,
>
> As you reviewed the rest of the series I'm wondering if you're planning
> to review this patch too.
Yes, I did not have schematics for D3 on hand when reviewing the rest of
the series. Will review it now that I do, thanks for the ping :-)
--
Regards,
Niklas Söderlund
^ permalink raw reply
* Re: [PATCH 0/2] Disable EtherAVB by default on R8A779{7|8}0 SoCs
From: Simon Horman @ 2018-05-13 8:23 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Mark Rutland, devicetree, Magnus Damm, Catalin Marinas,
Will Deacon, linux-renesas-soc, Rob Herring, linux-arm-kernel
In-Reply-To: <5844eef7-0f5e-6366-4d3d-a3a658e7f988@cogentembedded.com>
On Fri, May 11, 2018 at 11:18:36PM +0300, Sergei Shtylyov wrote:
> Hello!
>
> Here's the set of 2 patches against Simon Horman's 'renesas.git' repo's
> 'renesas-devel-20180511-v4.17-rc4' tag. I'm fixing the issue in the EtherAVB
> device nodes in the R8A779{7|8}0 device trees that missed the "status" prop,
> usually disabling the SoC devices in anticipation that the board device trees
> enable the devices according to their needs. There should be no issues with
> the current R8A779{7|8}0 board device trees, as all of them use EtherAVB
> anyway, so I'm sending the patches generated against the 'devel' branch...
>
> [1/2] arm64: dts: renesas: r8a77970: disable EtherAVB
> [2/2] arm64: dts: renesas: r8a77980: disable EtherAVB
Thanks, applied.
^ permalink raw reply
* Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input
From: Simon Horman @ 2018-05-13 8:17 UTC (permalink / raw)
To: Jacopo Mondi
Cc: niklas.soderlund, laurent.pinchart, geert, magnus.damm, robh+dt,
linux-renesas-soc, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1526032802-14376-4-git-send-email-jacopo+renesas@jmondi.org>
On Fri, May 11, 2018 at 12:00:02PM +0200, Jacopo Mondi wrote:
> Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> development board.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Hi Niklas,
As you reviewed the rest of the series I'm wondering if you're planning
to review this patch too.
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: renesas: initial V3HSK board device tree
From: Simon Horman @ 2018-05-13 8:07 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Mark Rutland, devicetree, Magnus Damm, Catalin Marinas,
Will Deacon, linux-renesas-soc, Rob Herring, linux-arm-kernel
In-Reply-To: <50de037c-7560-c261-f96a-f86065674c9b@cogentembedded.com>
On Thu, May 10, 2018 at 09:12:30PM +0300, Sergei Shtylyov wrote:
> Add the initial device tree for the V3H Starter Kit board.
> The board has 1 debug serial port (SCIF0); include support for it,
> so that the serial console can work.
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
This looks fine but I will wait to see if there are other reviews before
applying.
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply
* Re: [PATCH] dt-bindings: net: ravb: Add support for r8a77990 SoC
From: Simon Horman @ 2018-05-13 7:58 UTC (permalink / raw)
To: David Miller
Cc: yoshihiro.shimoda.uh, netdev, linux-renesas-soc, robh+dt,
mark.rutland, sergei.shtylyov, devicetree
In-Reply-To: <20180511.155942.16024095909155343.davem@davemloft.net>
On Fri, May 11, 2018 at 03:59:42PM -0400, David Miller wrote:
> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Date: Fri, 11 May 2018 12:18:56 +0900
>
> > Add documentation for r8a77990 compatible string to renesas ravb device
> > tree bindings documentation.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>
> I'm assuming this isn't targetted at one of my trees. Just FYI.
Hi Dave,
I think this is appropriate for net-next but if not I can take it.
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Shimoda-san,
please use "[PATCH net-next]" for non-bugfix networking updates which
are targeted at Dave's net-next tree. Bug fixes should be for "net".
Patches should of course apply cleanly to whichever tree it is targeted at.
^ permalink raw reply
* Re: [PATCH] dt-bindings: gpio: rcar: Add support for r8a77990
From: Simon Horman @ 2018-05-13 7:52 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: linus.walleij, robh+dt, mark.rutland, linux-renesas-soc,
linux-gpio, devicetree
In-Reply-To: <1526008386-26420-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>
On Fri, May 11, 2018 at 12:13:05PM +0900, Yoshihiro Shimoda wrote:
> Add compatible string for R-Car E3 (r8a77990) in gpio-rcar.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> diff --git a/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt b/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
> index 9744d42..378f132 100644
> --- a/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
> +++ b/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
> @@ -17,6 +17,7 @@ Required Properties:
> - "renesas,gpio-r8a7796": for R8A7796 (R-Car M3-W) compatible GPIO controller.
> - "renesas,gpio-r8a77965": for R8A77965 (R-Car M3-N) compatible GPIO controller.
> - "renesas,gpio-r8a77970": for R8A77970 (R-Car V3M) compatible GPIO controller.
> + - "renesas,gpio-r8a77990": for R8A77990 (R-Car E3) compatible GPIO controller.
> - "renesas,gpio-r8a77995": for R8A77995 (R-Car D3) compatible GPIO controller.
> - "renesas,rcar-gen1-gpio": for a generic R-Car Gen1 GPIO controller.
> - "renesas,rcar-gen2-gpio": for a generic R-Car Gen2 or RZ/G1 GPIO controller.
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: timer: renesas, cmt: Document r8a774[35] CMT support
From: Simon Horman @ 2018-05-13 7:47 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Thomas Gleixner, Daniel Lezcano, Geert Uytterhoeven,
devicetree@vger.kernel.org, Chris Paterson, Biju Das,
linux-renesas-soc@vger.kernel.org, Rob Herring, Mark Rutland
In-Reply-To: <TY1PR01MB1770621EC40038C4237B1BAAC0BE0@TY1PR01MB1770.jpnprd01.prod.outlook.com>
On Tue, Apr 10, 2018 at 09:36:39AM +0000, Fabrizio Castro wrote:
> Good morning gentlemen,
>
> I am very sorry to bother you again, but it seems this patch has no
> master. Is anybody willing to take it?
Patchwork tells me this has been Acked by Daniel Lezcano.
So I have decided to take this one in from the cold and apply it to the
renesas tree.
^ permalink raw reply
* Re: [PATCH v2 09/11] docs: Fix some broken references
From: Takashi Iwai @ 2018-05-13 7:13 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Catalin Marinas, Will Deacon, dri-devel, Jaroslav Kysela,
Eric Paris, linux-clk, James Morris, Alan Stern, xen-devel,
Boqun Feng, Nicholas Piggin, Thomas Gleixner, Antoine Jacquet,
Greg Kroah-Hartman, linux-usb, linux-kernel, Li Zefan,
linux-crypto, Mark Rutland, alsa-devel, Linux Doc Mailing List,
David Airlie, Max Filippov, Harry Wei, selinux, Paul
In-Reply-To: <e959f23d6f6905ee606fadfda13e2bb37deed017.1525870886.git.mchehab+samsung@kernel.org>
On Wed, 09 May 2018 15:18:52 +0200,
Mauro Carvalho Chehab wrote:
>
> As we move stuff around, some doc references are broken. Fix some of
> them via this script:
> ./scripts/documentation-file-ref-check --fix-rst
>
> Manually checked if the produced result is valid, removing a few
> false-positives.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
For sound stuff, feel free to take my ack (if it's not too late):
Acked-by: Takashi Iwai <tiwai@suse.de>
thanks,
Takashi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH] remoteproc: Add APSS based Qualcomm ADSP PIL driver for SDM845
From: Rohit kumar @ 2018-05-13 7:01 UTC (permalink / raw)
To: ohad, bjorn.andersson, robh+dt, mark.rutland, linux-remoteproc,
devicetree, linux-kernel, bgoswami, sbpata, asishb, rkarra
Cc: Rohit kumar, RajendraBabu Medisetti, Krishnamurthy Renu
This adds Qualcomm ADSP PIL driver support for SDM845 with ADSP bootup
and shutdown operation handled from Application Processor SubSystem(APSS).
Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
Signed-off-by: RajendraBabu Medisetti <rajendrabm@codeaurora.org>
Signed-off-by: Krishnamurthy Renu <krishnamurthy.renu@codeaurora.org>
---
.../devicetree/bindings/remoteproc/qcom,adsp.txt | 1 +
drivers/remoteproc/Makefile | 3 +-
drivers/remoteproc/qcom_adsp_pil.c | 122 ++++-----
drivers/remoteproc/qcom_adsp_pil.h | 86 ++++++
drivers/remoteproc/qcom_adsp_pil_sdm845.c | 304 +++++++++++++++++++++
5 files changed, 454 insertions(+), 62 deletions(-)
create mode 100644 drivers/remoteproc/qcom_adsp_pil.h
create mode 100644 drivers/remoteproc/qcom_adsp_pil_sdm845.c
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
index 728e419..a9fe033 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
@@ -10,6 +10,7 @@ on the Qualcomm ADSP Hexagon core.
"qcom,msm8974-adsp-pil"
"qcom,msm8996-adsp-pil"
"qcom,msm8996-slpi-pil"
+ "qcom,sdm845-apss-adsp-pil"
- interrupts-extended:
Usage: required
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 02627ed..759831b 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -14,7 +14,8 @@ obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
obj-$(CONFIG_KEYSTONE_REMOTEPROC) += keystone_remoteproc.o
-obj-$(CONFIG_QCOM_ADSP_PIL) += qcom_adsp_pil.o
+obj-$(CONFIG_QCOM_ADSP_PIL) += qcom_adsp.o
+qcom_adsp-objs += qcom_adsp_pil.o qcom_adsp_pil_sdm845.o
obj-$(CONFIG_QCOM_RPROC_COMMON) += qcom_common.o
obj-$(CONFIG_QCOM_Q6V5_PIL) += qcom_q6v5_pil.o
obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
index 89a86ce..9ab3698 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -1,5 +1,5 @@
/*
- * Qualcomm ADSP/SLPI Peripheral Image Loader for MSM8974 and MSM8996
+ * Qualcomm ADSP/SLPI Peripheral Image Loader for MSM8974, MSM8996 and SDM845.
*
* Copyright (C) 2016 Linaro Ltd
* Copyright (C) 2014 Sony Mobile Communications AB
@@ -22,7 +22,6 @@
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/of_device.h>
-#include <linux/platform_device.h>
#include <linux/qcom_scm.h>
#include <linux/regulator/consumer.h>
#include <linux/remoteproc.h>
@@ -30,56 +29,8 @@
#include <linux/soc/qcom/smem.h>
#include <linux/soc/qcom/smem_state.h>
-#include "qcom_common.h"
#include "remoteproc_internal.h"
-
-struct adsp_data {
- int crash_reason_smem;
- const char *firmware_name;
- int pas_id;
- bool has_aggre2_clk;
-
- const char *ssr_name;
- const char *sysmon_name;
- int ssctl_id;
-};
-
-struct qcom_adsp {
- struct device *dev;
- struct rproc *rproc;
-
- int wdog_irq;
- int fatal_irq;
- int ready_irq;
- int handover_irq;
- int stop_ack_irq;
-
- struct qcom_smem_state *state;
- unsigned stop_bit;
-
- struct clk *xo;
- struct clk *aggre2_clk;
-
- struct regulator *cx_supply;
- struct regulator *px_supply;
-
- int pas_id;
- int crash_reason_smem;
- bool has_aggre2_clk;
-
- struct completion start_done;
- struct completion stop_done;
-
- phys_addr_t mem_phys;
- phys_addr_t mem_reloc;
- void *mem_region;
- size_t mem_size;
-
- struct qcom_rproc_glink glink_subdev;
- struct qcom_rproc_subdev smd_subdev;
- struct qcom_rproc_ssr ssr_subdev;
- struct qcom_sysmon *sysmon;
-};
+#include "qcom_adsp_pil.h"
static int adsp_load(struct rproc *rproc, const struct firmware *fw)
{
@@ -112,18 +63,32 @@ static int adsp_start(struct rproc *rproc)
if (ret)
goto disable_cx_supply;
- ret = qcom_scm_pas_auth_and_reset(adsp->pas_id);
- if (ret) {
- dev_err(adsp->dev,
- "failed to authenticate image and release reset\n");
- goto disable_px_supply;
+ if (adsp->is_apss_controlled) {
+ ret = adsp->ops->bringup(adsp);
+ if (ret) {
+ dev_err(adsp->dev, "adsp bringup failed\n");
+ adsp->ops->bringdown(adsp);
+ goto disable_px_supply;
+ }
+ } else {
+ ret = qcom_scm_pas_auth_and_reset(adsp->pas_id);
+ if (ret) {
+ dev_err(adsp->dev,
+ "failed to authenticate image and release reset\n");
+ goto disable_px_supply;
+ }
}
ret = wait_for_completion_timeout(&adsp->start_done,
msecs_to_jiffies(5000));
if (!ret) {
dev_err(adsp->dev, "start timed out\n");
- qcom_scm_pas_shutdown(adsp->pas_id);
+
+ if (adsp->is_apss_controlled)
+ adsp->ops->bringdown(adsp);
+ else
+ qcom_scm_pas_shutdown(adsp->pas_id);
+
ret = -ETIMEDOUT;
goto disable_px_supply;
}
@@ -160,7 +125,11 @@ static int adsp_stop(struct rproc *rproc)
BIT(adsp->stop_bit),
0);
- ret = qcom_scm_pas_shutdown(adsp->pas_id);
+ if (adsp->is_apss_controlled)
+ ret = adsp->ops->bringdown(adsp);
+ else
+ ret = qcom_scm_pas_shutdown(adsp->pas_id);
+
if (ret)
dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
@@ -334,8 +303,9 @@ static int adsp_probe(struct platform_device *pdev)
if (!desc)
return -EINVAL;
- if (!qcom_scm_is_available())
- return -EPROBE_DEFER;
+ if (!desc->is_apss_controlled)
+ if (!qcom_scm_is_available())
+ return -EPROBE_DEFER;
rproc = rproc_alloc(&pdev->dev, pdev->name, &adsp_ops,
desc->firmware_name, sizeof(*adsp));
@@ -350,6 +320,7 @@ static int adsp_probe(struct platform_device *pdev)
adsp->pas_id = desc->pas_id;
adsp->crash_reason_smem = desc->crash_reason_smem;
adsp->has_aggre2_clk = desc->has_aggre2_clk;
+ adsp->is_apss_controlled = desc->is_apss_controlled;
platform_set_drvdata(pdev, adsp);
init_completion(&adsp->start_done);
@@ -399,6 +370,19 @@ static int adsp_probe(struct platform_device *pdev)
goto free_rproc;
}
+ if (adsp->is_apss_controlled) {
+ if (!desc->ops || !desc->ops->bringup ||
+ !desc->ops->bringdown || !desc->ops->map_regs) {
+ dev_err(&pdev->dev, "SoC ops not defined\n");
+ ret = -EINVAL;
+ goto free_rproc;
+ }
+ adsp->ops = desc->ops;
+ ret = adsp->ops->map_regs(adsp, pdev);
+ if (ret)
+ goto free_rproc;
+ }
+
qcom_add_glink_subdev(rproc, &adsp->glink_subdev);
qcom_add_smd_subdev(rproc, &adsp->smd_subdev);
qcom_add_ssr_subdev(rproc, &adsp->ssr_subdev, desc->ssr_name);
@@ -434,11 +418,24 @@ static int adsp_remove(struct platform_device *pdev)
return 0;
}
+static const struct adsp_data sdm845_apss_adsp_resource_init = {
+ .crash_reason_smem = 423,
+ .firmware_name = "adsp.mdt",
+ .pas_id = 1,
+ .has_aggre2_clk = false,
+ .is_apss_controlled = true,
+ .ssr_name = "lpass",
+ .sysmon_name = "adsp",
+ .ssctl_id = 0x14,
+ .ops = &sdm845_soc_ops,
+};
+
static const struct adsp_data adsp_resource_init = {
.crash_reason_smem = 423,
.firmware_name = "adsp.mdt",
.pas_id = 1,
.has_aggre2_clk = false,
+ .is_apss_controlled = false,
.ssr_name = "lpass",
.sysmon_name = "adsp",
.ssctl_id = 0x14,
@@ -449,6 +446,7 @@ static int adsp_remove(struct platform_device *pdev)
.firmware_name = "slpi.mdt",
.pas_id = 12,
.has_aggre2_clk = true,
+ .is_apss_controlled = false,
.ssr_name = "dsps",
.sysmon_name = "slpi",
.ssctl_id = 0x16,
@@ -458,6 +456,8 @@ static int adsp_remove(struct platform_device *pdev)
{ .compatible = "qcom,msm8974-adsp-pil", .data = &adsp_resource_init},
{ .compatible = "qcom,msm8996-adsp-pil", .data = &adsp_resource_init},
{ .compatible = "qcom,msm8996-slpi-pil", .data = &slpi_resource_init},
+ { .compatible = "qcom,sdm845-apss-adsp-pil",
+ .data = &sdm845_apss_adsp_resource_init},
{ },
};
MODULE_DEVICE_TABLE(of, adsp_of_match);
@@ -472,5 +472,5 @@ static int adsp_remove(struct platform_device *pdev)
};
module_platform_driver(adsp_driver);
-MODULE_DESCRIPTION("Qualcomm MSM8974/MSM8996 ADSP Peripherial Image Loader");
+MODULE_DESCRIPTION("Qualcomm MSM8974/MSM8996/SDM845 ADSP Peripherial Image Loader");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/remoteproc/qcom_adsp_pil.h b/drivers/remoteproc/qcom_adsp_pil.h
new file mode 100644
index 0000000..29fd086
--- /dev/null
+++ b/drivers/remoteproc/qcom_adsp_pil.h
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+// Copyright (c) 2018, The Linux Foundation. All rights reserved
+
+#ifndef __QCOM_ADSP_PIL_H__
+#define __QCOM_ADSP_PIL_H__
+
+#include <linux/platform_device.h>
+#include "qcom_common.h"
+
+struct qcom_adsp;
+
+struct soc_ops {
+ int (*bringup)(struct qcom_adsp *adsp);
+ int (*bringdown)(struct qcom_adsp *adsp);
+ int (*map_regs)(struct qcom_adsp *adsp, struct platform_device *pdev);
+};
+
+struct adsp_data {
+ int crash_reason_smem;
+ const char *firmware_name;
+ int pas_id;
+ bool has_aggre2_clk;
+ bool is_apss_controlled;
+ const char *ssr_name;
+ const char *sysmon_name;
+ int ssctl_id;
+ struct soc_ops *ops;
+};
+
+struct qcom_adsp {
+ struct device *dev;
+ struct rproc *rproc;
+
+ int wdog_irq;
+ int fatal_irq;
+ int ready_irq;
+ int handover_irq;
+ int stop_ack_irq;
+
+ struct qcom_smem_state *state;
+ unsigned int stop_bit;
+
+ struct clk *xo;
+ struct clk *aggre2_clk;
+
+ struct regulator *cx_supply;
+ struct regulator *px_supply;
+
+ int pas_id;
+ int crash_reason_smem;
+ bool has_aggre2_clk;
+ bool is_apss_controlled;
+
+ struct completion start_done;
+ struct completion stop_done;
+
+ phys_addr_t mem_phys;
+ phys_addr_t mem_reloc;
+ void *mem_region;
+ size_t mem_size;
+
+ struct soc_ops *ops;
+ void *priv_reg;
+
+ struct qcom_rproc_glink glink_subdev;
+ struct qcom_rproc_subdev smd_subdev;
+ struct qcom_rproc_ssr ssr_subdev;
+ struct qcom_sysmon *sysmon;
+};
+
+extern struct soc_ops sdm845_soc_ops;
+
+static inline void update_bits(void *reg, u32 mask_val, u32 set_val, u32 shift)
+{
+ u32 reg_val = 0;
+
+ reg_val = ((readl(reg)) & ~mask_val) | ((set_val << shift) & mask_val);
+ writel(reg_val, reg);
+}
+
+static inline unsigned int read_bit(void *reg, u32 mask, int shift)
+{
+ return ((readl(reg) & mask) >> shift);
+}
+
+#endif
diff --git a/drivers/remoteproc/qcom_adsp_pil_sdm845.c b/drivers/remoteproc/qcom_adsp_pil_sdm845.c
new file mode 100644
index 0000000..7518385
--- /dev/null
+++ b/drivers/remoteproc/qcom_adsp_pil_sdm845.c
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Qualcomm APSS Based ADSP bootup/shutdown ops for SDM845.
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+
+#include "qcom_adsp_pil.h"
+
+/* set values */
+#define CLK_ENABLE 0x1
+#define CLK_DISABLE 0x0
+/* time out value */
+#define ACK_TIMEOUT 200000
+/* mask values */
+#define CLK_MASK GENMASK(4, 0)
+#define EVB_MASK GENMASK(27, 4)
+#define SPIN_CLKOFF_MASK BIT(31)
+#define AUDIO_SYNC_RESET_MASK BIT(2)
+#define CLK_ENABLE_MASK BIT(0)
+#define HAL_CLK_MASK BIT(1)
+/* GCC register offsets */
+#define GCC_BASE 0x00147000
+#define SWAY_CBCR_OFFSET 0x00000008
+/*LPASS register base address and offsets*/
+#define LPASS_BASE 0x17000000
+#define AON_CBCR_OFFSET 0x00014098
+#define CMD_RCGR_OFFSET 0x00014000
+#define CFG_RCGR_OFFSET 0x00014004
+#define AHBS_AON_CBCR_OFFSET 0x00033000
+#define AHBM_AON_CBCR_OFFSET 0x00026000
+/*QDSP6SS register base address and offsets*/
+#define QDSP6SS_BASE 0x17300000
+#define RST_EVB_OFFSET 0x00000010
+#define SLEEP_CBCR_OFFSET 0x0000003C
+#define XO_CBCR_OFFSET 0x00000038
+#define CORE_CBCR_OFFSET 0x00000020
+#define CORE_START_OFFSET 0x00000400
+#define BOOT_CMD_OFFSET 0x00000404
+#define BOOT_STATUS_OFFSET 0x00000408
+#define RET_CFG_OFFSET 0x0000001C
+/*TCSR register base address and offsets*/
+#define TCSR_BASE 0x01F62000
+#define TCSR_LPASS_MASTER_IDLE_OFFSET 0x00000008
+#define TCSR_LPASS_HALTACK_OFFSET 0x00000004
+#define TCSR_LPASS_PWR_ON_OFFSET 0x00000010
+#define TCSR_LPASS_HALTREQ_OFFSET 0X00000000
+
+#define RPMH_PDC_SYNC_RESET_ADDR 0x0B2E0100
+#define AOSS_CC_LPASS_RESTART_ADDR 0x0C2D0000
+
+struct sdm845_reg {
+ void __iomem *gcc_base;
+ void __iomem *lpass_base;
+ void __iomem *qdsp6ss_base;
+ void __iomem *tcsr_base;
+ void __iomem *pdc_sync;
+ void __iomem *cc_lpass;
+};
+
+static int sdm845_map_registers(struct qcom_adsp *adsp,
+ struct platform_device *pdev)
+{
+ struct sdm845_reg *reg;
+
+ adsp->priv_reg = devm_kzalloc(&pdev->dev, sizeof(struct sdm845_reg),
+ GFP_KERNEL);
+ if (!adsp->priv_reg)
+ return -ENOMEM;
+
+ reg = adsp->priv_reg;
+
+ reg->gcc_base = devm_ioremap(adsp->dev, GCC_BASE, 0xc);
+ if (!reg->gcc_base) {
+ dev_err(adsp->dev, "%s: failed to map GCC base registers\n",
+ __func__);
+ return -ENOMEM;
+ }
+
+ reg->lpass_base = devm_ioremap(adsp->dev, LPASS_BASE, 0x8E004);
+ if (!reg->lpass_base) {
+ dev_err(adsp->dev, "%s: failed to map LPASS base registers\n",
+ __func__);
+ return -ENOMEM;
+ }
+ reg->qdsp6ss_base = devm_ioremap(adsp->dev, QDSP6SS_BASE, 0x40c);
+ if (!reg->qdsp6ss_base) {
+ dev_err(adsp->dev, "%s: failed to map QDSP6SS base registers\n",
+ __func__);
+ return -ENOMEM;
+ }
+ reg->tcsr_base = devm_ioremap(adsp->dev, TCSR_BASE, 0x14);
+ if (!reg->tcsr_base) {
+ dev_err(adsp->dev, "%s: failed to map TCSR base registers\n",
+ __func__);
+ return -ENOMEM;
+ }
+ reg->pdc_sync = devm_ioremap(adsp->dev, RPMH_PDC_SYNC_RESET_ADDR, 0x4);
+ if (!reg->pdc_sync) {
+ dev_err(adsp->dev, "%s: failed to map RPMH_PDC_SYNC_RESET register\n",
+ __func__);
+ return -ENOMEM;
+ }
+ reg->cc_lpass = devm_ioremap(adsp->dev, AOSS_CC_LPASS_RESTART_ADDR,
+ 0x4);
+ if (!reg->cc_lpass) {
+ dev_err(adsp->dev, "%s:failed to map AOSS_CC_LPASS_RESTART register\n",
+ __func__);
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static int clk_enable_spin(void *reg, int read_shift, int write_shift)
+{
+ u32 maxDelay = 500;
+ u32 val;
+
+ update_bits(reg, CLK_ENABLE_MASK, CLK_ENABLE, write_shift);
+ val = readl(reg);
+ if (!(readl(reg) & HAL_CLK_MASK)) {
+ /*
+ * wait for disabling of HW signal CLK_OFF to confirm that
+ * clock is actually ON.
+ */
+ while (maxDelay-- && read_bit(reg, SPIN_CLKOFF_MASK,
+ read_shift))
+ udelay(1);
+ }
+ if (!maxDelay) {
+ pr_err("%s: fail to update register = %p\n", __func__, reg);
+ return -ETIMEDOUT;
+ }
+ return 0;
+}
+
+static int sdm845_adsp_clk_enable(struct qcom_adsp *adsp)
+{
+ u32 ret;
+ u32 maxDelay = 100;
+ struct sdm845_reg *reg = adsp->priv_reg;
+
+ /* Enable SWAY clock */
+ ret = clk_enable_spin(reg->gcc_base + SWAY_CBCR_OFFSET, CLK_MASK, 0x0);
+ if (ret)
+ return ret;
+
+ /* Enable LPASS AHB AON Bus */
+ ret = clk_enable_spin(reg->lpass_base + AON_CBCR_OFFSET, CLK_MASK, 0x0);
+ if (ret)
+ return ret;
+
+ /* Set the AON clock root to be sourced by XO */
+ writel(CLK_DISABLE, reg->lpass_base + CFG_RCGR_OFFSET);
+ writel(CLK_ENABLE, reg->lpass_base + CMD_RCGR_OFFSET);
+
+ while (read_bit((reg->lpass_base + CMD_RCGR_OFFSET), CLK_ENABLE, 0)
+ && maxDelay--)
+ udelay(2);
+
+ if (!maxDelay) {
+ pr_err("%s: fail to enable CMD_RCGR clock\n", __func__);
+ return -ETIMEDOUT;
+ }
+
+ /* Enable the QDSP6SS AHBM and AHBS clocks */
+ ret = clk_enable_spin(reg->lpass_base + AHBS_AON_CBCR_OFFSET,
+ CLK_MASK, 0x0);
+ if (ret)
+ return ret;
+ ret = clk_enable_spin(reg->lpass_base + AHBM_AON_CBCR_OFFSET,
+ CLK_MASK, 0x0);
+ if (ret)
+ return ret;
+
+ /* Turn on the XO clock, required to boot FSM */
+ update_bits(reg->qdsp6ss_base + XO_CBCR_OFFSET, CLK_ENABLE_MASK,
+ CLK_ENABLE, 0x0);
+
+ /* Enable the QDSP6SS sleep clock for the QDSP6 watchdog enablement */
+ update_bits(reg->qdsp6ss_base + SLEEP_CBCR_OFFSET,
+ CLK_ENABLE_MASK, CLK_ENABLE, 0x0);
+
+ /* Configure QDSP6 core CBC to enable clock */
+ update_bits(reg->qdsp6ss_base + CORE_CBCR_OFFSET, CLK_ENABLE_MASK,
+ CLK_ENABLE, 0x0);
+ return 0;
+}
+
+static int sdm845_adsp_reset(struct qcom_adsp *adsp)
+{
+ u32 timeout = ACK_TIMEOUT;
+ struct sdm845_reg *reg = adsp->priv_reg;
+
+ /* De-assert QDSP6 stop core. QDSP6 will execute after out of reset */
+ update_bits(reg->qdsp6ss_base + CORE_START_OFFSET,
+ CLK_ENABLE_MASK, CLK_ENABLE, 0x0);
+ /* Trigger boot FSM to start QDSP6 */
+ writel(CLK_ENABLE, reg->qdsp6ss_base + BOOT_CMD_OFFSET);
+
+ /* Wait for core to come out of reset */
+ while ((!(readl(reg->qdsp6ss_base +
+ BOOT_STATUS_OFFSET))) && (timeout-- > 0))
+ udelay(5);
+
+ if (!timeout)
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
+static int sdm845_bringup(struct qcom_adsp *adsp)
+{
+ u32 ret;
+ struct sdm845_reg *reg = adsp->priv_reg;
+
+ ret = sdm845_adsp_clk_enable(adsp);
+ if (ret) {
+ dev_err(adsp->dev, "%s: sdm845_adsp_clk_enable failed\n",
+ __func__);
+ return ret;
+ }
+ /* Program boot address */
+ update_bits(reg->qdsp6ss_base + RST_EVB_OFFSET,
+ EVB_MASK, (adsp->mem_phys) >> 8, 0x4);
+
+ /* Wait for addresses to be programmed before starting adsp */
+ mb();
+ ret = sdm845_adsp_reset(adsp);
+ if (ret)
+ dev_err(adsp->dev, "%s: De-assert QDSP6 out of reset failed\n",
+ __func__);
+ return ret;
+}
+
+static int sdm845_bringdown(struct qcom_adsp *adsp)
+{
+ u32 acktimeout = ACK_TIMEOUT;
+ u32 temp;
+ struct sdm845_reg *reg = adsp->priv_reg;
+
+ /* Reset the retention logic */
+ update_bits(reg->qdsp6ss_base + RET_CFG_OFFSET,
+ CLK_ENABLE_MASK, CLK_ENABLE, 0x0);
+ /* Disable the slave way clock to LPASS */
+ update_bits(reg->gcc_base + SWAY_CBCR_OFFSET,
+ CLK_ENABLE_MASK, CLK_DISABLE, 0x0);
+
+ /* QDSP6 master port needs to be explicitly halted */
+ temp = read_bit(reg->tcsr_base + TCSR_LPASS_PWR_ON_OFFSET,
+ CLK_ENABLE, 0x0);
+ temp = temp && !read_bit(reg->tcsr_base + TCSR_LPASS_MASTER_IDLE_OFFSET,
+ CLK_ENABLE, 0x0);
+ if (temp) {
+ writel(CLK_ENABLE, reg->tcsr_base + TCSR_LPASS_HALTREQ_OFFSET);
+ /* Wait for halt ACK from QDSP6 */
+ while ((read_bit(reg->tcsr_base + TCSR_LPASS_HALTACK_OFFSET,
+ CLK_DISABLE, 0x0) == 0) && (acktimeout-- > 0))
+ udelay(5);
+
+ if (acktimeout) {
+ if (read_bit(reg->tcsr_base +
+ TCSR_LPASS_MASTER_IDLE_OFFSET,
+ CLK_ENABLE, 0x0) != 1)
+ dev_warn(adsp->dev,
+ "%s: failed to receive %s\n",
+ __func__, "TCSR MASTER ACK");
+ } else {
+ dev_err(adsp->dev, "%s: failed to receive halt ack\n",
+ __func__);
+ return -ETIMEDOUT;
+ }
+ }
+
+ /* Assert the LPASS PDC Reset */
+ update_bits(reg->pdc_sync, AUDIO_SYNC_RESET_MASK,
+ CLK_ENABLE, 0x2);
+ /* Place the LPASS processor into reset */
+ writel(CLK_ENABLE, reg->cc_lpass);
+ /* wait after asserting subsystem restart from AOSS */
+ udelay(200);
+
+ /* Clear the halt request for the AXIM and AHBM for Q6 */
+ writel(CLK_DISABLE, reg->tcsr_base + TCSR_LPASS_HALTREQ_OFFSET);
+
+ /* De-assert the LPASS PDC Reset */
+ update_bits(reg->pdc_sync, AUDIO_SYNC_RESET_MASK,
+ CLK_DISABLE, 0x2);
+ /* Remove the LPASS reset */
+ writel(CLK_DISABLE, reg->cc_lpass);
+ /* wait after de-asserting subsystem restart from AOSS */
+ udelay(200);
+
+ return 0;
+}
+
+struct soc_ops sdm845_soc_ops = {
+ .bringup = sdm845_bringup,
+ .bringdown = sdm845_bringdown,
+ .map_regs = sdm845_map_registers,
+};
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* Re: [PATCH v2 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver
From: Baolin Wang @ 2018-05-13 2:19 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Pavel Machek, Rob Herring, Mark Rutland, xiaotong.lu, Mark Brown,
linux-leds, DTML, LKML
In-Reply-To: <7c9d5eec-4e6b-9b26-470c-f8002cca5f27@gmail.com>
Hi Jacek and Pavel,
On 13 May 2018 at 04:44, Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> Hi Pavel,
>
>
> On 05/12/2018 10:35 AM, Pavel Machek wrote:
>>
>> Hi!
>>
>>>>> I disagree here. We already had the same discussion at the occasion
>>>>> of the patch [0] and it turned out to be a dead-end [1]. Now we have
>>>>> neither the driver nor the generic pattern interface.
>>>>>
>>>>> We also already have some older LED class drivers that implement custom
>>>>> pattern interfaces (e.g. drivers/leds/leds-lm3533.c) and the same
>>>>> approach can be applied in this case.
>>>>
>>>>
>>>> Please don't. It was mistake to implement custom pattern interfaces
>>>> back then, it is still mistake now.
>>>
>>>
>>> It turned out to be really hard to cover all known pattern generator
>>> implementations with generic interface. Sure, it would be nice to have
>>> one, but the whole discussion around [0] only unveiled the diversity of
>>> parameters to cover. And still new devices appear on the market.
>>>
>>> We would have to propose a set of pattern schemes and allow to
>>> add new ones to it.
>>
>>
>> I believe that what I'm proposing below is close enough to universal.
>>
>>>> If we really need solution now, I'd recommend "pattern" file with
>>>>
>>>> "<delta time> <brightness> <delta time> <brightness>".
>>>>
>>>> In this specific case, hardware only supports patterns in this format:
>>>>
>>>> low_time 0 rise_time 255 high_time 255 fall_time 0
>>>>
>>>> so driver would simply -EINVAL on anything else.
>>>
>>>
>>> I'm fine with the pattern file, but the pattern format would have
>>> to be defined in the per-driver ABI documentation. It wouldn't much
>>> differ from the custom pattern approach though, unless I'm missing some
>>> gain of having pattern setting in a uniformly named single sysfs file
>>> (with semantics differing from driver to driver).
>>
>>
>> I'm proposing "<delta time> <brightness> ..." sysfs file. It certainly
>> covers this hardware, it would be enough to cover the Qualcomm Pulse
>> generator (IIRC), and it would cover most uses cases of Nokia N900's
>> LED.
>>
>> Yes, we would need to document limitations of each chip. But it should
>> be easily possible to run pattern designed for Spreadtrum on N900,
>> even if it would not work the other way around.
>>
>> (If someone really wants to run complex patterns on simple hardware,
>> we can provide software emulation using same file format. I believe I
>> still have that patch somewhere.)
>
>
> OK, I've revised the discussion under Qualcomm LPG patch set and
> it seems that we have almost ready solution in [0], except the
> pattern_repeat file you mention in [1]. So probably Baolin could
> address your remarks from [1] and add pattern_repeat file to the
> patch that begins thread [0].
>
> [0] https://lkml.org/lkml/2017/11/15/27
> [1] https://lkml.org/lkml/2017/12/8/470
Thanks for your suggestion. So I will remove the sysfs part from the
new driver, then send incremental patches when introducing some common
LED interfaces.
--
Baolin.wang
Best Regards
^ permalink raw reply
* [PATCH] ARM: dts: da850-lego-ev3: remove unnecessary gpio-keys properties
From: David Lechner @ 2018-05-12 21:41 UTC (permalink / raw)
To: linux-arm-kernel
Cc: David Lechner, Sekhar Nori, Kevin Hilman, devicetree,
linux-kernel
This removes the #address-cells and #size-cells properties from the
gpio-keys node in the da850-lego-ev3 device tree. These properties are
not needed since the child nodes don't have a reg property.
Signed-off-by: David Lechner <david@lechnology.com>
---
arch/arm/boot/dts/da850-lego-ev3.dts | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm/boot/dts/da850-lego-ev3.dts b/arch/arm/boot/dts/da850-lego-ev3.dts
index d4a131b6a46e..3fe8db69f50c 100644
--- a/arch/arm/boot/dts/da850-lego-ev3.dts
+++ b/arch/arm/boot/dts/da850-lego-ev3.dts
@@ -33,8 +33,6 @@
*/
gpio_keys {
compatible = "gpio-keys";
- #address-cells = <1>;
- #size-cells = <0>;
label = "EV3 Brick Buttons";
pinctrl-names = "default";
pinctrl-0 = <&button_bias>;
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v10 00/27] ARM: davinci: convert to common clock framework
From: David Lechner @ 2018-05-12 21:11 UTC (permalink / raw)
To: Sekhar Nori, linux-clk, devicetree, linux-arm-kernel
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Kevin Hilman, Bartosz Golaszewski, Adam Ford, linux-kernel
In-Reply-To: <45ce3129-4a9e-54d2-e594-49b451571842@ti.com>
On 05/11/2018 10:26 AM, Sekhar Nori wrote:
> Hi David,
>
> On Wednesday 09 May 2018 10:55 PM, David Lechner wrote:
>> This series converts mach-davinci to use the common clock framework.
>>
>> The series works like this, the first 3 patches fix some issues with the clock
>> drivers that have already been accepted into the mainline kernel.
>
> I have not yet looked at the patches, but I got a bunch of W=1 warnings
> and some sparse warnings when building your branch. Please take a look
> at these. Unfortunately the output is mixed between sparse and compiler.
> The "expression using sizeof(void)" can be ignored as its a known issue
> with sparse, I believe.
>
I've started a common-clk-v11 branch on my GitHub that fixes most of these.
Also submitted "clk: davinci: psc-dm355: fix ASP0/1 clkdev lookups" that
fixes a couple more. I've purposely not fixed the davinci_clk_reset_* functions
since there is already a patch that will remove those functions in the future.
I'll wait a bit longer for DT review before re-sending v11 of this series.
^ 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