* Re: [PATCH v5] arm64: dts: mt8173: add mmsel clocks for 4K support
From: Matthias Brugger @ 2017-01-13 15:32 UTC (permalink / raw)
To: Daniel Kurtz, Bibby Hsieh
Cc: Mark Rutland, open list:OPEN FIRMWARE AND...,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
moderated list:ARM/Mediatek SoC support, Rob Herring, Pawel Moll,
Ian Campbell, Kumar Gala, Catalin Marinas, Will Deacon,
Yingjoe Chen, Sascha Hauer, James Liao, Lorenzo Pieralisi,
YH Huang
In-Reply-To: <CAGS+omAZvHmSz6bJcvisC1LZgxwbd-HzvYMDrHXzfkY3mZZEsw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 12/01/17 05:50, Daniel Kurtz wrote:
> Hi Matthias,
>
> (Trying again to send plain text email)...
>
> On Thu, Aug 4, 2016 at 10:57 AM, Bibby Hsieh <bibby.hsieh-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>> To support HDMI 4K resolution, mmsys need clcok
>> mm_sel to be 400MHz.
>>
>> The board .dts file should override the clock rate
>> property with the higher VENCPLL frequency the board
>> supports HDMI 4K resolution.
>>
>> Signed-off-by: Bibby Hsieh <bibby.hsieh-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>
> Reviewed-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>
Applied to v4.10-next/dts
Thanks
>
> It looks like this patch was lost. It is actually a prerequisite for
> MTK 4k HDMI support, which already landed in v4.9.
>
> See the email thread entitled:
> [PATCH v5 0/3] MT8173 HDMI 4K support <https://lkml.org/lkml/2016/9/28/893>
>
> Or these three:
>
> 0d2200794f0a drm/mediatek: modify the factor to make the pll_rate set
> in the 1G-2G range
> 968253bd7caa drm/mediatek: enhance the HDMI driving current
> d542b7c473f0 drm/mediatek: do mtk_hdmi_send_infoframe after HDMI clock enable
>
> -Dan
>
>> ---
>> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> index 78529e4..c3f32f3 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> @@ -690,6 +690,8 @@
>> compatible = "mediatek,mt8173-mmsys", "syscon";
>> reg = <0 0x14000000 0 0x1000>;
>> power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
>> + assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
>> + assigned-clock-rates = <400000000>;
>> #clock-cells = <1>;
>> };
>>
>> --
>> 1.7.9.5
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] arm64: dts: mt8173: Fix cpu_thermal cooling-maps contributions
From: Matthias Brugger @ 2017-01-13 15:36 UTC (permalink / raw)
To: Daniel Kurtz
Cc: Dawei Chien, Javi Merino, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Hans Verkuil, Mauro Carvalho Chehab,
Tiffany Lin, CK Hu, Andrew-CT Chen, Sascha Hauer, Minghsiu Tsai,
Yong Wu, Chunfeng Yun,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM64 PORT (AARCH64 ARCHITECTURE)
In-Reply-To: <20170113023006.150845-1-djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
On 13/01/17 03:30, Daniel Kurtz wrote:
> According to [0], the contribution field for each cooling-device express
> their relative power efficiency. Higher weights express higher power
> efficiency. Weighting is relative such that if each cooling device has a
> weight of 1 they are considered equal. This is particularly useful in
> heterogeneous systems where two cooling devices may perform the same kind
> of compute, but with different efficiency.
>
> [0] Documentation/thermal/power_allocator.txt
>
> According to Mediatek IC designer, the power efficiency ratio between the
> LITTLE core cluster (cooling-device cpu0) and big core cluster
> (cooling-device cpu1) is around 3:1 (3072:1024).
>
> Signed-off-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
applied, thanks.
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> index 12e702771f5c..9a3b0d20f7a8 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> @@ -182,12 +182,12 @@
> map@0 {
> trip = <&target>;
> cooling-device = <&cpu0 0 0>;
> - contribution = <1024>;
> + contribution = <3072>;
> };
> map@1 {
> trip = <&target>;
> cooling-device = <&cpu2 0 0>;
> - contribution = <2048>;
> + contribution = <1024>;
> };
> };
> };
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 3/5] i2c: mux: pca954x: Add interrupt controller support
From: Phil Reid @ 2017-01-13 15:43 UTC (permalink / raw)
To: Peter Rosin, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <383ee3cb-b452-259f-486f-682ea9526708-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 13/01/2017 17:11, Peter Rosin wrote:
> On 2017-01-09 10:02, Phil Reid wrote:
>> Various muxes can aggregate multiple interrupts from each i2c bus.
>> All of the muxes with interrupt support combine the active low irq lines
>> using an internal 'and' function and generate a combined active low
>> output. The muxes do provide the ability to read a control register to
>> determine which irq is active. By making the mux an irq controller isr
>> latency can potentially be reduced by reading the status register and
>> then only calling the registered isr on that bus segment.
>>
>> As there is no irq masking on the mux irq are disabled until irq_unmask is
>> called at least once.
>>
>
> I had a second reading of this patch. I'm still no master-of-irqs, though.
> Anyway, I have some questions below. I guess it mostly shows that I don't
> really know what I'm talking about here...
I'm no expert either...
>
>> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
>> ---
>> drivers/i2c/muxes/i2c-mux-pca954x.c | 127 +++++++++++++++++++++++++++++++++++-
>> 1 file changed, 125 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> index bbf088e..84fc767 100644
>> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
>> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> @@ -41,14 +41,19 @@
>> #include <linux/i2c.h>
>> #include <linux/i2c-mux.h>
>> #include <linux/i2c/pca954x.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> +#include <linux/of_irq.h>
>> #include <linux/pm.h>
>> #include <linux/slab.h>
>>
>> #define PCA954X_MAX_NCHANS 8
>>
>> +#define PCA954X_IRQ_OFFSET 4
>> +
>> enum pca_type {
>> pca_9540,
>> pca_9542,
>> @@ -63,6 +68,7 @@ enum pca_type {
>> struct chip_desc {
>> u8 nchans;
>> u8 enable; /* used for muxes only */
>> + u8 has_irq;
>> enum muxtype {
>> pca954x_ismux = 0,
>> pca954x_isswi
>> @@ -75,6 +81,9 @@ struct pca954x {
>> u8 last_chan; /* last register value */
>> u8 deselect;
>> struct i2c_client *client;
>> +
>> + struct irq_domain *irq;
>> + unsigned int irq_mask;
>> };
>>
>> /* Provide specs for the PCA954x types we know about */
>> @@ -87,19 +96,23 @@ struct pca954x {
>> [pca_9542] = {
>> .nchans = 2,
>> .enable = 0x4,
>> + .has_irq = 1,
>> .muxtype = pca954x_ismux,
>> },
>> [pca_9543] = {
>> .nchans = 2,
>> + .has_irq = 1,
>> .muxtype = pca954x_isswi,
>> },
>> [pca_9544] = {
>> .nchans = 4,
>> .enable = 0x4,
>> + .has_irq = 1,
>> .muxtype = pca954x_ismux,
>> },
>> [pca_9545] = {
>> .nchans = 4,
>> + .has_irq = 1,
>> .muxtype = pca954x_isswi,
>> },
>> [pca_9547] = {
>> @@ -222,6 +235,102 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
>> return pca954x_reg_write(muxc->parent, client, data->last_chan);
>> }
>>
>> +static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
>> +{
>> + struct pca954x *data = dev_id;
>> + unsigned int child_irq;
>> + int ret, i, handled;
>> +
>> + ret = i2c_smbus_read_byte(data->client);
>> + if (ret < 0)
>> + return IRQ_NONE;
>> +
>> + for (i = 0; i < data->chip->nchans; i++) {
>> + if (ret & BIT(PCA954X_IRQ_OFFSET + i)) {
>> + child_irq = irq_linear_revmap(data->irq, i);
>> + handle_nested_irq(child_irq);
>> + handled++;
>> + }
>> + }
>> + return handled ? IRQ_HANDLED : IRQ_NONE;
>> +}
>> +
>> +static void pca954x_irq_mask(struct irq_data *idata)
>> +{
>> + struct pca954x *data = irq_data_get_irq_chip_data(idata);
>> + unsigned int pos = idata->hwirq;
>> +
>> + data->irq_mask &= ~BIT(pos);
>> + if (!data->irq_mask)
>> + disable_irq(data->client->irq);
>> +}
>> +
>> +static void pca954x_irq_unmask(struct irq_data *idata)
>> +{
>> + struct pca954x *data = irq_data_get_irq_chip_data(idata);
>> + unsigned int pos = idata->hwirq;
>> +
>> + if (!data->irq_mask)
>> + enable_irq(data->client->irq);
>> + data->irq_mask |= BIT(pos);
>> +}
>
> I assume the irq core makes sure that .irq_mask and .irq_unmask may not
> be called concurrently?
I also wasn't 100% sure about this myself.
Looking again at other drivers and the core irq code I'm still not sure.
But I think on review a lock of some kind is needed.
I think a spin_lock_irqsave would do it.
Will test next week and post a new version.
>
>> +
>> +static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
>> +{
>> + if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW)
>> + return -EINVAL;
>> + return 0;
>> +}
>> +
>> +static struct irq_chip pca954x_irq_chip = {
>> + .name = "i2c-mux-pca954x",
>> + .irq_mask = pca954x_irq_mask,
>> + .irq_unmask = pca954x_irq_unmask,
>> + .irq_set_type = pca954x_irq_set_type,
>> +};
>> +
>> +static int pca954x_irq_setup(struct i2c_mux_core *muxc)
>> +{
>> + struct pca954x *data = i2c_mux_priv(muxc);
>> + struct i2c_client *client = data->client;
>> + int c, err, irq;
>> +
>> + if (!data->chip->has_irq || client->irq <= 0)
>> + return 0;
>
> I assume "client->irq <= 0" means that users not specifying any interrupts
> continue to behave as they use to, right?
>
> BTW, what does client->irq == 0 represent?
This one I'm fairly confident on. Initially I had "client->irq < 0"
Which caused problems with an i2c mux that doesn't have irq's defined in the dt.
This commit seems to confirm that.
commit dab472eb931b ("i2c / ACPI: Use 0 to indicate that device does not have interrupt assigned")
And various patches converting to "<= 0" checks.
eg:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-August/364630.html
>
>
>
>> +
>> + data->irq = irq_domain_add_linear(client->dev.of_node,
>> + data->chip->nchans,
>> + &irq_domain_simple_ops, data);
>> + if (!data->irq)
>> + return -ENODEV;
>> +
>> + for (c = 0; c < data->chip->nchans; c++) {
>> + irq = irq_create_mapping(data->irq, c);
>> + irq_set_chip_data(irq, data);
>> + irq_set_chip_and_handler(irq, &pca954x_irq_chip,
>> + handle_simple_irq);
>> + }
>> +
>> + err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
>> + pca954x_irq_handler,
>> + IRQF_ONESHOT | IRQF_SHARED,
>> + "pca954x", data);
>> + if (err)
>> + goto err_req_irq;
>> +
>> + disable_irq(data->client->irq);
>> +
>> + return 0;
>> +err_req_irq:
>> + for (c = 0; c < data->chip->nchans; c++) {
>> + irq = irq_find_mapping(data->irq, c);
>> + irq_dispose_mapping(irq);
>> + }
>> + irq_domain_remove(data->irq);
>> +
>> + return err;
>> +}
>> +
>> /*
>> * I2C init/probing/exit functions
>> */
>> @@ -286,6 +395,10 @@ static int pca954x_probe(struct i2c_client *client,
>> idle_disconnect_dt = of_node &&
>> of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
>>
>> + ret = pca954x_irq_setup(muxc);
>> + if (ret)
>> + goto fail_del_adapters;
>> +
>> /* Now create an adapter for each channel */
>> for (num = 0; num < data->chip->nchans; num++) {
>> bool idle_disconnect_pd = false;
>> @@ -311,7 +424,7 @@ static int pca954x_probe(struct i2c_client *client,
>> dev_err(&client->dev,
>> "failed to register multiplexed adapter"
>> " %d as bus %d\n", num, force);
>> - goto virt_reg_failed;
>> + goto fail_del_adapters;
>> }
>> }
>>
>> @@ -322,7 +435,7 @@ static int pca954x_probe(struct i2c_client *client,
>>
>> return 0;
>>
>> -virt_reg_failed:
>> +fail_del_adapters:
>> i2c_mux_del_adapters(muxc);
>> return ret;
>> }
>> @@ -330,6 +443,16 @@ static int pca954x_probe(struct i2c_client *client,
>> static int pca954x_remove(struct i2c_client *client)
>> {
>> struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>> + struct pca954x *data = i2c_mux_priv(muxc);
>> + int c, irq;
>> +
>> + if (data->irq) {
>> + for (c = 0; c < data->chip->nchans; c++) {
>> + irq = irq_find_mapping(data->irq, c);
>> + irq_dispose_mapping(irq);
>> + }
>> + irq_domain_remove(data->irq);
>> + }
>>
>> i2c_mux_del_adapters(muxc);
>> return 0;
>>
>
>
>
--
Regards
Phil Reid
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v1 2/2] arm: dts: mt2701: add nor flash node
From: Marek Vasut @ 2017-01-13 16:13 UTC (permalink / raw)
To: Matthias Brugger, Boris Brezillon, Guochun Mao
Cc: David Woodhouse, Brian Norris, Richard Weinberger,
Cyrille Pitchen, Rob Herring, Mark Rutland, Russell King,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d7b866ad-5b2f-62d3-3aa3-6fa42803d625-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 01/13/2017 04:12 PM, Matthias Brugger wrote:
>
>
> On 13/01/17 15:17, Boris Brezillon wrote:
>> On Fri, 13 Jan 2017 15:13:29 +0800
>> Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>>
>>> Add Mediatek nor flash node.
>>>
>>> Signed-off-by: Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>> ---
>>> arch/arm/boot/dts/mt2701-evb.dts | 25 +++++++++++++++++++++++++
>>> arch/arm/boot/dts/mt2701.dtsi | 12 ++++++++++++
>>> 2 files changed, 37 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/mt2701-evb.dts
>>> b/arch/arm/boot/dts/mt2701-evb.dts
>>> index 082ca88..85e5ae8 100644
>>> --- a/arch/arm/boot/dts/mt2701-evb.dts
>>> +++ b/arch/arm/boot/dts/mt2701-evb.dts
>>> @@ -24,6 +24,31 @@
>>> };
>>> };
>>>
>>> +&nor_flash {
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&nor_pins_default>;
>>> + status = "okay";
>>> + flash@0 {
>>> + compatible = "jedec,spi-nor";
>>> + reg = <0>;
>>> + };
>>> +};
>>> +
>>> +&pio {
>>> + nor_pins_default: nor {
>>> + pins1 {
>>> + pinmux = <MT2701_PIN_240_EXT_XCS__FUNC_EXT_XCS>,
>>> + <MT2701_PIN_241_EXT_SCK__FUNC_EXT_SCK>,
>>> + <MT2701_PIN_239_EXT_SDIO0__FUNC_EXT_SDIO0>,
>>> + <MT2701_PIN_238_EXT_SDIO1__FUNC_EXT_SDIO1>,
>>> + <MT2701_PIN_237_EXT_SDIO2__FUNC_EXT_SDIO2>,
>>> + <MT2701_PIN_236_EXT_SDIO3__FUNC_EXT_SDIO3>;
>>> + drive-strength = <MTK_DRIVE_4mA>;
>>> + bias-pull-up;
>>> + };
>>> + };
>>> +};
>>> +
>>> &uart0 {
>>> status = "okay";
>>> };
>>> diff --git a/arch/arm/boot/dts/mt2701.dtsi
>>> b/arch/arm/boot/dts/mt2701.dtsi
>>> index bdf8954..1eefce4 100644
>>> --- a/arch/arm/boot/dts/mt2701.dtsi
>>> +++ b/arch/arm/boot/dts/mt2701.dtsi
>>> @@ -227,6 +227,18 @@
>>> status = "disabled";
>>> };
>>>
>>> + nor_flash: spi@11014000 {
>>> + compatible = "mediatek,mt2701-nor",
>>> + "mediatek,mt8173-nor";
>>
>> Why define both here? Is "mediatek,mt8173-nor" really providing a
>> subset of the features supported by "mediatek,mt2701-nor"?
>>
>
> I think even if the ip block is the same, we should provide both
> bindings, just in case in the future we find out that mt2701 has some
> hidden bug, feature or bug-feature. This way even if we update the
> driver, we stay compatible with older device tree blobs in the wild.
>
> We can drop the mt2701-nor in the bindings definition if you want.
This exactly. We should have a DT compat in the form:
compatible = "vendor,<soc>-block", "vendor,<oldest-compat-soc>-block";
Then if we find a problem in the future, we can match on the
"vendor,<soc>-block" and still support the old DTs.
The question is, does the "vendor,<soc>-block" go into the binding
document as well or do we only have "vendor,<oldest-compat-soc>-block"
there ?
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 0/4] Add support for es8388 on the rock2
From: Romain Perier @ 2017-01-13 16:14 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
Romain Perier
This set of patches adds a machine driver for rockchip boards that use
ES8388 codecs. It also adds slave mode to the es8328 driver that
currently only supported the master mode. Then, it adds the required DT
definitions to link rockchip-i2s to the es8388 analog output.
This work is based on the initial work that was done by Sjoerd Simons
<sjoerd.simons@collabora.co.uk> with some improvements, changes and more
commits.
Romain Perier (4):
ASoC: es8328-i2c: Add compatible for ES8388
ASoC: es8328: Add support for slave mode
ASoC: rockchip: Add machine driver for ES8388 codecs
arm: dts: Add support for ES8388 to the Radxa Rock 2
Documentation/devicetree/bindings/sound/es8328.txt | 2 +-
.../devicetree/bindings/sound/rockchip-es8388.txt | 28 +++
arch/arm/boot/dts/rk3288-rock2-square.dts | 37 +++
sound/soc/codecs/es8328-i2c.c | 3 +-
sound/soc/codecs/es8328.c | 28 ++-
sound/soc/rockchip/Kconfig | 9 +
sound/soc/rockchip/Makefile | 2 +
sound/soc/rockchip/rockchip_es8388.c | 270 +++++++++++++++++++++
8 files changed, 371 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/sound/rockchip-es8388.txt
create mode 100644 sound/soc/rockchip/rockchip_es8388.c
--
2.9.3
^ permalink raw reply
* [PATCH 1/4] ASoC: es8328-i2c: Add compatible for ES8388
From: Romain Perier @ 2017-01-13 16:14 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
Romain Perier
In-Reply-To: <20170113161452.14313-1-romain.perier@collabora.com>
This commit adds a compatible string for everest,es8388. This is
an audio codec that is compatible with es8328.
Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
Documentation/devicetree/bindings/sound/es8328.txt | 2 +-
sound/soc/codecs/es8328-i2c.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/es8328.txt b/Documentation/devicetree/bindings/sound/es8328.txt
index 30ea8a3..33fbf05 100644
--- a/Documentation/devicetree/bindings/sound/es8328.txt
+++ b/Documentation/devicetree/bindings/sound/es8328.txt
@@ -4,7 +4,7 @@ This device supports both I2C and SPI.
Required properties:
- - compatible : "everest,es8328"
+ - compatible : Should be "everest,es8328" or "everest,es8388"
- DVDD-supply : Regulator providing digital core supply voltage 1.8 - 3.6V
- AVDD-supply : Regulator providing analog supply voltage 3.3V
- PVDD-supply : Regulator providing digital IO supply voltage 1.8 - 3.6V
diff --git a/sound/soc/codecs/es8328-i2c.c b/sound/soc/codecs/es8328-i2c.c
index 2d05b5d..09ae67f 100644
--- a/sound/soc/codecs/es8328-i2c.c
+++ b/sound/soc/codecs/es8328-i2c.c
@@ -19,13 +19,14 @@
#include "es8328.h"
static const struct i2c_device_id es8328_id[] = {
- { "es8328", 0 },
+ { "es8328", "es8388", 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, es8328_id);
static const struct of_device_id es8328_of_match[] = {
{ .compatible = "everest,es8328", },
+ { .compatible = "everest,es8388", },
{ }
};
MODULE_DEVICE_TABLE(of, es8328_of_match);
--
2.9.3
^ permalink raw reply related
* [PATCH 2/4] ASoC: es8328: Add support for slave mode
From: Romain Perier @ 2017-01-13 16:14 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
Romain Perier
In-Reply-To: <20170113161452.14313-1-romain.perier@collabora.com>
Currently, the function that changes the DAI format only supports master
mode. Trying to use a slave mode exits the function with -EINVAL and
leave the codec misconfigured. This commits adds support for enabling
the slave mode.
Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
sound/soc/codecs/es8328.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/es8328.c b/sound/soc/codecs/es8328.c
index 37722194..054e123 100644
--- a/sound/soc/codecs/es8328.c
+++ b/sound/soc/codecs/es8328.c
@@ -588,10 +588,18 @@ static int es8328_set_dai_fmt(struct snd_soc_dai *codec_dai,
struct snd_soc_codec *codec = codec_dai->codec;
u8 dac_mode = 0;
u8 adc_mode = 0;
+ bool master;
- /* set master/slave audio interface */
- if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBM_CFM)
- return -EINVAL;
+ switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+ case SND_SOC_DAIFMT_CBM_CFM:
+ master = true;
+ break;
+ case SND_SOC_DAIFMT_CBS_CFS:
+ master = false;
+ break;
+ default:
+ return -EINVAL;
+ }
/* interface format */
switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
@@ -620,9 +628,17 @@ static int es8328_set_dai_fmt(struct snd_soc_dai *codec_dai,
snd_soc_update_bits(codec, ES8328_ADCCONTROL4,
ES8328_ADCCONTROL4_ADCFORMAT_MASK, adc_mode);
- /* Master serial port mode, with BCLK generated automatically */
- snd_soc_update_bits(codec, ES8328_MASTERMODE,
- ES8328_MASTERMODE_MSC, ES8328_MASTERMODE_MSC);
+ if (master) {
+ /* Master serial port mode, with BCLK generated automatically */
+ snd_soc_update_bits(codec, ES8328_MASTERMODE,
+ ES8328_MASTERMODE_MSC,
+ ES8328_MASTERMODE_MSC);
+ } else {
+ /* Slave serial port mode */
+ snd_soc_update_bits(codec, ES8328_MASTERMODE,
+ ES8328_MASTERMODE_MSC,
+ 0);
+ }
return 0;
}
--
2.9.3
^ permalink raw reply related
* [PATCH 3/4] ASoC: rockchip: Add machine driver for ES8388 codecs
From: Romain Perier @ 2017-01-13 16:14 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
Romain Perier
In-Reply-To: <20170113161452.14313-1-romain.perier@collabora.com>
The driver is used for Rockchip boards using an audio codec compatible
with ES8388.
This commit is based on the initial work that was done by Sjoerd Simons
<sjoerd.simons@collabora.co.uk> with some improvements.
Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
.../devicetree/bindings/sound/rockchip-es8388.txt | 28 +++
sound/soc/rockchip/Kconfig | 9 +
sound/soc/rockchip/Makefile | 2 +
sound/soc/rockchip/rockchip_es8388.c | 270 +++++++++++++++++++++
4 files changed, 309 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/rockchip-es8388.txt
create mode 100644 sound/soc/rockchip/rockchip_es8388.c
diff --git a/Documentation/devicetree/bindings/sound/rockchip-es8388.txt b/Documentation/devicetree/bindings/sound/rockchip-es8388.txt
new file mode 100644
index 0000000..03a0507
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/rockchip-es8388.txt
@@ -0,0 +1,28 @@
+ROCKCHIP with ES8388 CODECS
+
+Required properties:
+- compatible: "rockchip,rockchip-audio-es8388"
+- rockchip,model: The user-visible name of this sound complex
+- rockchip,i2s-controller: The phandle of the Rockchip I2S controller that's
+ connected to the CODEC
+- rockchip,audio-codec: The phandle of the es8388 audio codec
+
+Optionnal properties:
+- rockchip,hp-en-gpios = The phandle of the GPIO that power up/down the
+ headphone.
+- rockchip,hp-det-gpios = The phandle of the GPIO that detects the headphone
+- pinctrl-names, pinctrl-0: Please refer to pinctrl-bindings.txt
+
+Example:
+
+sound {
+ compatible = "rockchip,rockchip-audio-es8388";
+ rockchip,model = "Analog audio output";
+ rockchip,i2s-controller = <&i2s>;
+ rockchip,audio-codec = <&es8388>;
+ rockchip,hp-en-gpios = <&gpio8 0 GPIO_ACTIVE_HIGH>;
+ rockchip,hp-det-gpios = <&gpio7 7 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&headphone>;
+};
+
diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index c783f9a..ec982c5 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -42,6 +42,15 @@ config SND_SOC_ROCKCHIP_RT5645
Say Y or M here if you want to add support for SoC audio on Rockchip
boards using the RT5645/RT5650 codec, such as Veyron.
+config SND_SOC_ROCKCHIP_ES8388
+ tristate "ASoC support for Rockchip boards using a ES8388 codec"
+ depends on SND_SOC_ROCKCHIP && I2C && GPIOLIB
+ select SND_SOC_ROCKCHIP_I2S
+ select SND_SOC_ES8328_I2C if I2C
+ help
+ Say Y or M here if you want to add support for SoC audio on Rockchip
+ boards using the ES8388 Codec such as Radxa Rock 2
+
config SND_SOC_RK3399_GRU_SOUND
tristate "ASoC support multiple codecs for Rockchip RK3399 GRU boards"
depends on SND_SOC_ROCKCHIP && I2C && GPIOLIB && CLKDEV_LOOKUP && SPI
diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
index 84e5c7c..759db5e 100644
--- a/sound/soc/rockchip/Makefile
+++ b/sound/soc/rockchip/Makefile
@@ -7,8 +7,10 @@ obj-$(CONFIG_SND_SOC_ROCKCHIP_SPDIF) += snd-soc-rockchip-spdif.o
snd-soc-rockchip-max98090-objs := rockchip_max98090.o
snd-soc-rockchip-rt5645-objs := rockchip_rt5645.o
+snd-soc-rockchip-es8388-objs := rockchip_es8388.o
snd-soc-rk3399-gru-sound-objs := rk3399_gru_sound.o
obj-$(CONFIG_SND_SOC_ROCKCHIP_MAX98090) += snd-soc-rockchip-max98090.o
obj-$(CONFIG_SND_SOC_ROCKCHIP_RT5645) += snd-soc-rockchip-rt5645.o
+obj-$(CONFIG_SND_SOC_ROCKCHIP_ES8388) += snd-soc-rockchip-es8388.o
obj-$(CONFIG_SND_SOC_RK3399_GRU_SOUND) += snd-soc-rk3399-gru-sound.o
diff --git a/sound/soc/rockchip/rockchip_es8388.c b/sound/soc/rockchip/rockchip_es8388.c
new file mode 100644
index 0000000..b23ce8e
--- /dev/null
+++ b/sound/soc/rockchip/rockchip_es8388.c
@@ -0,0 +1,270 @@
+/*
+ * Rockchip machine ASoC driver for boards using a ES8388 CODEC.
+ *
+ * Copyright (c) 2016, Collabora Ltd.
+ *
+ * Authors: Sjoerd Simons <sjoerd.simons@collabora.com>,
+ * Romain Perier <romain.perier@collabora.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <sound/core.h>
+#include <sound/jack.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+
+#include "rockchip_i2s.h"
+
+#define DRV_NAME "rockchip-snd-es8388"
+
+struct rk_es8388_drvdata {
+ int gpio_hp_en;
+ int gpio_hp_det;
+};
+
+static int rk_es8388_hp_power(struct snd_soc_dapm_widget *w,
+ struct snd_kcontrol *k, int event)
+{
+ struct rk_es8388_drvdata *machine = snd_soc_card_get_drvdata(w->dapm->card);
+
+ if (!gpio_is_valid(machine->gpio_hp_en))
+ return 0;
+
+ gpio_set_value_cansleep(machine->gpio_hp_en,
+ SND_SOC_DAPM_EVENT_ON(event));
+
+ return 0;
+}
+
+static struct snd_soc_jack headphone_jack;
+static struct snd_soc_jack_pin headphone_jack_pins[] = {
+ {
+ .pin = "Headphone",
+ .mask = SND_JACK_HEADPHONE
+ },
+};
+
+static const struct snd_soc_dapm_widget rk_dapm_widgets[] = {
+ SND_SOC_DAPM_HP("Headphone", rk_es8388_hp_power),
+};
+
+static const struct snd_soc_dapm_route rk_audio_map[] = {
+ {"Headphone", NULL, "LOUT2"},
+ {"Headphone", NULL, "ROUT2"},
+};
+
+static const struct snd_kcontrol_new rk_mc_controls[] = {
+ SOC_DAPM_PIN_SWITCH("Headphone"),
+};
+
+static int rk_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params)
+{
+ int ret = 0;
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *codec_dai = rtd->codec_dai;
+ int mclk;
+
+ switch (params_rate(params)) {
+ case 8000:
+ case 16000:
+ case 24000:
+ case 32000:
+ case 48000:
+ case 64000:
+ case 96000:
+ mclk = 12288000;
+ break;
+ case 11025:
+ case 22050:
+ case 44100:
+ case 88200:
+ mclk = 11289600;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
+ SND_SOC_CLOCK_OUT);
+
+ if (ret && ret != -ENOTSUPP) {
+ dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+ return ret;
+ }
+
+ ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
+ SND_SOC_CLOCK_IN);
+ if (ret && ret != -ENOTSUPP) {
+ dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static struct snd_soc_jack_gpio rk_hp_jack_gpio = {
+ .name = "Headphone detection",
+ .report = SND_JACK_HEADPHONE,
+ .debounce_time = 150
+};
+
+static int rk_init(struct snd_soc_pcm_runtime *runtime)
+{
+ struct rk_es8388_drvdata *machine = snd_soc_card_get_drvdata(runtime->card);
+
+ /* Enable Headset Jack detection */
+ if (gpio_is_valid(machine->gpio_hp_det)) {
+ snd_soc_card_jack_new(runtime->card, "Headphone Jack",
+ SND_JACK_HEADPHONE, &headphone_jack,
+ headphone_jack_pins,
+ ARRAY_SIZE(headphone_jack_pins));
+ rk_hp_jack_gpio.gpio = machine->gpio_hp_det;
+ snd_soc_jack_add_gpios(&headphone_jack, 1, &rk_hp_jack_gpio);
+ }
+
+ return 0;
+}
+
+static struct snd_soc_ops rk_ops = {
+ .hw_params = rk_hw_params,
+};
+
+static struct snd_soc_dai_link rk_dailink = {
+ .name = "ES8388",
+ .stream_name = "Audio",
+ .codec_dai_name = "es8328-hifi-analog",
+ .init = rk_init,
+ .ops = &rk_ops,
+ /* Set es8388 as slave */
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_CBS_CFS,
+};
+
+static struct snd_soc_card snd_soc_card_rk = {
+ .name = "I2S-ES8388",
+ .dai_link = &rk_dailink,
+ .num_links = 1,
+ .num_aux_devs = 0,
+ .dapm_widgets = rk_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(rk_dapm_widgets),
+ .dapm_routes = rk_audio_map,
+ .num_dapm_routes = ARRAY_SIZE(rk_audio_map),
+ .controls = rk_mc_controls,
+ .num_controls = ARRAY_SIZE(rk_mc_controls),
+};
+
+static int snd_rk_mc_probe(struct platform_device *pdev)
+{
+ int ret = 0;
+ struct snd_soc_card *card = &snd_soc_card_rk;
+ struct device_node *np = pdev->dev.of_node;
+ struct rk_es8388_drvdata *machine;
+
+ machine = devm_kzalloc(&pdev->dev, sizeof(struct rk_es8388_drvdata),
+ GFP_KERNEL);
+
+ if (!machine)
+ return -ENOMEM;
+
+ card->dev = &pdev->dev;
+
+ machine->gpio_hp_det = of_get_named_gpio(np,
+ "rockchip,hp-det-gpios", 0);
+ if (machine->gpio_hp_det == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ machine->gpio_hp_en = of_get_named_gpio(np,
+ "rockchip,hp-en-gpios", 0);
+ if (machine->gpio_hp_en == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ if (gpio_is_valid(machine->gpio_hp_en)) {
+ ret = devm_gpio_request_one(&pdev->dev, machine->gpio_hp_en,
+ GPIOF_OUT_INIT_LOW, "hp_en");
+ if (ret) {
+ dev_err(card->dev, "cannot get hp_en gpio\n");
+ return ret;
+ }
+ }
+
+ ret = snd_soc_of_parse_card_name(card, "rockchip,model");
+ if (ret) {
+ dev_err(card->dev, "SoC parse card name failed %d\n", ret);
+ return ret;
+ }
+
+ rk_dailink.codec_of_node = of_parse_phandle(np, "rockchip,audio-codec",
+ 0);
+ if (!rk_dailink.codec_of_node) {
+ dev_err(&pdev->dev,
+ "Property 'rockchip,audio-codec' missing or invalid\n");
+ return -EINVAL;
+ }
+
+ rk_dailink.cpu_of_node = of_parse_phandle(np, "rockchip,i2s-controller",
+ 0);
+ if (!rk_dailink.cpu_of_node) {
+ dev_err(&pdev->dev,
+ "Property 'rockchip,i2s-controller' missing or invalid\n");
+ return -EINVAL;
+ }
+
+ rk_dailink.platform_of_node = rk_dailink.cpu_of_node;
+ snd_soc_card_set_drvdata(card, machine);
+
+ ret = devm_snd_soc_register_card(&pdev->dev, card);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Soc register card failed %d\n", ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, card);
+
+ return ret;
+}
+
+static const struct of_device_id rockchip_es8388_of_match[] = {
+ { .compatible = "rockchip,rockchip-audio-es8388", },
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, rockchip_es8388_of_match);
+
+static struct platform_driver snd_rk_es8388_driver = {
+ .probe = snd_rk_mc_probe,
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ .pm = &snd_soc_pm_ops,
+ .of_match_table = rockchip_es8388_of_match,
+ },
+};
+
+module_platform_driver(snd_rk_es8388_driver);
+
+MODULE_AUTHOR("Sjoerd Simons");
+MODULE_DESCRIPTION("Rockchip es8388 machine ASoC driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
--
2.9.3
^ permalink raw reply related
* [PATCH 4/4] arm: dts: Add support for ES8388 to the Radxa Rock 2
From: Romain Perier @ 2017-01-13 16:14 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
Romain Perier
In-Reply-To: <20170113161452.14313-1-romain.perier@collabora.com>
This commit adds the DT definition of the es8388 i2c device
found at address 0x10. It also adds the definition for connecting
the Rockchip I2S to the es8388 analog output.
This commit is based on the initial work that was done by Sjoerd Simons
<sjoerd.simons@collabora.com> with some improvements.
Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
arch/arm/boot/dts/rk3288-rock2-square.dts | 37 +++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts b/arch/arm/boot/dts/rk3288-rock2-square.dts
index 96a2e74..4f9b7d2 100644
--- a/arch/arm/boot/dts/rk3288-rock2-square.dts
+++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
@@ -86,6 +86,17 @@
#sound-dai-cells = <0>;
};
+ sound_es8388 {
+ compatible = "rockchip,rockchip-audio-es8388";
+ rockchip,model = "Analog audio output";
+ rockchip,i2s-controller = <&i2s>;
+ rockchip,audio-codec = <&es8388>;
+ rockchip,hp-en-gpios = <&gpio8 0 GPIO_ACTIVE_HIGH>;
+ rockchip,hp-det-gpios = <&gpio7 7 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&headphone>;
+ };
+
sdio_pwrseq: sdio-pwrseq {
compatible = "mmc-pwrseq-simple";
clocks = <&hym8563>;
@@ -173,10 +184,29 @@
};
};
+&i2c2 {
+ status = "okay";
+
+ es8388: es8388@10 {
+ compatible = "everest,es8388", "everest,es8328";
+ reg = <0x10>;
+ AVDD-supply = <&vcca_codec>;
+ DVDD-supply = <&vcca_codec>;
+ HPVDD-supply = <&vcca_codec>;
+ PVDD-supply = <&vcca_codec>;
+ clocks = <&cru SCLK_I2S0_OUT>;
+ clock-names = "i2s_clk_out";
+ };
+};
+
&i2c5 {
status = "okay";
};
+&i2s {
+ status = "okay";
+};
+
&pinctrl {
ir {
ir_int: ir-int {
@@ -190,6 +220,13 @@
};
};
+ sound {
+ headphone: headphone {
+ rockchip,pins = <8 0 RK_FUNC_GPIO &pcfg_pull_up>,
+ <7 7 RK_FUNC_GPIO &pcfg_pull_none>;
+ };
+ };
+
usb {
host_vbus_drv: host-vbus-drv {
rockchip,pins = <0 14 RK_FUNC_GPIO &pcfg_pull_none>;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH linux v1 1/2] Documentation: dt-bindings: Document bindings for ASPEED AST2400/AST2500 pwm and fan tach controller device driver
From: Rob Herring @ 2017-01-13 16:21 UTC (permalink / raw)
To: Jaghathiswari Rankappagounder Natarajan
Cc: openbmc, joel, jdelvare, linux, linux-hwmon, linux-kernel, corbet,
linux-doc, mark.rutland, devicetree
In-Reply-To: <20170109215935.30067-2-jaghu@google.com>
On Mon, Jan 09, 2017 at 01:59:34PM -0800, Jaghathiswari Rankappagounder Natarajan wrote:
> This binding provides interface for adding values related to ASPEED
> AST2400/2500 PWM and Fan tach controller support.
> The PWM controller can support upto 8 PWM output ports.
> The Fan tach controller can support upto 16 tachometer inputs.
> PWM clock types M, N and 0 are three types just to have three independent
> PWM sources.
>
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
> ---
> .../devicetree/bindings/hwmon/aspeed-pwm-tacho.txt | 153 +++++++++++++++++++++
Perhaps bindings/pwm/... even though this is more than just PWM.
> 1 file changed, 153 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
>
> diff --git a/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
> new file mode 100644
> index 000000000000..8f346409ee8c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
> @@ -0,0 +1,153 @@
> +ASPEED AST2400/AST2500 PWM and Fan Tacho controller device driver
> +
> +The ASPEED PWM controller can support upto 8 PWM outputs. The ASPEED Fan Tacho
> +controller can support upto 16 tachometer inputs. The PWM controller supports
> +3 types of frequency mode PWM for fan speed control. PWM clock types M, N and 0
> +are 3 types of frequency mode PWM just to have 3 independent PWM sources.
> +
> +Required properties for pwm_tacho node:
> +- #address-cells : should be 1.
> +
> +- #size-cells : should be 1.
> +
> +- reg : address and length of the register set for the device.
> +
> +- pinctrl-names : a pinctrl state named "default" must be defined.
> +
> +- pinctrl-0 : phandle referencing pin configuration of the AST2400/AST2500 PWM
> + ports.
> +
> +- compatible : should be "aspeed,aspeed2400-pwm-tacho" for AST2400 or
> + "aspeed,aspeed2500-pwm-tacho" for AST2500.
> +
> +- clocks : a fixed clock providing input clock frequency(PWM
> + and Fan Tach clock)
> +
> +type_values subnode format:
Don't use '_' in node or property names.
> +===========================
> +Under type_values subnode there can be upto 3 child nodes indicating type M/N/O
> +values. Atleast one child node is required.
> +
> +Required properties for the child node(type M/N/O):
> +- pwm_period : indicates type M/N/O PWM period, as per the AST2400/AST2500
> + datasheet. integer value in the range 0 to 255.
> +
> +- pwm_clock_division_l : indicates type M/N/O PWM clock division L value,
> + as per the AST2400/AST2500 datasheet.
> + integer value in the range 0 to 15.
> + 0 here indicates divide 1, 1 indicates divide 2,
> + 2 indicates divide 4, 3 indicates divide 6, and so on
> + till 15 indicates divide 30.
> +
> +- pwm_clock_division_h : indicates type M/N/O PWM clock division H value,
> + as per the AST2400/AST2500 datasheet.
> + integer value in the range 0 to 15.
> + 0 here indicates divide 1, 1 indicates divide 2,
> + 2 indicates divide 4, 3 indicates divide 8, and so on
> + till 15 indicates divide 32768.
Can't you have a single divider value and driver convert to register
values? Really, you should specify the PWM period in ns/us and calculate
the divider based on the input clock freq. (i.e. use the clock binding).
There's already PWM binding to specify the period.
I think you should have a node for the fan using the PWM binding and
perhaps moving some of these properties to the fan node.
> +
> +- fan_tach_enable : indicates fan tach enable of type M/N/O as per the
> + AST2400/AST2500 datasheet. boolean value.
> +
> +- fan_tach_clock_division : indicates fan tach clock division as per the
> + AST2400/AST2500 datasheet.
> + integer value in the range 0 to 7.
> + 0 indicates divide 4, 1 indicates divide 16,
> + 2 indicates divide 64, 3 indicates divide 256
> + and so on till 7 indicates divide 65536.
> +
> +- fan_tach_mode_selection : indicates fan tach mode mode selection as per the
> + AST2400/AST2500 datasheet. integer value in the
> + range 0 to 2. 0 indicates falling edge, 1 indicates
> + rising edge and 2 indicates both edges.
> +
> +- fan_tach_period : indicates fan tach period as per the AST2400/AST2500
> + datasheet. integer value (can be upto 16 bits long).
> +
> +pwm_port subnode format:
> +========================
> +Under pwm_port subnode there can upto 8 child nodes each indicating values
> +for one of the 8 PWM output ports.
> +
> +Required properties for each child node(starting from PWM A through PWM H):
> +- enable : enable PWM #X port, X ranges from A through H. boolean value.
A connection in the PWM binding would imply this.
> +
> +- type : indicates type selection value of PWM #X port, X ranges from A
> + through H. integer value in the range 0 to 2;
> + 0 indicates type M, 1 indicates type N, 2 indicates type O.
This is meaningless to me.
> +- fan_ctrl : set the PWM fan control initial value. integer value between
> + 0(off) and 255(full speed).
> +
> +fan_tach_channel subnode format:
> +================================
> +Under fan_tach_channel subnode there can be upto 16 child nodes each indicating
> +values for one of the 16 fan tach channels.
> +
> +Required properties for each child node(starting from fan tach #0 through
> +fan tach #16):
> +- fan-ctrl-gpios : should specify the tachometer input pin on the hardware.
I don't understand the connection here. You have a tach block with 16
inputs on the SoC, so why the GPIOs? If the GPIOs go to the fan, then a
fan node should have the GPIO properties.
> +
> +- enable : enable fan tach #X, X ranges from 0 through 16. boolean value.
'status' property is the standard way to enable a node or not. Not sure
if that makes sense here though.
> +
> +- pwm_source : indicates PWM source of fan tach #X, X ranges from 0 through 16.
> + integer value in the range 0 to 7. 0 indicates PWM port A,
> + 1 indicates PWM port B and so on till 7 indicates PWM port H.
^ permalink raw reply
* Re: [PATCH v1 2/2] arm: dts: mt2701: add nor flash node
From: Boris Brezillon @ 2017-01-13 16:28 UTC (permalink / raw)
To: Marek Vasut
Cc: Matthias Brugger, Guochun Mao, David Woodhouse, Brian Norris,
Richard Weinberger, Cyrille Pitchen, Rob Herring, Mark Rutland,
Russell King, linux-mtd, devicetree, linux-arm-kernel,
linux-mediatek, linux-kernel
In-Reply-To: <b850b751-1d8d-0053-cf1f-d5314542e5c3@gmail.com>
On Fri, 13 Jan 2017 17:13:55 +0100
Marek Vasut <marek.vasut@gmail.com> wrote:
> On 01/13/2017 04:12 PM, Matthias Brugger wrote:
> >
> >
> > On 13/01/17 15:17, Boris Brezillon wrote:
> >> On Fri, 13 Jan 2017 15:13:29 +0800
> >> Guochun Mao <guochun.mao@mediatek.com> wrote:
> >>
> >>> Add Mediatek nor flash node.
> >>>
> >>> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> >>> ---
> >>> arch/arm/boot/dts/mt2701-evb.dts | 25 +++++++++++++++++++++++++
> >>> arch/arm/boot/dts/mt2701.dtsi | 12 ++++++++++++
> >>> 2 files changed, 37 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/mt2701-evb.dts
> >>> b/arch/arm/boot/dts/mt2701-evb.dts
> >>> index 082ca88..85e5ae8 100644
> >>> --- a/arch/arm/boot/dts/mt2701-evb.dts
> >>> +++ b/arch/arm/boot/dts/mt2701-evb.dts
> >>> @@ -24,6 +24,31 @@
> >>> };
> >>> };
> >>>
> >>> +&nor_flash {
> >>> + pinctrl-names = "default";
> >>> + pinctrl-0 = <&nor_pins_default>;
> >>> + status = "okay";
> >>> + flash@0 {
> >>> + compatible = "jedec,spi-nor";
> >>> + reg = <0>;
> >>> + };
> >>> +};
> >>> +
> >>> +&pio {
> >>> + nor_pins_default: nor {
> >>> + pins1 {
> >>> + pinmux = <MT2701_PIN_240_EXT_XCS__FUNC_EXT_XCS>,
> >>> + <MT2701_PIN_241_EXT_SCK__FUNC_EXT_SCK>,
> >>> + <MT2701_PIN_239_EXT_SDIO0__FUNC_EXT_SDIO0>,
> >>> + <MT2701_PIN_238_EXT_SDIO1__FUNC_EXT_SDIO1>,
> >>> + <MT2701_PIN_237_EXT_SDIO2__FUNC_EXT_SDIO2>,
> >>> + <MT2701_PIN_236_EXT_SDIO3__FUNC_EXT_SDIO3>;
> >>> + drive-strength = <MTK_DRIVE_4mA>;
> >>> + bias-pull-up;
> >>> + };
> >>> + };
> >>> +};
> >>> +
> >>> &uart0 {
> >>> status = "okay";
> >>> };
> >>> diff --git a/arch/arm/boot/dts/mt2701.dtsi
> >>> b/arch/arm/boot/dts/mt2701.dtsi
> >>> index bdf8954..1eefce4 100644
> >>> --- a/arch/arm/boot/dts/mt2701.dtsi
> >>> +++ b/arch/arm/boot/dts/mt2701.dtsi
> >>> @@ -227,6 +227,18 @@
> >>> status = "disabled";
> >>> };
> >>>
> >>> + nor_flash: spi@11014000 {
> >>> + compatible = "mediatek,mt2701-nor",
> >>> + "mediatek,mt8173-nor";
> >>
> >> Why define both here? Is "mediatek,mt8173-nor" really providing a
> >> subset of the features supported by "mediatek,mt2701-nor"?
> >>
> >
> > I think even if the ip block is the same, we should provide both
> > bindings, just in case in the future we find out that mt2701 has some
> > hidden bug, feature or bug-feature. This way even if we update the
> > driver, we stay compatible with older device tree blobs in the wild.
> >
> > We can drop the mt2701-nor in the bindings definition if you want.
Oh, sorry, I misunderstood. What I meant is that if you want to
list/support all possible compatibles, maybe you should just put one
compatible in your DT and patch your driver (+ binding doc) to define
all of them.
>
> This exactly. We should have a DT compat in the form:
> compatible = "vendor,<soc>-block", "vendor,<oldest-compat-soc>-block";
> Then if we find a problem in the future, we can match on the
> "vendor,<soc>-block" and still support the old DTs.
Not sure it's only in term of whose IP appeared first. My understanding
is that it's a way to provide inheritance. For example:
"<soc-vendor>,<ip-version>", "<ip-vendor>,<ip-version>";
or
"<soc-vendor>,<full-featured-ip-version>","<soc-vendor>,<basic-feature-ip-version>";
BTW, which one is the oldest between mt8173 and mt2701? :-)
^ permalink raw reply
* Re: [PATCH 02/37] PCI: dwc: designware: Add new *ops* for cpu addr fixup
From: Joao Pinto @ 2017-01-13 16:34 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Bjorn Helgaas, Jingoo Han, Joao Pinto,
Arnd Bergmann
Cc: linux-pci, linux-doc, linux-kernel, devicetree, linux-omap,
linux-arm-kernel, linux-samsung-soc, linuxppc-dev,
linux-arm-kernel, linux-arm-msm, nsekhar
In-Reply-To: <1484216786-17292-3-git-send-email-kishon@ti.com>
Hi Kishon,
Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> Some platforms (like dra7xx) require only the least 28 bits of the
> corresponding 32 bit CPU address to be programmed in the address
> translation unit. This modified address is stored in io_base/mem_base/
> cfg0_base/cfg1_base in dra7xx_pcie_host_init. While this is okay for
> host mode where the address range is fixed, device mode requires
> different addresses to be programmed based on the host buffer address.
> Add a new ops to get the least 28 bits of the corresponding 32 bit
> CPU address and invoke it before programming the address translation
> unit.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/pci/dwc/pcie-designware.c | 3 +++
> drivers/pci/dwc/pcie-designware.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c
> index bed1999..d68bc7b 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -195,6 +195,9 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
> {
> u32 retries, val;
>
> + if (pp->ops->cpu_addr_fixup)
> + cpu_addr = pp->ops->cpu_addr_fixup(cpu_addr);
> +
> if (pp->iatu_unroll_enabled) {
> dw_pcie_writel_unroll(pp, index, PCIE_ATU_UNR_LOWER_BASE,
> lower_32_bits(cpu_addr));
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index a567ea2..32f4602 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -54,6 +54,7 @@ struct pcie_port {
> };
>
> struct pcie_host_ops {
> + u64 (*cpu_addr_fixup)(u64 cpu_addr);
> u32 (*readl_rc)(struct pcie_port *pp, u32 reg);
> void (*writel_rc)(struct pcie_port *pp, u32 reg, u32 val);
> int (*rd_own_conf)(struct pcie_port *pp, int where, int size, u32 *val);
>
I think this is an acceptable fixup, I am ok with it.
Reviewed-By: Joao Pinto <jpinto@synopsys.com>
Joao
^ permalink raw reply
* Re: [PATCH 04/37] PCI: dwc: designware: Move the register defines to designware header file
From: Joao Pinto @ 2017-01-13 16:35 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Bjorn Helgaas, Jingoo Han, Joao Pinto,
Arnd Bergmann
Cc: linux-pci, linux-doc, linux-kernel, devicetree, linux-omap,
linux-arm-kernel, linux-samsung-soc, linuxppc-dev,
linux-arm-kernel, linux-arm-msm, nsekhar
In-Reply-To: <1484216786-17292-5-git-send-email-kishon@ti.com>
Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> No functional change. Move the register defines and other macros from
> pcie-designware.c to pcie-designware.h. This is in preparation to
> split the pcie-designware.c file into designware core file and host
> specific file.
>
> While at that also fix a checkpatch warning.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/pci/dwc/pcie-designware.c | 70 ------------------------------------
> drivers/pci/dwc/pcie-designware.h | 71 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 71 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c
> index d68bc7b..0b928dc 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -25,76 +25,6 @@
>
> #include "pcie-designware.h"
Make sense.
Reviewed-By: Joao Pinto <jpinto@synopsys.com>
^ permalink raw reply
* Re: [PATCH 06/37] PCI: dwc: Rename cfg_read/cfg_write to read/write
From: Joao Pinto @ 2017-01-13 16:36 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Bjorn Helgaas, Jingoo Han, Joao Pinto,
Arnd Bergmann
Cc: linux-pci, linux-doc, linux-kernel, devicetree, linux-omap,
linux-arm-kernel, linux-samsung-soc, linuxppc-dev,
linux-arm-kernel, linux-arm-msm, nsekhar, Murali Karicheri,
Stanimir Varbanov, Pratyush Anand
In-Reply-To: <1484216786-17292-7-git-send-email-kishon@ti.com>
Hi,
Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> No functional change. dw_pcie_cfg_read/dw_pcie_cfg_write doesn't do
> anything specific to access configuration space. It can be just renamed
> to dw_pcie_read/dw_pcie_write and used to read/write data to dbi space.
> This is in preparation for added endpoint support to linux kernel.
>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: Murali Karicheri <m-karicheri2@ti.com>
> Cc: Joao Pinto <Joao.Pinto@synopsys.com>
> Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
> Cc: Pratyush Anand <pratyush.anand@gmail.com>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/pci/dwc/pci-dra7xx.c | 16 ++++++++--------
> drivers/pci/dwc/pci-exynos.c | 4 ++--
> drivers/pci/dwc/pci-keystone-dw.c | 4 ++--
> drivers/pci/dwc/pcie-designware.c | 12 ++++++------
> drivers/pci/dwc/pcie-designware.h | 4 ++--
> drivers/pci/dwc/pcie-qcom.c | 2 +-
> drivers/pci/dwc/pcie-spear13xx.c | 24 ++++++++++++------------
> 7 files changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index aeeab74..38b0c9a 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -114,22 +114,22 @@ static int dra7xx_pcie_establish_link(struct dra7xx_pcie *dra7xx)
> }
>
> if (dra7xx->link_gen == 1) {
> - dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> - 4, ®);
> + dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> + 4, ®);
> if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
> reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
> reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
> - dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> - PCI_EXP_LNKCAP, 4, reg);
> + dw_pcie_write(pp->dbi_base + exp_cap_off +
> + PCI_EXP_LNKCAP, 4, reg);
> }
>
> - dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> - 2, ®);
> + dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> + 2, ®);
> if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
> reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
> reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
> - dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> - PCI_EXP_LNKCTL2, 2, reg);
> + dw_pcie_write(pp->dbi_base + exp_cap_off +
> + PCI_EXP_LNKCTL2, 2, reg);
> }
> }
>
> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
> index c179e7a..e3fbff4 100644
> --- a/drivers/pci/dwc/pci-exynos.c
> +++ b/drivers/pci/dwc/pci-exynos.c
> @@ -429,7 +429,7 @@ static int exynos_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> int ret;
>
> exynos_pcie_sideband_dbi_r_mode(exynos_pcie, true);
> - ret = dw_pcie_cfg_read(pp->dbi_base + where, size, val);
> + ret = dw_pcie_read(pp->dbi_base + where, size, val);
> exynos_pcie_sideband_dbi_r_mode(exynos_pcie, false);
> return ret;
> }
> @@ -441,7 +441,7 @@ static int exynos_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
> int ret;
>
> exynos_pcie_sideband_dbi_w_mode(exynos_pcie, true);
> - ret = dw_pcie_cfg_write(pp->dbi_base + where, size, val);
> + ret = dw_pcie_write(pp->dbi_base + where, size, val);
> exynos_pcie_sideband_dbi_w_mode(exynos_pcie, false);
> return ret;
> }
> diff --git a/drivers/pci/dwc/pci-keystone-dw.c b/drivers/pci/dwc/pci-keystone-dw.c
> index 9397c46..4875334 100644
> --- a/drivers/pci/dwc/pci-keystone-dw.c
> +++ b/drivers/pci/dwc/pci-keystone-dw.c
> @@ -444,7 +444,7 @@ int ks_dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
>
> addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn);
>
> - return dw_pcie_cfg_read(addr + where, size, val);
> + return dw_pcie_read(addr + where, size, val);
> }
>
> int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> @@ -456,7 +456,7 @@ int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
>
> addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn);
>
> - return dw_pcie_cfg_write(addr + where, size, val);
> + return dw_pcie_write(addr + where, size, val);
> }
>
> /**
> diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c
> index 0b928dc..d0ea310 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -33,7 +33,7 @@
>
> static struct pci_ops dw_pcie_ops;
>
> -int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
> +int dw_pcie_read(void __iomem *addr, int size, u32 *val)
> {
> if ((uintptr_t)addr & (size - 1)) {
> *val = 0;
> @@ -54,7 +54,7 @@ int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
> return PCIBIOS_SUCCESSFUL;
> }
>
> -int dw_pcie_cfg_write(void __iomem *addr, int size, u32 val)
> +int dw_pcie_write(void __iomem *addr, int size, u32 val)
> {
> if ((uintptr_t)addr & (size - 1))
> return PCIBIOS_BAD_REGISTER_NUMBER;
> @@ -108,7 +108,7 @@ static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> if (pp->ops->rd_own_conf)
> return pp->ops->rd_own_conf(pp, where, size, val);
>
> - return dw_pcie_cfg_read(pp->dbi_base + where, size, val);
> + return dw_pcie_read(pp->dbi_base + where, size, val);
> }
>
> static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
> @@ -117,7 +117,7 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
> if (pp->ops->wr_own_conf)
> return pp->ops->wr_own_conf(pp, where, size, val);
>
> - return dw_pcie_cfg_write(pp->dbi_base + where, size, val);
> + return dw_pcie_write(pp->dbi_base + where, size, val);
> }
>
> static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
> @@ -638,7 +638,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
> type, cpu_addr,
> busdev, cfg_size);
> - ret = dw_pcie_cfg_read(va_cfg_base + where, size, val);
> + ret = dw_pcie_read(va_cfg_base + where, size, val);
> if (pp->num_viewport <= 2)
> dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
> PCIE_ATU_TYPE_IO, pp->io_base,
> @@ -676,7 +676,7 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
> type, cpu_addr,
> busdev, cfg_size);
> - ret = dw_pcie_cfg_write(va_cfg_base + where, size, val);
> + ret = dw_pcie_write(va_cfg_base + where, size, val);
> if (pp->num_viewport <= 2)
> dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
> PCIE_ATU_TYPE_IO, pp->io_base,
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index a6cf9262..b6ddb05 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -146,8 +146,8 @@ struct pcie_host_ops {
>
> u32 dw_pcie_readl_rc(struct pcie_port *pp, u32 reg);
> void dw_pcie_writel_rc(struct pcie_port *pp, u32 reg, u32 val);
> -int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val);
> -int dw_pcie_cfg_write(void __iomem *addr, int size, u32 val);
> +int dw_pcie_read(void __iomem *addr, int size, u32 *val);
> +int dw_pcie_write(void __iomem *addr, int size, u32 val);
> irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
> void dw_pcie_msi_init(struct pcie_port *pp);
> int dw_pcie_wait_for_link(struct pcie_port *pp);
> diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c
> index 2c24c45..d75fc02 100644
> --- a/drivers/pci/dwc/pcie-qcom.c
> +++ b/drivers/pci/dwc/pcie-qcom.c
> @@ -630,7 +630,7 @@ static int qcom_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> return PCIBIOS_SUCCESSFUL;
> }
>
> - return dw_pcie_cfg_read(pp->dbi_base + where, size, val);
> + return dw_pcie_read(pp->dbi_base + where, size, val);
> }
>
> static struct pcie_host_ops qcom_pcie_dw_ops = {
> diff --git a/drivers/pci/dwc/pcie-spear13xx.c b/drivers/pci/dwc/pcie-spear13xx.c
> index 5970566..7acf91e 100644
> --- a/drivers/pci/dwc/pcie-spear13xx.c
> +++ b/drivers/pci/dwc/pcie-spear13xx.c
> @@ -91,34 +91,34 @@ static int spear13xx_pcie_establish_link(struct spear13xx_pcie *spear13xx_pcie)
> * default value in capability register is 512 bytes. So force
> * it to 128 here.
> */
> - dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, &val);
> + dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, &val);
> val &= ~PCI_EXP_DEVCTL_READRQ;
> - dw_pcie_cfg_write(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, val);
> + dw_pcie_write(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, val);
>
> - dw_pcie_cfg_write(pp->dbi_base + PCI_VENDOR_ID, 2, 0x104A);
> - dw_pcie_cfg_write(pp->dbi_base + PCI_DEVICE_ID, 2, 0xCD80);
> + dw_pcie_write(pp->dbi_base + PCI_VENDOR_ID, 2, 0x104A);
> + dw_pcie_write(pp->dbi_base + PCI_DEVICE_ID, 2, 0xCD80);
>
> /*
> * if is_gen1 is set then handle it, so that some buggy card
> * also works
> */
> if (spear13xx_pcie->is_gen1) {
> - dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> - 4, &val);
> + dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> + 4, &val);
> if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
> val &= ~((u32)PCI_EXP_LNKCAP_SLS);
> val |= PCI_EXP_LNKCAP_SLS_2_5GB;
> - dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> - PCI_EXP_LNKCAP, 4, val);
> + dw_pcie_write(pp->dbi_base + exp_cap_off +
> + PCI_EXP_LNKCAP, 4, val);
> }
>
> - dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> - 2, &val);
> + dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> + 2, &val);
> if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
> val &= ~((u32)PCI_EXP_LNKCAP_SLS);
> val |= PCI_EXP_LNKCAP_SLS_2_5GB;
> - dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> - PCI_EXP_LNKCTL2, 2, val);
> + dw_pcie_write(pp->dbi_base + exp_cap_off +
> + PCI_EXP_LNKCTL2, 2, val);
> }
> }
>
>
Make sense.
Reviewed-By: Joao Pinto <jpinto@synopsys.com>
^ permalink raw reply
* Re: [PATCH] Documentation: dt-bindings: Add binding documentation for TI clkctrl clocks
From: Rob Herring @ 2017-01-13 16:36 UTC (permalink / raw)
To: Tony Lindgren
Cc: Michael Turquette, Stephen Boyd, Tero Kristo,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Paul Walmsley
In-Reply-To: <20170110154400.GT2630-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
On Tue, Jan 10, 2017 at 07:44:01AM -0800, Tony Lindgren wrote:
> * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [170109 15:43]:
> > Texas Instruments omap variant SoCs starting with omap4 have a clkctrl
> > clock controller instance for each interconnect target module. The clkctrl
> > controls functional and interface clocks for the module.
> >
> > The clkctrl clocks are currently handled by arch/arm/mach-omap2 hwmod code.
> > With this binding and a related clock device driver we can start moving the
> > clkctrl clock handling to live in drivers/clk/ti.
> >
> > For hardware reference, see omap4430 TRM "Table 3-1312. L4PER_CM2 Registers
> > Mapping Summary" for example. It show one instance of a clkctrl clock
> > controller with multiple clkctrl registers.
> >
> > Note that this binding allows keeping the clockdomain related parts out of
> > drivers/clock. The CLKCTCTRL and DYNAMICDEP registers can be handled by
> > using a separate driver in drivers/soc/ti and genpd. If the clockdomain
> > driver needs to know it's clocks, we can just set the the clkctrl device
> > instances to be children of the related clockdomain device.
> >
> > On omap4 CM_L3INIT_USB_HOST_HS_CLKCTRL on omap5 has eight OPTFCLKEN bits.
> > So we need to shift the clock index to avoid index conflict for the clock
> > consumer binding with the next clkctrl offset on omap4.
> >
> > Cc: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> > ---
> >
> > So here's what I was able to come up for the clkctr binding based on
> > all we've discussed so far. Can you guys please take a look and see
> > if it looks OK to you before we do the device driver?
> >
> > Also, does anybody have better suggestions for addressing the optional
> > clocks in each clkctrl register?
>
> The other option that might be worth considering is to make use of the
> #clock-cells property. Then the index of any optional clock could be passed
> in the second cell.
>
> The third cell could be used to set the modulemode for the clock (software
> controlled or hardware controlled) instead of using a custom property
> at the clock controllel level.
I guess I prefer this way. Or you could do a mixture of both proposals
with 2 cells. The first being the clock id and the 2nd flags.
What's the max optional clocks in theory? B picked from the current
worst case seems a bit worrying. Why not 16? Upper half is offset, lower
half is index.
> In that case clock consume usage would look like the following using
> #clock-cells = <3>:
>
> #define OMAP4_CLKCTRL_OFFSET 0x20
> #define MODULEMODE_HWCTRL 1
> #define MODULEMODE_SWCTRL 2
Can you make one of these 0 instead or is both being set valid?
>
> #define OMAP_CLKCTRL_INDEX(offset) ((offset) - OMAP4_CLKCTRL_OFFSET)
>
> #define OMAP4_GPTIMER10_CLKTRL OMAP_CLKCTRL_INDEX(0x28)
> #define OMAP4_GPTIMER11_CLKTRL OMAP_CLKCTRL_INDEX(0x30)
> #define OMAP4_GPTIMER2_CLKTRL OMAP_CLKCTRL_INDEX(0x38)
> ...
> #define OMAP4_GPIO2_CLKCTRL OMAP_CLKCTRL_INDEX(0x60)
> ...
>
> &gpio2 {
> clocks = <&cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL 0 MODULEMODE_HWCTRL
> &cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL_DBCLK 1 0>;
Drop the _DBCLK here, right?
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 10/37] PCI: dwc: designware: Fix style errors in pcie-designware.c
From: Joao Pinto @ 2017-01-13 16:38 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Bjorn Helgaas, Jingoo Han, Joao Pinto,
Arnd Bergmann
Cc: linux-pci, linux-doc, linux-kernel, devicetree, linux-omap,
linux-arm-kernel, linux-samsung-soc, linuxppc-dev,
linux-arm-kernel, linux-arm-msm, nsekhar
In-Reply-To: <1484216786-17292-11-git-send-email-kishon@ti.com>
Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> No functional change. Fix all checkpatch warnings and check errors
> in pcie-designware.c
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/pci/dwc/pcie-designware.c | 42 ++++++++++++++++++-------------------
> 1 file changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c
> index 89cdb6b..ff04074 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -40,13 +40,13 @@ int dw_pcie_read(void __iomem *addr, int size, u32 *val)
> return PCIBIOS_BAD_REGISTER_NUMBER;
> }
>
> - if (size == 4)
> + if (size == 4) {
> *val = readl(addr);
> - else if (size == 2)
> + } else if (size == 2) {
> *val = readw(addr);
> - else if (size == 1)
> + } else if (size == 1) {
> *val = readb(addr);
> - else {
> + } else {
> *val = 0;
> return PCIBIOS_BAD_REGISTER_NUMBER;
> }
> @@ -203,16 +203,15 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>
> for (i = 0; i < MAX_MSI_CTRLS; i++) {
> dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4,
> - (u32 *)&val);
> + (u32 *)&val);
> if (val) {
> ret = IRQ_HANDLED;
> pos = 0;
> while ((pos = find_next_bit(&val, 32, pos)) != 32) {
> irq = irq_find_mapping(pp->irq_domain,
> - i * 32 + pos);
> - dw_pcie_wr_own_conf(pp,
> - PCIE_MSI_INTR0_STATUS + i * 12,
> - 4, 1 << pos);
> + i * 32 + pos);
> + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS +
> + i * 12, 4, 1 << pos);
> generic_handle_irq(irq);
> pos++;
> }
> @@ -278,8 +277,9 @@ static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
> static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
> {
> int irq, pos0, i;
> - struct pcie_port *pp = (struct pcie_port *) msi_desc_to_pci_sysdata(desc);
> + struct pcie_port *pp;
>
> + pp = (struct pcie_port *)msi_desc_to_pci_sysdata(desc);
> pos0 = bitmap_find_free_region(pp->msi_irq_in_use, MAX_MSI_IRQS,
> order_base_2(no_irqs));
> if (pos0 < 0)
> @@ -341,7 +341,7 @@ static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
> }
>
> static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
> - struct msi_desc *desc)
> + struct msi_desc *desc)
> {
> int irq, pos;
> struct pcie_port *pp = pdev->bus->sysdata;
> @@ -389,7 +389,7 @@ static void dw_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
> {
> struct irq_data *data = irq_get_irq_data(irq);
> struct msi_desc *msi = irq_data_get_msi_desc(data);
> - struct pcie_port *pp = (struct pcie_port *) msi_desc_to_pci_sysdata(msi);
> + struct pcie_port *pp = (struct pcie_port *)msi_desc_to_pci_sysdata(msi);
>
> clear_irq_range(pp, irq, 1, data->hwirq);
> }
> @@ -431,7 +431,7 @@ int dw_pcie_link_up(struct dw_pcie *pci)
> }
>
> static int dw_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
> - irq_hw_number_t hwirq)
> + irq_hw_number_t hwirq)
> {
> irq_set_chip_and_handler(irq, &dw_msi_irq_chip, handle_simple_irq);
> irq_set_chip_data(irq, domain->host_data);
> @@ -468,8 +468,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
>
> cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
> if (cfg_res) {
> - pp->cfg0_size = resource_size(cfg_res)/2;
> - pp->cfg1_size = resource_size(cfg_res)/2;
> + pp->cfg0_size = resource_size(cfg_res) / 2;
> + pp->cfg1_size = resource_size(cfg_res) / 2;
> pp->cfg0_base = cfg_res->start;
> pp->cfg1_base = cfg_res->start + pp->cfg0_size;
> } else if (!pp->va_cfg0_base) {
> @@ -508,8 +508,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
> break;
> case 0:
> pp->cfg = win->res;
> - pp->cfg0_size = resource_size(pp->cfg)/2;
> - pp->cfg1_size = resource_size(pp->cfg)/2;
> + pp->cfg0_size = resource_size(pp->cfg) / 2;
> + pp->cfg1_size = resource_size(pp->cfg) / 2;
> pp->cfg0_base = pp->cfg->start;
> pp->cfg1_base = pp->cfg->start + pp->cfg0_size;
> break;
> @@ -615,7 +615,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
> }
>
> static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> - u32 devfn, int where, int size, u32 *val)
> + u32 devfn, int where, int size, u32 *val)
> {
> int ret, type;
> u32 busdev, cfg_size;
> @@ -654,7 +654,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> }
>
> static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> - u32 devfn, int where, int size, u32 val)
> + u32 devfn, int where, int size, u32 val)
> {
> int ret, type;
> u32 busdev, cfg_size;
> @@ -711,7 +711,7 @@ static int dw_pcie_valid_device(struct pcie_port *pp, struct pci_bus *bus,
> }
>
> static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
> - int size, u32 *val)
> + int size, u32 *val)
> {
> struct pcie_port *pp = bus->sysdata;
>
> @@ -727,7 +727,7 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
> }
>
> static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
> - int where, int size, u32 val)
> + int where, int size, u32 val)
> {
> struct pcie_port *pp = bus->sysdata;
>
>
Always good to make clean up! Thanks!
Acked-By: Joao Pinto <jpinto@synopsys.com>
^ permalink raw reply
* Re: [PATCH v4 2/2] dt-bindings: clk: add rockchip,grf property for RK3399
From: Rob Herring @ 2017-01-13 16:41 UTC (permalink / raw)
To: Xing Zheng
Cc: heiko-4mtYJXux2i+zQB+pC5nmwQ, dianders-hpIqsD4AKlfQT0dZR+AlfA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Michael Turquette, Stephen Boyd, Mark Rutland,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484028930-20305-3-git-send-email-zhengxing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On Tue, Jan 10, 2017 at 02:15:30PM +0800, Xing Zheng wrote:
> Add support for rockchip,grf property which is used for GRF muxes
> on RK3399.
>
> Signed-off-by: Xing Zheng <zhengxing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
>
> Changes in v4:
> - update the decription for rockchip,grf property
>
> Changes in v3: None
> Changes in v2: None
>
> Documentation/devicetree/bindings/clock/rockchip,rk3399-cru.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/4] ASoC: es8328-i2c: Add compatible for ES8388
From: Heiko Stuebner @ 2017-01-13 16:43 UTC (permalink / raw)
To: Romain Perier
Cc: Jaroslav Kysela, Takashi Iwai, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sjoerd Simons
In-Reply-To: <20170113161452.14313-2-romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Hi Romain,
Am Freitag, 13. Januar 2017, 17:14:49 CET schrieb Romain Perier:
> This commit adds a compatible string for everest,es8388. This is
> an audio codec that is compatible with es8328.
>
> Signed-off-by: Romain Perier <romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> ---
> Documentation/devicetree/bindings/sound/es8328.txt | 2 +-
> sound/soc/codecs/es8328-i2c.c | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/es8328.txt
> b/Documentation/devicetree/bindings/sound/es8328.txt index 30ea8a3..33fbf05
> 100644
> --- a/Documentation/devicetree/bindings/sound/es8328.txt
> +++ b/Documentation/devicetree/bindings/sound/es8328.txt
> @@ -4,7 +4,7 @@ This device supports both I2C and SPI.
>
> Required properties:
>
> - - compatible : "everest,es8328"
> + - compatible : Should be "everest,es8328" or "everest,es8388"
> - DVDD-supply : Regulator providing digital core supply voltage 1.8 -
> 3.6V - AVDD-supply : Regulator providing analog supply voltage 3.3V
> - PVDD-supply : Regulator providing digital IO supply voltage 1.8 - 3.6V
> diff --git a/sound/soc/codecs/es8328-i2c.c b/sound/soc/codecs/es8328-i2c.c
> index 2d05b5d..09ae67f 100644
> --- a/sound/soc/codecs/es8328-i2c.c
> +++ b/sound/soc/codecs/es8328-i2c.c
> @@ -19,13 +19,14 @@
> #include "es8328.h"
>
> static const struct i2c_device_id es8328_id[] = {
> - { "es8328", 0 },
> + { "es8328", "es8388", 0 },
that looks strange or I'm just blind.
struct i2c_device_id {
char name[I2C_NAME_SIZE];
kernel_ulong_t driver_data; /* Data private to the driver */
};
so shouldn't this be
static const struct i2c_device_id es8328_id[] = {
{ "es8328", 0 },
+ { "es8388", 0 },
instead?
> { }
> };
> MODULE_DEVICE_TABLE(i2c, es8328_id);
>
> static const struct of_device_id es8328_of_match[] = {
> { .compatible = "everest,es8328", },
> + { .compatible = "everest,es8388", },
> { }
> };
> MODULE_DEVICE_TABLE(of, es8328_of_match);
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v1 2/2] arm: dts: mt2701: add nor flash node
From: Marek Vasut @ 2017-01-13 16:44 UTC (permalink / raw)
To: Boris Brezillon
Cc: Matthias Brugger, Guochun Mao, David Woodhouse, Brian Norris,
Richard Weinberger, Cyrille Pitchen, Rob Herring, Mark Rutland,
Russell King, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170113172825.75d545a3@bbrezillon>
On 01/13/2017 05:28 PM, Boris Brezillon wrote:
> On Fri, 13 Jan 2017 17:13:55 +0100
> Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On 01/13/2017 04:12 PM, Matthias Brugger wrote:
>>>
>>>
>>> On 13/01/17 15:17, Boris Brezillon wrote:
>>>> On Fri, 13 Jan 2017 15:13:29 +0800
>>>> Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>>>>
>>>>> Add Mediatek nor flash node.
>>>>>
>>>>> Signed-off-by: Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>>>> ---
>>>>> arch/arm/boot/dts/mt2701-evb.dts | 25 +++++++++++++++++++++++++
>>>>> arch/arm/boot/dts/mt2701.dtsi | 12 ++++++++++++
>>>>> 2 files changed, 37 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/mt2701-evb.dts
>>>>> b/arch/arm/boot/dts/mt2701-evb.dts
>>>>> index 082ca88..85e5ae8 100644
>>>>> --- a/arch/arm/boot/dts/mt2701-evb.dts
>>>>> +++ b/arch/arm/boot/dts/mt2701-evb.dts
>>>>> @@ -24,6 +24,31 @@
>>>>> };
>>>>> };
>>>>>
>>>>> +&nor_flash {
>>>>> + pinctrl-names = "default";
>>>>> + pinctrl-0 = <&nor_pins_default>;
>>>>> + status = "okay";
>>>>> + flash@0 {
>>>>> + compatible = "jedec,spi-nor";
>>>>> + reg = <0>;
>>>>> + };
>>>>> +};
>>>>> +
>>>>> +&pio {
>>>>> + nor_pins_default: nor {
>>>>> + pins1 {
>>>>> + pinmux = <MT2701_PIN_240_EXT_XCS__FUNC_EXT_XCS>,
>>>>> + <MT2701_PIN_241_EXT_SCK__FUNC_EXT_SCK>,
>>>>> + <MT2701_PIN_239_EXT_SDIO0__FUNC_EXT_SDIO0>,
>>>>> + <MT2701_PIN_238_EXT_SDIO1__FUNC_EXT_SDIO1>,
>>>>> + <MT2701_PIN_237_EXT_SDIO2__FUNC_EXT_SDIO2>,
>>>>> + <MT2701_PIN_236_EXT_SDIO3__FUNC_EXT_SDIO3>;
>>>>> + drive-strength = <MTK_DRIVE_4mA>;
>>>>> + bias-pull-up;
>>>>> + };
>>>>> + };
>>>>> +};
>>>>> +
>>>>> &uart0 {
>>>>> status = "okay";
>>>>> };
>>>>> diff --git a/arch/arm/boot/dts/mt2701.dtsi
>>>>> b/arch/arm/boot/dts/mt2701.dtsi
>>>>> index bdf8954..1eefce4 100644
>>>>> --- a/arch/arm/boot/dts/mt2701.dtsi
>>>>> +++ b/arch/arm/boot/dts/mt2701.dtsi
>>>>> @@ -227,6 +227,18 @@
>>>>> status = "disabled";
>>>>> };
>>>>>
>>>>> + nor_flash: spi@11014000 {
>>>>> + compatible = "mediatek,mt2701-nor",
>>>>> + "mediatek,mt8173-nor";
>>>>
>>>> Why define both here? Is "mediatek,mt8173-nor" really providing a
>>>> subset of the features supported by "mediatek,mt2701-nor"?
>>>>
>>>
>>> I think even if the ip block is the same, we should provide both
>>> bindings, just in case in the future we find out that mt2701 has some
>>> hidden bug, feature or bug-feature. This way even if we update the
>>> driver, we stay compatible with older device tree blobs in the wild.
>>>
>>> We can drop the mt2701-nor in the bindings definition if you want.
>
> Oh, sorry, I misunderstood. What I meant is that if you want to
> list/support all possible compatibles, maybe you should just put one
> compatible in your DT and patch your driver (+ binding doc) to define
> all of them.
Uh, what ? I lost you here :-)
>> This exactly. We should have a DT compat in the form:
>> compatible = "vendor,<soc>-block", "vendor,<oldest-compat-soc>-block";
>> Then if we find a problem in the future, we can match on the
>> "vendor,<soc>-block" and still support the old DTs.
>
> Not sure it's only in term of whose IP appeared first. My understanding
> is that it's a way to provide inheritance. For example:
>
> "<soc-vendor>,<ip-version>", "<ip-vendor>,<ip-version>";
>
> or
>
> "<soc-vendor>,<full-featured-ip-version>","<soc-vendor>,<basic-feature-ip-version>";
>
> BTW, which one is the oldest between mt8173 and mt2701? :-)
And that's another thing and I agree with you, but I don't think that's
what we're discussing in this thread. But (!), OT, I think we should
codify the rules in Documentation/ . This discussion came up multiple
times recently.
And my question still stands, what do we put into the DT here, IMO
compatible = "mediatek,mt2701-nor", "mediatek,mt8173-nor";
and what goes into the binding document ? I guess both too ?
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 11/37] PCI: dwc: Split pcie-designware.c into host and core files
From: Joao Pinto @ 2017-01-13 16:49 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Bjorn Helgaas, Jingoo Han, Joao Pinto,
Arnd Bergmann
Cc: devicetree, linux-samsung-soc, linux-doc, linux-pci, nsekhar,
linux-kernel, linux-arm-kernel, linux-arm-msm, linux-omap,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1484216786-17292-12-git-send-email-kishon@ti.com>
Às 10:26 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> Split pcie-designware.c into pcie-designware-host.c that contains
> the host specific parts of the driver and pcie-designware.c that
> contains the parts used by both host driver and endpoint driver.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/pci/dwc/Makefile | 2 +-
> drivers/pci/dwc/pcie-designware-host.c | 619 ++++++++++++++++++++++++++++++++
> drivers/pci/dwc/pcie-designware.c | 613 +------------------------------
> drivers/pci/dwc/pcie-designware.h | 8 +
> 4 files changed, 634 insertions(+), 608 deletions(-)
> create mode 100644 drivers/pci/dwc/pcie-designware-host.c
>
> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
> index 7d27c14..3b57e55 100644
> --- a/drivers/pci/dwc/Makefile
> +++ b/drivers/pci/dwc/Makefile
> @@ -1,4 +1,4 @@
(snip...)
> -static void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> - int type, u64 cpu_addr, u64 pci_addr,
> - u32 size)
> +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> + u64 cpu_addr, u64 pci_addr, u32 size)
> {
> u32 retries, val;
>
> @@ -186,220 +151,6 @@ static void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> dev_err(pci->dev, "iATU is not being enabled\n");
> }
Kishon, iATU only makes sense in The Root Complex (host), so it should be inside
the pcie-designware-host.
>
> -static struct irq_chip dw_msi_irq_chip = {
> - .name = "PCI-MSI",
> - .irq_enable = pci_msi_unmask_irq,
> - .irq_disable = pci_msi_mask_irq,
> - .irq_mask = pci_msi_mask_irq,
> - .irq_unmask = pci_msi_unmask_irq,
> -};
> -
(snip...)
> -
> -static const struct irq_domain_ops msi_domain_ops = {
> - .map = dw_pcie_msi_map,
> -};
> -
> static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
> {
> u32 val;
> @@ -454,303 +192,11 @@ static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
> return 0;
> }
Kishon, iATU only makes sense in The Root Complex (host), so it should be inside
the pcie-designware-host.
(snip...)
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index 491fbe3..808d17b 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -14,6 +14,10 @@
> #ifndef _PCIE_DESIGNWARE_H
> #define _PCIE_DESIGNWARE_H
>
> +#include <linux/irq.h>
> +#include <linux/msi.h>
> +#include <linux/pci.h>
> +
> /* Parameters for the waiting for link up routine */
> #define LINK_WAIT_MAX_RETRIES 10
> #define LINK_WAIT_USLEEP_MIN 90000
> @@ -167,4 +171,8 @@ struct dw_pcie {
> void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val);
> int dw_pcie_link_up(struct dw_pcie *pci);
> int dw_pcie_wait_for_link(struct dw_pcie *pci);
> +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> + int type, u64 cpu_addr, u64 pci_addr,
> + u32 size);
> +void dw_pcie_setup(struct dw_pcie *pci);
Kishon, iATU only makes sense in The Root Complex (host), so it should be inside
the pcie-designware-host as static.
> #endif /* _PCIE_DESIGNWARE_H */
>
Thanks,
Joao
^ permalink raw reply
* Re: [PATCH v1 2/2] arm: dts: mt2701: add nor flash node
From: Boris Brezillon @ 2017-01-13 16:56 UTC (permalink / raw)
To: Marek Vasut
Cc: Matthias Brugger, Guochun Mao, David Woodhouse, Brian Norris,
Richard Weinberger, Cyrille Pitchen, Rob Herring, Mark Rutland,
Russell King, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <86c997be-f500-eaa1-3ba5-d21cff6223b7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, 13 Jan 2017 17:44:12 +0100
Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 01/13/2017 05:28 PM, Boris Brezillon wrote:
> > On Fri, 13 Jan 2017 17:13:55 +0100
> > Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> >> On 01/13/2017 04:12 PM, Matthias Brugger wrote:
> >>>
> >>>
> >>> On 13/01/17 15:17, Boris Brezillon wrote:
> >>>> On Fri, 13 Jan 2017 15:13:29 +0800
> >>>> Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >>>>
> >>>>> Add Mediatek nor flash node.
> >>>>>
> >>>>> Signed-off-by: Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>>>> ---
> >>>>> arch/arm/boot/dts/mt2701-evb.dts | 25 +++++++++++++++++++++++++
> >>>>> arch/arm/boot/dts/mt2701.dtsi | 12 ++++++++++++
> >>>>> 2 files changed, 37 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm/boot/dts/mt2701-evb.dts
> >>>>> b/arch/arm/boot/dts/mt2701-evb.dts
> >>>>> index 082ca88..85e5ae8 100644
> >>>>> --- a/arch/arm/boot/dts/mt2701-evb.dts
> >>>>> +++ b/arch/arm/boot/dts/mt2701-evb.dts
> >>>>> @@ -24,6 +24,31 @@
> >>>>> };
> >>>>> };
> >>>>>
> >>>>> +&nor_flash {
> >>>>> + pinctrl-names = "default";
> >>>>> + pinctrl-0 = <&nor_pins_default>;
> >>>>> + status = "okay";
> >>>>> + flash@0 {
> >>>>> + compatible = "jedec,spi-nor";
> >>>>> + reg = <0>;
> >>>>> + };
> >>>>> +};
> >>>>> +
> >>>>> +&pio {
> >>>>> + nor_pins_default: nor {
> >>>>> + pins1 {
> >>>>> + pinmux = <MT2701_PIN_240_EXT_XCS__FUNC_EXT_XCS>,
> >>>>> + <MT2701_PIN_241_EXT_SCK__FUNC_EXT_SCK>,
> >>>>> + <MT2701_PIN_239_EXT_SDIO0__FUNC_EXT_SDIO0>,
> >>>>> + <MT2701_PIN_238_EXT_SDIO1__FUNC_EXT_SDIO1>,
> >>>>> + <MT2701_PIN_237_EXT_SDIO2__FUNC_EXT_SDIO2>,
> >>>>> + <MT2701_PIN_236_EXT_SDIO3__FUNC_EXT_SDIO3>;
> >>>>> + drive-strength = <MTK_DRIVE_4mA>;
> >>>>> + bias-pull-up;
> >>>>> + };
> >>>>> + };
> >>>>> +};
> >>>>> +
> >>>>> &uart0 {
> >>>>> status = "okay";
> >>>>> };
> >>>>> diff --git a/arch/arm/boot/dts/mt2701.dtsi
> >>>>> b/arch/arm/boot/dts/mt2701.dtsi
> >>>>> index bdf8954..1eefce4 100644
> >>>>> --- a/arch/arm/boot/dts/mt2701.dtsi
> >>>>> +++ b/arch/arm/boot/dts/mt2701.dtsi
> >>>>> @@ -227,6 +227,18 @@
> >>>>> status = "disabled";
> >>>>> };
> >>>>>
> >>>>> + nor_flash: spi@11014000 {
> >>>>> + compatible = "mediatek,mt2701-nor",
> >>>>> + "mediatek,mt8173-nor";
> >>>>
> >>>> Why define both here? Is "mediatek,mt8173-nor" really providing a
> >>>> subset of the features supported by "mediatek,mt2701-nor"?
> >>>>
> >>>
> >>> I think even if the ip block is the same, we should provide both
> >>> bindings, just in case in the future we find out that mt2701 has some
> >>> hidden bug, feature or bug-feature. This way even if we update the
> >>> driver, we stay compatible with older device tree blobs in the wild.
> >>>
> >>> We can drop the mt2701-nor in the bindings definition if you want.
> >
> > Oh, sorry, I misunderstood. What I meant is that if you want to
> > list/support all possible compatibles, maybe you should just put one
> > compatible in your DT and patch your driver (+ binding doc) to define
> > all of them.
>
> Uh, what ? I lost you here :-)
>
> >> This exactly. We should have a DT compat in the form:
> >> compatible = "vendor,<soc>-block", "vendor,<oldest-compat-soc>-block";
> >> Then if we find a problem in the future, we can match on the
> >> "vendor,<soc>-block" and still support the old DTs.
> >
> > Not sure it's only in term of whose IP appeared first. My understanding
> > is that it's a way to provide inheritance. For example:
> >
> > "<soc-vendor>,<ip-version>", "<ip-vendor>,<ip-version>";
> >
> > or
> >
> > "<soc-vendor>,<full-featured-ip-version>","<soc-vendor>,<basic-feature-ip-version>";
> >
> > BTW, which one is the oldest between mt8173 and mt2701? :-)
>
> And that's another thing and I agree with you, but I don't think that's
> what we're discussing in this thread. But (!), OT, I think we should
> codify the rules in Documentation/ . This discussion came up multiple
> times recently.
>
> And my question still stands, what do we put into the DT here, IMO
> compatible = "mediatek,mt2701-nor", "mediatek,mt8173-nor";
I'd say
compatible = "mediatek,mt8173-nor";
because both compatible are referring to very specific IP version. It's
not the same as
compatible = "mediatek,mt8173-nor", "mediatek,mt81xx-nor";
where you clearly have a generic compatible which is overloaded by a
specific one.
But anyway, I'm not the one taking the decision here, let's wait for DT
maintainers reviews.
> and what goes into the binding document ? I guess both too ?
If both exist, they should be both documented.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 5/5] Documentation:powerpc: Add device-tree bindings for power-mgt
From: Rob Herring @ 2017-01-13 16:57 UTC (permalink / raw)
To: Gautham R. Shenoy
Cc: Stewart Smith, devicetree, Michael Neuling, linux-pm,
Shreyas B. Prabhu, Daniel Lezcano, Rafael J. Wysocki,
linux-kernel, Paul Mackerras, Shilpasri G Bhat,
Oliver O'Halloran, mark.rutland, linuxppc-dev
In-Reply-To: <1484039224-5630-6-git-send-email-ego@linux.vnet.ibm.com>
On Tue, Jan 10, 2017 at 02:37:04PM +0530, Gautham R. Shenoy wrote:
> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>
> Document the device-tree bindings defining the the properties under
> the @power-mgt node in the device tree that describe the idle states
> for Linux running on baremetal POWER servers.
>
We have "common" idle state bindings. Perhaps some explanation why those
can't be used.
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
> [v4]-> [v5]: Fixed a couple of typos.
>
> .../devicetree/bindings/powerpc/opal/power-mgt.txt | 125 +++++++++++++++++++++
> 1 file changed, 125 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
>
> diff --git a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> new file mode 100644
> index 0000000..4967831
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> @@ -0,0 +1,125 @@
> +IBM Power-Management Bindings
> +=============================
> +
> +Linux running on baremetal POWER machines has access to the processor
> +idle states. The description of these idle states is exposed via the
> +node @power-mgt in the device-tree by the firmware.
> +
> +Definitions:
> +----------------
> +Typically each idle state has the following associated properties:
> +
> +- name: The name of the idle state as defined by the firmware.
> +
> +- flags: indicating some aspects of this idle states such as the
> + extent of state-loss, whether timebase is stopped on this
> + idle states and so on. The flag bits are as follows:
> +
> +- exit-latency: The latency involved in transitioning the state of the
> + CPU from idle to running.
> +
> +- target-residency: The minimum time that the CPU needs to reside in
> + this idle state in order to accrue power-savings
> + benefit.
> +
> +Properties
> +----------------
> +The following properties provide details about the idle states. These
> +properties are optional unless mentioned otherwise below.
-names is optional but everything else seems to require it. It is not
clear what the binding looks like if -names is not present.
> +
> +- ibm,cpu-idle-state-names:
> + Array of strings containing the names of the idle states.
> +
> +- ibm,cpu-idle-state-flags:
> + Array of unsigned 32-bit values containing the values of the
> + flags associated with the the aforementioned idle-states. This
> + property is required on POWER9 whenever
> + ibm,cpu-idle-state-names is defined and the length of this
> + property array should be the same as
> + ibm,-cpu-idle-state-names.The flag bits are as follows:
s/ibm,-cpu/ibm,cpu/
Needs a space after the period.
> + 0x00000001 /* Decrementer would stop */
> + 0x00000002 /* Needs timebase restore */
> + 0x00001000 /* Restore GPRs like nap */
> + 0x00002000 /* Restore hypervisor resource from PACA pointer */
> + 0x00004000 /* Program PORE to restore PACA pointer */
> + 0x00010000 /* This is a nap state */
> + 0x00020000 /* This is a fast-sleep state */
> + 0x00040000 /* This is a winkle state */
> + 0x00080000 /* This is a fast-sleep state which requires a */
> + /* software workaround for restoring the timebase*/
> + 0x00800000 /* This state uses SPR PMICR instruction */
> + 0x00100000 /* This is a fast stop state */
> + 0x00200000 /* This is a deep-stop state */
> +
> +- ibm,cpu-idle-state-latencies-ns:
> + Array of unsigned 32-bit values containing the values of the
> + exit-latencies (in ns) for the idle states in
> + ibm,cpu-idle-state-names. This property is required whenever
> + ibm,cpu-idle-state-names is defined and the length of this
> + property array should be the same as
> + ibm,-cpu-idle-state-names.
Same typo.
> +
> +- ibm,cpu-idle-state-residency-ns:
> + Array of unsigned 32-bit values containing the values of the
> + target-residency (in ns) for the idle states in
> + ibm,cpu-idle-state-names. On POWER8 this is an optional
> + property. If the property is absent, the target residency for
> + the "Nap", "FastSleep" are defined to 10000 and 300000000
> + respectively. On POWER9 this property must be defined if
> + ibm,cpu-idle-state-names is defined and the length should be
> + same as that of ibm,cpu-idle-state-names.
> +
> +- ibm,cpu-idle-state-psscr:
> + Array of unsigned 64-bit values containing the values for the
> + PSSCR for each of the idle states in ibm,cpu-idle-state-names.
> + This property is required on POWER9 whenever
> + ibm,cpu-idle-state-names is defined and the length of this
> + property array should be the same as
> + ibm,cpu-idle-state-names.
> +
> +- ibm,cpu-idle-state-psscr-mask:
> + Array of unsigned 64-bit values containing the masks
> + indicating which psscr fields are set in the corresponding
> + entries of ibm,cpu-idle-state-psscr. This property is
> + required on POWER9 whenever ibm,cpu-idle-state-names is
> + defined and the length of this property array should be the
> + same as ibm,cpu-idle-state-names.
> +
> + Whenever the firmware sets an entry in
> + ibm,cpu-idle-state-psscr-mask value to 0xf, it implies that
> + only the Requested Level (RL) field of the corresponding entry
> + in ibm,cpu-idle-state-psscr should be considered by the
> + kernel. For such idle states, the kernel would set the
> + remaining fields of the psscr to the following sane-default
> + values.
> +
> + - ESL and EC bits are to 1. So wakeup from any stop
> + state will be at vector 0x100.
> +
> + - MTL and PSLL are set to the maximum allowed value as
> + per the ISA, i.e. 15.
> +
> + - The Transition Rate, TR is set to the Maximum value
> + 3.
> +
> + For all the other values of the entry in
> + ibm,cpu-idle-state-psscr-mask, the Kernel expects all the
> + psscr fields of the corresponding entry in
> + ibm,cpu-idle-state-psscr to be correctly set by the firmware.
> +
> +- ibm,cpu-idle-state-pmicr:
> + Array of unsigned 64-bit values containing the pmicr values
> + for the idle states in ibm,cpu-idle-state-names. This 64-bit
> + register value is to be set in pmicr for the corresponding
> + state if the flag indicates that pmicr SPR should be set. This
> + is an optional property on POWER8 and is absent on
> + POWER9. When present on POWER8, the length of this property
> + array should be the same as ibm,cpu-idle-state-names.
> +
> +- ibm,cpu-idle-state-pmicr:-mask
> + Array of unsigned 64-bit values containing the mask indicating
> + which of the fields of the PMICR are set in the corresponding
> + entries in ibm,cpu-idle-state-pmicr. This is an optional
> + property on POWER8 and is absent on POWER9. When present on
> + POWER8, the length of this property array should be the same
> + as ibm,cpu-idle-state-names.
> --
> 1.9.4
>
^ permalink raw reply
* Re: [PATCH] regulator: anatop: Add support for "anatop-enable-bit"
From: Rob Herring @ 2017-01-13 17:07 UTC (permalink / raw)
To: Andrey Smirnov
Cc: linux-kernel, yurovsky, Liam Girdwood, Mark Brown, Mark Rutland,
Shawn Guo, devicetree
In-Reply-To: <20170110163015.22444-2-andrew.smirnov@gmail.com>
On Tue, Jan 10, 2017 at 08:30:14AM -0800, Andrey Smirnov wrote:
> Add code to support support for "anatop-enable-bit" device-tree
> property. This property translates to LINREG_ENABLE bit in real hardware
> and is present on 1p1, 2p5 and 3p0 regulators on i.MX6 and 1p0d regulator
> on i.MX7.
>
> Cc: yurovsky@gmail.com
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>
> Note: "anatop-enable-bit" has already found its way into upstream tree
> before this patch (probably not on purpose). See imx6ul.dtsi and
> imx7s.dtsi for concrete examples
But since the kernel is not using it, it doesn't really mean we have to
keep it. Yes, other OSs could be, but that's unlikely.
> .../devicetree/bindings/regulator/anatop-regulator.txt | 1 +
> drivers/regulator/anatop-regulator.c | 12 ++++++++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
> index 37c4ea0..1d58c8c 100644
> --- a/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/anatop-regulator.txt
> @@ -14,6 +14,7 @@ Optional properties:
> - anatop-delay-bit-shift: Bit shift for the step time register
> - anatop-delay-bit-width: Number of bits used in the step time register
> - vin-supply: The supply for this regulator
> +- anatop-enable-bit: Regulator enable bit offset
Looks like we have several properties describing the register layout.
IMO, we should drop all of them and use the compatible strings to
determine those instead.
Rob
^ permalink raw reply
* Re: [PATCH 09/37] PCI: dwc: designware: Parse *num-lanes* property in dw_pcie_setup_rc
From: Joao Pinto @ 2017-01-13 17:13 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Bjorn Helgaas, Jingoo Han, Joao Pinto,
Arnd Bergmann
Cc: devicetree, linux-samsung-soc, linux-doc, linux-pci, nsekhar,
linux-kernel, linux-arm-kernel, linux-arm-msm, linux-omap,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1484216786-17292-10-git-send-email-kishon@ti.com>
Hi,
Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> *num-lanes* dt property is parsed in dw_pcie_host_init. However
> *num-lanes* property is applicable to both root complex mode and
> endpoint mode. As a first step, move the parsing of this property
> outside dw_pcie_host_init. This is in preparation for splitting
> pcie-designware.c to pcie-designware.c and pcie-designware-host.c
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/pci/dwc/pcie-designware.c | 18 +++++++++++-------
> drivers/pci/dwc/pcie-designware.h | 1 -
> 2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c
> index 00a0fdc..89cdb6b 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -551,10 +551,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
> }
> }
>
> - ret = of_property_read_u32(np, "num-lanes", &pci->lanes);
> - if (ret)
> - pci->lanes = 0;
> -
> ret = of_property_read_u32(np, "num-viewport", &pci->num_viewport);
> if (ret)
> pci->num_viewport = 2;
> @@ -751,18 +747,26 @@ static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
>
> void dw_pcie_setup_rc(struct pcie_port *pp)
> {
> + int ret;
> + u32 lanes;
> u32 val;
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct device *dev = pci->dev;
> + struct device_node *np = dev->of_node;
>
> /* get iATU unroll support */
> pci->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pci);
> dev_dbg(pci->dev, "iATU unroll: %s\n",
> pci->iatu_unroll_enabled ? "enabled" : "disabled");
>
> + ret = of_property_read_u32(np, "num-lanes", &lanes);
> + if (ret)
> + lanes = 0;
You moved from host_init to root complex setup function, which in my opinion did
not improve (in this scope).
I suggest that instead of making so much intermediary patches, which is nice to
understand your development sequence, but hard to review. Wouldn't be better to
condense some of the patches? We would have a cloear vision of the final product :)
Joao
> +
> /* set the number of lanes */
> val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
> val &= ~PORT_LINK_MODE_MASK;
> - switch (pci->lanes) {
> + switch (lanes) {
> case 1:
> val |= PORT_LINK_MODE_1_LANES;
> break;
> @@ -776,7 +780,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
> val |= PORT_LINK_MODE_8_LANES;
> break;
> default:
> - dev_err(pci->dev, "num-lanes %u: invalid value\n", pci->lanes);
> + dev_err(pci->dev, "num-lanes %u: invalid value\n", lanes);
> return;
> }
> dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
> @@ -784,7 +788,7 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
> /* set link width speed control register */
> val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> val &= ~PORT_LOGIC_LINK_WIDTH_MASK;
> - switch (pci->lanes) {
> + switch (lanes) {
> case 1:
> val |= PORT_LOGIC_LINK_WIDTH_1_LANES;
> break;
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index d4b3d43..491fbe3 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -148,7 +148,6 @@ struct dw_pcie_ops {
> struct dw_pcie {
> struct device *dev;
> void __iomem *dbi_base;
> - u32 lanes;
> u32 num_viewport;
> u8 iatu_unroll_enabled;
> struct pcie_port pp;
>
^ permalink raw reply
* Re: [PATCH] Documentation: dt-bindings: Add binding documentation for TI clkctrl clocks
From: Tony Lindgren @ 2017-01-13 17:13 UTC (permalink / raw)
To: Rob Herring
Cc: Michael Turquette, Stephen Boyd, Tero Kristo,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Paul Walmsley
In-Reply-To: <20170113163647.6xbhoafb5xddtvnz@rob-hp-laptop>
* Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [170113 08:37]:
> On Tue, Jan 10, 2017 at 07:44:01AM -0800, Tony Lindgren wrote:
> > * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [170109 15:43]:
> > > Texas Instruments omap variant SoCs starting with omap4 have a clkctrl
> > > clock controller instance for each interconnect target module. The clkctrl
> > > controls functional and interface clocks for the module.
> > >
> > > The clkctrl clocks are currently handled by arch/arm/mach-omap2 hwmod code.
> > > With this binding and a related clock device driver we can start moving the
> > > clkctrl clock handling to live in drivers/clk/ti.
> > >
> > > For hardware reference, see omap4430 TRM "Table 3-1312. L4PER_CM2 Registers
> > > Mapping Summary" for example. It show one instance of a clkctrl clock
> > > controller with multiple clkctrl registers.
> > >
> > > Note that this binding allows keeping the clockdomain related parts out of
> > > drivers/clock. The CLKCTCTRL and DYNAMICDEP registers can be handled by
> > > using a separate driver in drivers/soc/ti and genpd. If the clockdomain
> > > driver needs to know it's clocks, we can just set the the clkctrl device
> > > instances to be children of the related clockdomain device.
> > >
> > > On omap4 CM_L3INIT_USB_HOST_HS_CLKCTRL on omap5 has eight OPTFCLKEN bits.
> > > So we need to shift the clock index to avoid index conflict for the clock
> > > consumer binding with the next clkctrl offset on omap4.
> > >
> > > Cc: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
> > > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> > > ---
> > >
> > > So here's what I was able to come up for the clkctr binding based on
> > > all we've discussed so far. Can you guys please take a look and see
> > > if it looks OK to you before we do the device driver?
> > >
> > > Also, does anybody have better suggestions for addressing the optional
> > > clocks in each clkctrl register?
> >
> > The other option that might be worth considering is to make use of the
> > #clock-cells property. Then the index of any optional clock could be passed
> > in the second cell.
> >
> > The third cell could be used to set the modulemode for the clock (software
> > controlled or hardware controlled) instead of using a custom property
> > at the clock controllel level.
>
> I guess I prefer this way. Or you could do a mixture of both proposals
> with 2 cells. The first being the clock id and the 2nd flags.
OK. I don't think we can do it with two cells with using real hardware
offsets for the clocks though. So in that case I'd prefer the three
cell binding as below.
> What's the max optional clocks in theory? B picked from the current
> worst case seems a bit worrying. Why not 16? Upper half is offset, lower
> half is index.
It seems the max is "stuff it into whatever bits are available" in the
register :) And I just noticed omap5 CM_L3INIT_USB_HOST_HS_CLKCTRL has 10
optional clocks, not 8. So yeah let's assume it could be even more.
> > In that case clock consume usage would look like the following using
> > #clock-cells = <3>:
> >
> > #define OMAP4_CLKCTRL_OFFSET 0x20
> > #define MODULEMODE_HWCTRL 1
> > #define MODULEMODE_SWCTRL 2
>
> Can you make one of these 0 instead or is both being set valid?
For MODULEMODE clock 0 means disabed. HWCTRL and SWCTRL are flags for
enabled mode.
So clock index 0 would be the MODULEMODE clock, index 1 first optional clock
and so on. The index could be also be the offset in the actual register
if you prefer that.
> > #define OMAP_CLKCTRL_INDEX(offset) ((offset) - OMAP4_CLKCTRL_OFFSET)
> >
> > #define OMAP4_GPTIMER10_CLKTRL OMAP_CLKCTRL_INDEX(0x28)
> > #define OMAP4_GPTIMER11_CLKTRL OMAP_CLKCTRL_INDEX(0x30)
> > #define OMAP4_GPTIMER2_CLKTRL OMAP_CLKCTRL_INDEX(0x38)
> > ...
> > #define OMAP4_GPIO2_CLKCTRL OMAP_CLKCTRL_INDEX(0x60)
> > ...
> >
> > &gpio2 {
> > clocks = <&cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL 0 MODULEMODE_HWCTRL
> > &cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL_DBCLK 1 0>;
>
> Drop the _DBCLK here, right?
Ah sorry yeah this should be:
&gpio2 {
clocks = <&cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL 0 MODULEMODE_HWCTRL
&cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL 1 0>;
};
or if using actual bit offsets within the register instead of optional
clock instance count:
&gpio2 {
clocks = <&cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL 0 MODULEMODE_HWCTRL
&cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL 8 0>;
};
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
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