* Re: [PATCH] ARM: tegra: add device tree for SHIELD
From: Arend van Spriel @ 2014-02-26 21:10 UTC (permalink / raw)
To: Alexandre Courbot, Stephen Warren, Thierry Reding, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King
Cc: devicetree@vger.kernel.org, linux-tegra@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <530D731B.5050305@nvidia.com>
On 02/26/2014 05:52 AM, Alexandre Courbot wrote:
> On 02/25/2014 06:52 PM, Arend van Spriel wrote:
>> On 02/25/2014 03:13 AM, Alexandre Courbot wrote:
>>>>
>>>>> + /* Wifi */
>>>>> + sdhci@78000000 {
>>>>> + status = "okay";
>>>>> + bus-width = <4>;
>>>>> + broken-cd;
>>>>> + keep-power-in-suspend;
>>>>> + cap-sdio-irq;
>>>>
>>>> Is non-removable better than broken-cd, or are they entirely unrelated?
>>>
>>> They are unrelated actually. With non-removable the driver expects the
>>> device to always be there since boot, and does not check for the card to
>>> be removed/added after boot. broken-cd indicates there is no CD line and
>>> the device should be polled regularly.
>>>
>>> For the Wifi chip, non-removable would be the correct setting
>>> hardware-wise, but there is a trap: the chip has its reset line asserted
>>> at boot-time, and you need to set GPIO 229 to de-assert it. Only after
>>> that will the device be detected on the SDIO bus. Since it lacks a CD
>>> line, it must be polled, hence the broken-cd property.
>>>
>>> This also raises another, redundant problem with DT bindings: AFAIK we
>>> currently have no way to let the system know the device will only appear
>>> after a given GPIO is set. It would also be nice to be able to give some
>>> parameters to the Wifi driver through the DT (like the OOB interrupt).
>>> Right now the Wifi chip is brought up by exporting the GPIO and writing
>>> to it from user-space, and the OOB interrupt is not used.
>>
>> Hi Alexandre,
>>
>> I recently posted a proposal for brcmfmac DT binding [1]. I did receive
>> some comments, but it would be great if you (and/or others involved) had
>> a look at it as well and give me some feedback. DT work still needs to
>> grow on me.
>
> Hi Arend, (and thanks again for all the help with getting the chip to
> work!)
>
> Great, I'm not subscribed to the devicetree list and so have missed this
> thread, but I'm glad to see it.
>
> I don't think I have much to add to the comments you already received
> there. I'd need it to reference the 32K clock (which I currently
> force-enable manually), the OOB interrupt, and the reset pin as a GPIO
> (as for SHIELD the device needs to be put out of reset using an
> active-low GPIO before anything can happen). That last property could be
> optional as I suspect most designs won't use it.
>
> Getting the device out of reset should be done before the bus probes the
> non-removable device, so I wonder how this would fit wrt. the DT
> power-on sequencing series by Olof. Something tells me this could rather
> be a property of the bus, but physically speaking the pin is connected
> to the wifi chip, so... Maybe we could get the platform driver to ask
> the bus to probe again after enabling power/getting the device out of
> reset?
Actually, brcmfmac provides a platform driver and a sdio driver. At the
end of the platform probe it registers the sdio driver, which will
trigger the bus to probe again. I am not sure how that would relate to
the DT power-on sequencing you mentioned.
Regards,
Arend
^ permalink raw reply
* Re: [PATCHv1 1/2] rx51_battery: convert to iio consumer
From: Belisko Marek @ 2014-02-26 21:43 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Jonathan Cameron, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Grant Likely, LKML,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Pali Rohár
In-Reply-To: <1393375569-21751-2-git-send-email-sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
Hi Sebastian,
On Wed, Feb 26, 2014 at 1:46 AM, Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> wrote:
> Update rx51-battery driver to use the new IIO API of
> twl4030-madc and add DT support.
>
> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
> ---
> drivers/power/rx51_battery.c | 68 +++++++++++++++++++++++++++++---------------
> 1 file changed, 45 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/power/rx51_battery.c b/drivers/power/rx51_battery.c
> index 1bc5857..f7cb58e 100644
> --- a/drivers/power/rx51_battery.c
> +++ b/drivers/power/rx51_battery.c
> @@ -24,34 +24,27 @@
> #include <linux/power_supply.h>
> #include <linux/slab.h>
> #include <linux/i2c/twl4030-madc.h>
> -
> -/* RX51 specific channels */
> -#define TWL4030_MADC_BTEMP_RX51 TWL4030_MADC_ADCIN0
> -#define TWL4030_MADC_BCI_RX51 TWL4030_MADC_ADCIN4
> +#include <linux/iio/consumer.h>
> +#include <linux/of.h>
>
> struct rx51_device_info {
> struct device *dev;
> struct power_supply bat;
> + struct iio_channel *channel_temp;
> + struct iio_channel *channel_bsi;
> + struct iio_channel *channel_vbat;
> };
>
> /*
> * Read ADCIN channel value, code copied from maemo kernel
> */
> -static int rx51_battery_read_adc(int channel)
> +static int rx51_battery_read_adc(struct iio_channel *channel)
> {
> - struct twl4030_madc_request req;
> -
> - req.channels = channel;
> - req.do_avg = 1;
> - req.method = TWL4030_MADC_SW1;
> - req.func_cb = NULL;
> - req.type = TWL4030_MADC_WAIT;
> - req.raw = true;
> -
> - if (twl4030_madc_conversion(&req) <= 0)
> - return -ENODATA;
> -
> - return req.rbuf[ffs(channel) - 1];
> + int val, err;
> + err = iio_read_channel_average_raw(channel, &val);
Where this function comes from? I cannot find it in current linux-next
(only iio_read_channel_raw()).
Am I missing some patches? Thx. BTW when I convert to iio consumer and
use DT some of values work
but some of them just report 0 :( (I don't have latest twl4030-madc
iio patches).
> + if (err < 0)
> + return err;
> + return val;
> }
>
> /*
> @@ -60,10 +53,12 @@ static int rx51_battery_read_adc(int channel)
> */
> static int rx51_battery_read_voltage(struct rx51_device_info *di)
> {
> - int voltage = rx51_battery_read_adc(TWL4030_MADC_VBAT);
> + int voltage = rx51_battery_read_adc(di->channel_vbat);
>
> - if (voltage < 0)
> + if (voltage < 0) {
> + dev_err(di->dev, "Could not read ADC: %d\n", voltage);
> return voltage;
> + }
>
> return 1000 * (10000 * voltage / 1705);
> }
> @@ -112,7 +107,10 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
> {
> int min = 0;
> int max = ARRAY_SIZE(rx51_temp_table2) - 1;
> - int raw = rx51_battery_read_adc(TWL4030_MADC_BTEMP_RX51);
> + int raw = rx51_battery_read_adc(di->channel_temp);
> +
> + if (raw < 0)
> + dev_err(di->dev, "Could not read ADC: %d\n", raw);
>
> /* Zero and negative values are undefined */
> if (raw <= 0)
> @@ -146,10 +144,12 @@ static int rx51_battery_read_temperature(struct rx51_device_info *di)
> */
> static int rx51_battery_read_capacity(struct rx51_device_info *di)
> {
> - int capacity = rx51_battery_read_adc(TWL4030_MADC_BCI_RX51);
> + int capacity = rx51_battery_read_adc(di->channel_bsi);
>
> - if (capacity < 0)
> + if (capacity < 0) {
> + dev_err(di->dev, "Could not read ADC: %d\n", capacity);
> return capacity;
> + }
>
> return 1280 * (1200 * capacity)/(1024 - capacity);
> }
> @@ -213,12 +213,25 @@ static int rx51_battery_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, di);
>
> + di->dev = &pdev->dev;
> di->bat.name = dev_name(&pdev->dev);
> di->bat.type = POWER_SUPPLY_TYPE_BATTERY;
> di->bat.properties = rx51_battery_props;
> di->bat.num_properties = ARRAY_SIZE(rx51_battery_props);
> di->bat.get_property = rx51_battery_get_property;
>
> + di->channel_temp = iio_channel_get(di->dev, "temp");
> + if (IS_ERR(di->channel_temp))
> + return PTR_ERR(di->channel_temp);
> +
> + di->channel_bsi = iio_channel_get(di->dev, "bsi");
> + if (IS_ERR(di->channel_bsi))
> + return PTR_ERR(di->channel_bsi);
> +
> + di->channel_vbat = iio_channel_get(di->dev, "vbat");
> + if (IS_ERR(di->channel_vbat))
> + return PTR_ERR(di->channel_vbat);
> +
> ret = power_supply_register(di->dev, &di->bat);
> if (ret)
> return ret;
> @@ -235,12 +248,21 @@ static int rx51_battery_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id n900_battery_of_match[] = {
> + {.compatible = "nokia,n900-battery", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, n900_battery_of_match);
> +#endif
> +
> static struct platform_driver rx51_battery_driver = {
> .probe = rx51_battery_probe,
> .remove = rx51_battery_remove,
> .driver = {
> .name = "rx51-battery",
> .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(n900_battery_of_match),
> },
> };
> module_platform_driver(rx51_battery_driver);
> --
> 1.8.5.3
>
BR,
marek
--
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer
Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com
--
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/3] NFC: trf7970a: Add driver with ISO/IEC 14443 Type 2 Tag Support
From: Mark A. Greer @ 2014-02-26 21:53 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Lauro Ramos Venancio, Aloisio Almeida Jr, Felipe Balbi,
Erick Macias, Thierry Escande,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-nfc-hn68Rpc1hR1g9hUCZPvPmw,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140221005059.GS18868@zurbaran>
On Fri, Feb 21, 2014 at 01:50:59AM +0100, Samuel Ortiz wrote:
> Hi Mark,
Hi Samuel - thanks for the feedback.
> Code looks quite nice, especially since this looks like a fairly complex
> driver. And the initial coments are quite useful, I appreciate that.
> I have a few comments/questions though:
>
>
> > + * been received and there isn't an error). The delay is 3 ms since delays
> > + * over 2 ms have been observed during testing.
> Would you say this timeout depend on the SPI bus bandwidth ?
It'll be one of the factors but the tag's response characteristics are
a big factor too. I can bump it up a bit more if you prefer or do you
want me to somehow calculate it from the SPI bitrate?
> > +#define TRF7970A_QUIRK_IRQ_STATUS_READ_ERRATA BIT(0)
> Are there actually any TRF970 devices that do not need this quirk ?
Felipe thinks so :)
> > +static void trf7970a_drain_fifo(struct trf7970a *trf, u8 status)
> > +{
> > + struct sk_buff *skb = trf->rx_skb;
> > + int ret;
> > + u8 fifo_bytes;
> > +
> > + if (status & TRF7970A_IRQ_STATUS_ERROR) {
> > + trf7970a_abort_and_send_err(trf, -EIO);
> > + return;
> > + }
> > +
> > + ret = trf7970a_read(trf, TRF7970A_FIFO_STATUS, &fifo_bytes);
> > + if (ret) {
> > + trf7970a_abort_and_send_err(trf, ret);
> > + return;
> > + }
> > +
> > + dev_dbg(trf->dev, "fifo_bytes: 0x%x\n", fifo_bytes);
> > +
> > + if (!fifo_bytes)
> > + goto no_rx_data;
> > +
> > + if (fifo_bytes & TRF7970A_FIFO_STATUS_OVERFLOW) {
> > + dev_err(trf->dev, "%s - fifo overflow: 0x%x\n", __func__,
> > + fifo_bytes);
> > + trf7970a_abort_and_send_err(trf, -EIO);
> > + return;
> > + }
> > +
> > + if (fifo_bytes > skb_tailroom(skb)) {
> > + skb = skb_copy_expand(skb, skb_headroom(skb),
> > + max_t(int, fifo_bytes,
> > + TRF7970A_RX_SKB_ALLOC_SIZE),
> > + GFP_KERNEL);
> So there could be more pending bytes in the FIFO than you can accomodate
> in your rx_skb ?
Yes.
> Could we avoid that by allocating rx_skb to match the
> FIFO size ?
The alloc size (256) is already 2x the FIFO size (128) to try to minimize
this case from executing. We never know how many total bytes may come in
so we can't eliminate the code altogether.
> > +static void trf7970a_timeout_work_handler(struct work_struct *work)
> > +{
> > + struct trf7970a *trf = container_of(work, struct trf7970a,
> > + timeout_work.work);
> > +
> > + dev_dbg(trf->dev, "TIMEOUT - state: %d, ignore_timeout: %d\n",
> > + trf->state, trf->ignore_timeout);
> > +
> > + mutex_lock(&trf->lock);
> > +
> > + if (trf->ignore_timeout)
> > + trf->ignore_timeout = false;
> > + else if (trf->state == TRF7970A_ST_WAIT_FOR_RX_DATA_CONT)
> > + trf7970a_send_upstream(trf); /* No more rx data so send up */
> > + else
> > + trf7970a_abort_and_send_err(trf, -ETIMEDOUT);
> > +
> > + mutex_unlock(&trf->lock);
> > +}
> > +
> > +/* ----------------------------------------------------------------- */
> Nitpick: I suppose you're separating the internal logic from the digital
> ops here ? Please add one comment line for that.
OK
> > +static int trf7970a_in_send_cmd(struct nfc_digital_dev *ddev,
> > + struct sk_buff *skb, u16 timeout,
> > + nfc_digital_cmd_complete_t cb, void *arg)
> > +{
> > + struct trf7970a *trf = nfc_digital_get_drvdata(ddev);
> > + char *prefix;
> > + unsigned int len;
> > + int ret;
> > +
> > + dev_dbg(trf->dev, "New request - state: %d, timeout: %d ms, len: %d\n",
> > + trf->state, timeout, skb->len);
> > +
> > + if (skb->len > TRF7970A_TX_MAX)
> > + return -EINVAL;
> > +
> > + mutex_lock(&trf->lock);
> > +
> > + if ((trf->state != TRF7970A_ST_IDLE) &&
> > + (trf->state != TRF7970A_ST_IDLE_RX_BLOCKED)) {
> > + dev_err(trf->dev, "%s - Bogus state: %d\n", __func__,
> > + trf->state);
> > + ret = -EIO;
> > + goto out_err;
> > + }
> > +
> > + trf->rx_skb = nfc_alloc_recv_skb(TRF7970A_RX_SKB_ALLOC_SIZE,
> > + GFP_KERNEL);
> > + if (!trf->rx_skb) {
> > + dev_dbg(trf->dev, "Can't alloc rx_skb\n");
> > + ret = -ENOMEM;
> > + goto out_err;
> > + }
> > +
> > + if (trf->state == TRF7970A_ST_IDLE_RX_BLOCKED) {
> > + ret = trf7970a_cmd(trf, TRF7970A_CMD_ENABLE_RX);
> > + if (ret)
> > + goto out_err;
> > +
> > + trf->state = TRF7970A_ST_IDLE;
> > + }
> > +
> > + ret = trf7970a_per_cmd_config(trf, skb);
> > + if (ret)
> > + goto out_err;
> > +
> > + trf->ddev = ddev;
> > + trf->tx_skb = skb;
> As you're going to carry this guy around and may need it from e.g. your
> threaded interrupt handler, shouldn't you take a reference (skb_get) on it ?
> I'm concerned by the fact that you could see your tx_skb disappear from
> abort_cmd and get an IRQ before your state is set to IDLE.
> Hmm, I guess that's protected by the mutex and so when you get an abort
> from the digital stack you reset the state to IDLE and no one should try
> to touch tx_skb after you release the mutex. Is that what you had in
> mind ?
It is but taking a reference is a good idea. I'll add that.
> > +static int trf7970a_probe(struct spi_device *spi)
> > +{
> > + struct device_node *np = spi->dev.of_node;
> > + const struct spi_device_id *id = spi_get_device_id(spi);
> > + struct trf7970a *trf;
> > + int ret;
> > +
> > + if (!np) {
> > + dev_err(&spi->dev, "No Device Tree entry\n");
> > + return -EINVAL;
> > + }
> > +
> > + trf = devm_kzalloc(&spi->dev, sizeof(*trf), GFP_KERNEL);
> > + if (!trf)
> > + return -ENOMEM;
> > +
> > + trf->state = TRF7970A_ST_OFF;
> > + trf->dev = &spi->dev;
> > + trf->spi = spi;
> > + trf->quirks = id->driver_data;
> > + trf->initialized = false;
> > +
> > + spi->mode = SPI_MODE_1;
> > + spi->bits_per_word = 8;
> > +
> > + /* Get the optional Slave Select GPIO used for SPI with SS mode */
> > + trf->ss_gpio = of_get_named_gpio(np, "ti,ss-gpio", 0);
> > + if (trf->ss_gpio >= 0) {
> > + ret = devm_gpio_request_one(trf->dev, trf->ss_gpio,
> > + GPIOF_DIR_OUT | GPIOF_INIT_LOW, "SS");
> > + if (ret) {
> > + dev_err(trf->dev, "Can't request SS GPIO: %d\n", ret);
> > + return ret;
> > + }
> > + } else {
> > + dev_info(trf->dev, "Using SPI without SS mode\n");
> > + }
> > +
> > + /* There are two enable pins - both must be present */
> > + trf->en_gpio = of_get_named_gpio(np, "ti,enable-gpios", 0);
> > + if (trf->en_gpio < 0) {
> > + dev_err(trf->dev, "No EN GPIO property\n");
> > + return ret;
> You probably want to return trf->en_gpio or otherwise the devm code
> won't release your managed resources.
Oops. Good catch, thx.
> > + }
> > +
> > + ret = devm_gpio_request_one(trf->dev, trf->en_gpio,
> > + GPIOF_DIR_OUT | GPIOF_INIT_LOW, "EN");
> > + if (ret) {
> > + dev_err(trf->dev, "Can't request EN GPIO: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + trf->en2_gpio = of_get_named_gpio(np, "ti,enable-gpios", 1);
> > + if (trf->en2_gpio < 0) {
> > + dev_err(trf->dev, "No EN2 GPIO property\n");
> > + return ret;
> Ditto.
OK
Mark
--
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 2/3] NFC: trf7970a: Add ISO/IEC 15693 Support
From: Mark A. Greer @ 2014-02-26 21:53 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Lauro Ramos Venancio, Aloisio Almeida Jr, Felipe Balbi,
Erick Macias, Thierry Escande,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-nfc-hn68Rpc1hR1g9hUCZPvPmw,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140221010044.GT18868@zurbaran>
On Fri, Feb 21, 2014 at 02:00:44AM +0100, Samuel Ortiz wrote:
> Hi Mark,
>
> Just one nitpick here:
>
> > +static int trf7970a_is_iso15693_write_or_lock(u8 cmd)
> > +{
> > + int ret;
> > +
> > + switch (cmd) {
> > + case ISO15693_CMD_WRITE_SINGLE_BLOCK:
> > + case ISO15693_CMD_LOCK_BLOCK:
> > + case ISO15693_CMD_WRITE_MULTIPLE_BLOCK:
> > + case ISO15693_CMD_WRITE_AFI:
> > + case ISO15693_CMD_LOCK_AFI:
> > + case ISO15693_CMD_WRITE_DSFID:
> > + case ISO15693_CMD_LOCK_DSFID:
> > + ret = 1;
> return 1;
>
> and return 0 on the default case.
> Simpler code and you don't need the ret variable.
OK
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 3/3] NFC: trf7970a: Add DTS Documentation
From: Mark A. Greer @ 2014-02-26 21:54 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Lauro Ramos Venancio, Aloisio Almeida Jr, Felipe Balbi,
Erick Macias, Thierry Escande,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-nfc-hn68Rpc1hR1g9hUCZPvPmw,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140221010407.GU18868@zurbaran>
On Fri, Feb 21, 2014 at 02:04:07AM +0100, Samuel Ortiz wrote:
> Hi Mark,
>
> On Fri, Jan 31, 2014 at 03:17:11PM -0700, Mark A. Greer wrote:
> > Describe the properies used by the trf7970a
> > RFID/NFC/15693 transceiver driver.
> >
> > Signed-off-by: Mark A. Greer <mgreer-luAo+O/VEmrlveNOaEYElw@public.gmane.org>
> > ---
> > .../devicetree/bindings/net/nfc/trf7970a.txt | 37 ++++++++++++++++++++++
> > 1 file changed, 37 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> Not a DT expert, but this looks good to me.
> Could you please amend this patch by adding
> Documentation/devicetree/bindings/net/nfc/ to the MAINTAINERS NFC
> section ?
Sure.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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: [PATCHv1 1/2] rx51_battery: convert to iio consumer
From: Sebastian Reichel @ 2014-02-26 21:54 UTC (permalink / raw)
To: Belisko Marek
Cc: Dmitry Eremin-Solenikov, David Woodhouse, Jonathan Cameron,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Grant Likely, LKML,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Pali Rohár
In-Reply-To: <CAAfyv37B=xn3AktnUxPDpuey+5S73vW0PJnuUZtorP4rajPKiw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 833 bytes --]
Hi,
On Wed, Feb 26, 2014 at 10:43:40PM +0100, Belisko Marek wrote:
> [...]
> > + int val, err;
> > + err = iio_read_channel_average_raw(channel, &val);
> Where this function comes from? I cannot find it in current linux-next
> (only iio_read_channel_raw()). Am I missing some patches? Thx.
Ah right. I planned to send a patch adding this function together
with the rx51-battery patchset, but it seems I forgot to include it.
Sorry for the inconvenience. I will send it out as a separate patch
now.
> BTW when I convert to iio consumer and use DT some of values work
> but some of them just report 0 :( (I don't have latest twl4030-madc
> iio patches).
Can you retry with the twl4030-madc iio patch from today? The
older patchsets, which do not contain the "tested on real hw"
note are slightly broken.
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] phy: micrel: add of configuration for LED mode
From: David Miller @ 2014-02-26 22:00 UTC (permalink / raw)
To: ben.dooks; +Cc: linux-kernel, devicetree, netdev, f.fainelli
In-Reply-To: <1393415280-10227-1-git-send-email-ben.dooks@codethink.co.uk>
From: Ben Dooks <ben.dooks@codethink.co.uk>
Date: Wed, 26 Feb 2014 11:48:00 +0000
> Add support for the led-mode property for the following PHYs
> which have a single LED mode configuration value.
>
> KSZ8001 and KSZ8041 which both use register 0x1e bits 15,14 and
> KSZ8021, KSZ8031 and KSZ8051 which use register 0x1f bits 5,4
> to control the LED configuration.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Applied, thanks.
^ permalink raw reply
* [PATCH v8.1 1/6] dt-bindings: sram: describe option to reserve parts of the memory
From: Heiko Stübner @ 2014-02-26 22:02 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
arm-DgEjT+Ai2ygdnm+yROfE0A, Grant Likely, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Philipp Zabel,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
Sachin Kamat
In-Reply-To: <201402251419.31151.arnd-r2nGTMty4D4@public.gmane.org>
Some SoCs need parts of their sram for special purposes. So while being part
of the peripheral, it should not be part of the genpool controlling the sram.
Therefore add the option to define reserved regions as subnodes of the
sram-node similar to defining reserved global memory regions.
Originally
Suggested-by: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Using subnodes for reserved regions
Suggested-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Tested-by: Ulrich Prinz <ulrich.prinz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Hi Arnd,
I've changed the range definition as suggested. I've also changed my
rockchip dt-files accordingly, but will wait with respinning the whole
series until the direction with this binding is a bit safer.
I've also amended the "compatible"-property for the subnodes. It now
mimics the text from the generic reserved memory nodes [0],
which got no complaints from the DT maintainers.
Heiko
[0] https://lkml.org/lkml/2014/2/4/172
Documentation/devicetree/bindings/misc/sram.txt | 35 +++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/Documentation/devicetree/bindings/misc/sram.txt b/Documentation/devicetree/bindings/misc/sram.txt
index 4d0a00e..36cbe5a 100644
--- a/Documentation/devicetree/bindings/misc/sram.txt
+++ b/Documentation/devicetree/bindings/misc/sram.txt
@@ -8,9 +8,44 @@ Required properties:
- reg : SRAM iomem address range
+Reserving sram areas:
+---------------------
+
+Each child of the sram node specifies a region of reserved memory. Each
+child node should use a 'reg' property to specify a specific range of
+reserved memory.
+
+Following the generic-names recommended practice, node names should
+reflect the purpose of the node. Unit address (@<address>) should be
+appended to the name.
+
+Required properties in the sram node:
+
+- #address-cells, #size-cells : should use the same values as the root node
+- ranges : standard definition, should translate from local addresses
+ within the sram to bus addresses
+
+Required properties in the area nodes:
+
+- reg : iomem address range, relative to the SRAM range
+
+Optional properties in the area nodes:
+
+- compatible : standard definition, should contain a vendor specific string
+ in the form <vendor>,[<device>-]<usage>
+
Example:
sram: sram@5c000000 {
compatible = "mmio-sram";
reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */
+
+ #adress-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x5c000000 0x40000>;
+
+ smp-sram@100 {
+ compatible = "socvendor,smp-sram";
+ reg = <0x100 0x50>;
+ };
};
--
1.8.5.3
--
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 related
* Re: [PATCH 3/4] ARM: dts: OMAP4: Add IOMMU nodes
From: Suman Anna @ 2014-02-26 22:06 UTC (permalink / raw)
To: Laurent Pinchart
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Tony Lindgren,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Benoit Cousson,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Florian Vaussard
In-Reply-To: <5025565.C2g8cq3k2V@avalon>
Hi Laurent,
On 02/26/2014 03:05 PM, Laurent Pinchart wrote:
> Hi Suman,
>
> Thank you for the patch.
>
> On Thursday 13 February 2014 12:22:55 Suman Anna wrote:
>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>
>> Add the IOMMU nodes for the DSP and IPU subsystems. The external
>> address space for DSP starts at 0x20000000 in OMAP4 compared to
>> 0x11000000 in OMAP3, and the addresses beyond 0xE0000000 are
>> private address space for the Cortex-M3 cores in the IPU subsystem.
>> The MMU within the IPU sub-system also supports a bus error back
>> capability, not available on the DSP MMU.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>> [s-anna-l0cyMroinI0@public.gmane.org: dma-window updates and bus error back addition]
>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>> ---
>> arch/arm/boot/dts/omap4.dtsi | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index d3f8a6e..1885f90 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -461,6 +461,23 @@
>> dma-names = "tx", "rx";
>> };
>>
>> + mmu_dsp: mmu@4a066000 {
>> + compatible = "ti,omap4-iommu";
>> + reg = <0x4a066000 0xff>;
>> + interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
>> + ti,hwmods = "mmu_dsp";
>> + dma-window = <0x20000000 0xdffff000>;
>> + };
>> +
>> + mmu_ipu: mmu@55082000 {
>> + compatible = "ti,omap4-iommu";
>> + reg = <0x55082000 0xff>;
>> + interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
>> + ti,hwmods = "mmu_ipu";
>> + dma-window = <0 0xdffff000>;
>
> I'm not too familiar with the M3 MPU in the OMAP4, but doesn't its memory map
> also include other reserved regions, such as 0x55040000- 0x5505ffff to access
> the ISS ?
The MMU integration into the M3 MPU subsystem is actually slightly
different to that seen on DSP in OMAP3. The M3 MPU subsystem actually
has one additional type of MMU, called the Attribute MMU/Unicache MMU
immediately after the M3 processor. It is programmed completely from the
M3, and is used mainly for setting the cache attributes and valid
address ranges and translations for accessing address ranges like the
ISS space. The L2 MMU interprets the remaining addresses, so yes, there
are certain other address ranges that never go through the L2MMU. The
dma-window values are used by omap-iovmm, but OMAP4 does not make use of
omap-iovmm.
regards
Suman
>
>> + ti,iommu-bus-err-back;
>> + };
>> +
>> wdt2: wdt@4a314000 {
>> compatible = "ti,omap4-wdt", "ti,omap3-wdt";
>> reg = <0x4a314000 0x80>;
>
^ permalink raw reply
* Re: [PATCH v2 3/6] spi: sh-msiof: Add support for R-Car H2 and M2
From: Laurent Pinchart @ 2014-02-26 22:16 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Mark Brown, Takashi Yoshii, Magnus Damm, linux-spi, linux-sh,
linux-kernel, Geert Uytterhoeven, devicetree
In-Reply-To: <1393323673-2751-4-git-send-email-geert@linux-m68k.org>
Hi Geert,
Thank you for the patch.
Overall the series is great. I ran some time ago into issues with CCF due to
the driver use of spi-bitbang, I'm happy to see this being fixed, thanks a
lot. I have one small comment below though.
On Tuesday 25 February 2014 11:21:10 Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>
> Add support for the MSIOF variant in the R-Car H2 (r8a7790) and M2
> (r8a7791) SoCs.
>
> Binding documentation:
> - Add future-proof "renesas,msiof-<soctype>" compatible values,
> - The default for "renesas,rx-fifo-size" is 256 on R-Car H2 and M2,
> - "renesas,tx-fifo-size" and "renesas,rx-fifo-size" are deprecated for
> soctype-specific bindings,
> - Add example bindings.
>
> Implementation:
> - MSIOF on R-Car H2 and M2 requires the transmission of dummy data if
> data is being received only (cfr. "Set SICTR.TSCKE to 1" and "Write
> dummy transmission data to SITFDR" in paragraph "Transmit and Receive
> Procedures" of the Hardware User's Manual).
> - As RX depends on TX, MSIOF on R-Car H2 and M2 also lacks the RSCR
> register (Receive Clock Select Register), and some bits in the RMDR1
> (Receive Mode Register 1) and TMDR2 (Transmit Mode Register 2)
> registers.
> - Use the recently introduced SPI_MASTER_MUST_TX flag to enable support
> for dummy transmission in the SPI core, and to differentiate from other
> MSIOF implementations in code paths that need this.
> - New DT compatible values ("renesas,msiof-r8a7790" and
> "renesas,msiof-r8a7791") are added, as well as new platform device
> names ("spi_r8a7790_msiof" and "spi_r8a7791_msiof").
> - The default RX FIFO size is 256 words on R-Car H2 and M2.
>
> This is loosely based on a set of patches from Takashi Yoshii
> <takasi-y@ops.dti.ne.jp>.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> Cc: Takashi Yoshii <takasi-y@ops.dti.ne.jp>
> Cc: devicetree@vger.kernel.org
> ---
> v2:
> - Rebased on top of new "spi: sh-msiof: Move default FIFO sizes to device
> ID data",
> - The default RX FIFO size is 256 words on R-Car H2 and M2,
> - Deprecated overriding the FIFO size,
> - Synced DT example with node from real DTS.
>
> Documentation/devicetree/bindings/spi/sh-msiof.txt | 23 +++++++++++++++--
> drivers/spi/spi-sh-msiof.c | 23 ++++++++++++++---
> 2 files changed, 41 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt
> b/Documentation/devicetree/bindings/spi/sh-msiof.txt index
> eae3c8c9300e..1f0cb33763a1 100644
> --- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
> +++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
> @@ -1,8 +1,13 @@
> Renesas MSIOF spi controller
>
> Required properties:
> -- compatible : "renesas,sh-msiof" for SuperH, or
> +- compatible : "renesas,msiof-<soctype>" for SoCs,
> + "renesas,sh-msiof" for SuperH, or
> "renesas,sh-mobile-msiof" for SH Mobile series.
> + Examples with soctypes are:
> + "renesas,msiof-sh7724" (SH)
Given that the driver doesn't handle the "renesas,msiof-sh7724" compatible
string this might not be a good example. Furthermore SuperH doesn't have DT
support. I would thus drop the "renesas,sh-msiof" compatible string from patch
1/6 and wouldn't mention sh7724 here. I very much doubt that someone would
have developed DT support for SuperH on the side and shipped products that
would be broken by this change :-)
> + "renesas,msiof-r8a7790" (R-Car H2)
> + "renesas,msiof-r8a7791" (R-Car M2)
> - reg : Offset and length of the register set for the
> device - interrupt-parent : The phandle for the interrupt controller
> that services interrupts for this device
> @@ -13,10 +18,24 @@ Required properties:
> Optional properties:
> - clocks : Must contain a reference to the functional clock.
> - num-cs : Total number of chip-selects (default is 1)
> +
> +Optional properties, deprecated for soctype-specific bindings:
> - renesas,tx-fifo-size : Overrides the default tx fifo size given in words
> (default is 64)
> - renesas,rx-fifo-size : Overrides the default rx fifo size given in words
> - (default is 64)
> + (default is 64, or 256 on R-Car H2 and M2)
>
> Pinctrl properties might be needed, too. See
> Documentation/devicetree/bindings/pinctrl/renesas,*.
> +
> +Example:
> +
> + msiof0: spi@e6e20000 {
> + compatible = "renesas,msiof-r8a7791";
> + reg = <0 0xe6e20000 0 0x0064>;
> + interrupts = <0 156 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&mstp0_clks R8A7791_CLK_MSIOF0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index bf389184924d..3baef2bacaed 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -34,6 +34,7 @@
> struct sh_msiof_chipdata {
> u16 tx_fifo_size;
> u16 rx_fifo_size;
> + u16 master_flags;
> };
>
> struct sh_msiof_spi_priv {
> @@ -214,7 +215,8 @@ static void sh_msiof_spi_set_clk_regs(struct
> sh_msiof_spi_priv *p, k = min_t(int, k, ARRAY_SIZE(sh_msiof_spi_clk_table)
> - 1);
>
> sh_msiof_write(p, TSCR, sh_msiof_spi_clk_table[k].scr);
> - sh_msiof_write(p, RSCR, sh_msiof_spi_clk_table[k].scr);
> + if (!(p->chipdata->master_flags & SPI_MASTER_MUST_TX))
> + sh_msiof_write(p, RSCR, sh_msiof_spi_clk_table[k].scr);
> }
>
> static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p,
> @@ -237,6 +239,10 @@ static void sh_msiof_spi_set_pin_regs(struct
> sh_msiof_spi_priv *p, tmp |= !cs_high << MDR1_SYNCAC_SHIFT;
> tmp |= lsb_first << MDR1_BITLSB_SHIFT;
> sh_msiof_write(p, TMDR1, tmp | MDR1_TRMD | TMDR1_PCON);
> + if (p->chipdata->master_flags & SPI_MASTER_MUST_TX) {
> + /* These bits are reserved if RX needs TX */
> + tmp &= ~0x0000ffff;
> + }
> sh_msiof_write(p, RMDR1, tmp);
>
> tmp = 0;
> @@ -257,7 +263,7 @@ static void sh_msiof_spi_set_mode_regs(struct
> sh_msiof_spi_priv *p, {
> u32 dr2 = MDR2_BITLEN1(bits) | MDR2_WDLEN1(words);
>
> - if (tx_buf)
> + if (tx_buf || (p->chipdata->master_flags & SPI_MASTER_MUST_TX))
> sh_msiof_write(p, TMDR2, dr2);
> else
> sh_msiof_write(p, TMDR2, dr2 | MDR2_GRPMASK1);
> @@ -666,11 +672,20 @@ static u32 sh_msiof_spi_txrx_word(struct spi_device
> *spi, unsigned nsecs, static const struct sh_msiof_chipdata sh_data = {
> .tx_fifo_size = 64,
> .rx_fifo_size = 64,
> + .master_flags = 0,
> +};
> +
> +static const struct sh_msiof_chipdata r8a779x_data = {
> + .tx_fifo_size = 64,
> + .rx_fifo_size = 256,
> + .master_flags = SPI_MASTER_MUST_TX,
> };
>
> static const struct of_device_id sh_msiof_match[] = {
> { .compatible = "renesas,sh-msiof", .data = &sh_data },
> { .compatible = "renesas,sh-mobile-msiof", .data = &sh_data },
> + { .compatible = "renesas,msiof-r8a7790", .data = &r8a779x_data },
> + { .compatible = "renesas,msiof-r8a7791", .data = &r8a779x_data },
> {},
> };
> MODULE_DEVICE_TABLE(of, sh_msiof_match);
> @@ -789,7 +804,7 @@ static int sh_msiof_spi_probe(struct platform_device
> *pdev) /* init master and bitbang code */
> master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
> master->mode_bits |= SPI_LSB_FIRST | SPI_3WIRE;
> - master->flags = 0;
> + master->flags = p->chipdata->master_flags;
> master->bus_num = pdev->id;
> master->dev.of_node = pdev->dev.of_node;
> master->num_chipselect = p->info->num_chipselect;
> @@ -832,6 +847,8 @@ static int sh_msiof_spi_remove(struct platform_device
> *pdev)
>
> static struct platform_device_id spi_driver_ids[] = {
> { "spi_sh_msiof", (kernel_ulong_t)&sh_data },
> + { "spi_r8a7790_msiof", (kernel_ulong_t)&r8a779x_data },
> + { "spi_r8a7791_msiof", (kernel_ulong_t)&r8a779x_data },
> {},
> };
> MODULE_DEVICE_TABLE(platform, spi_driver_ids);
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings
From: Suman Anna @ 2014-02-26 22:18 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Tony Lindgren,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kumar Gala,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Florian Vaussard
In-Reply-To: <1825184.Z7lGjn3qsj@avalon>
Hi Laurent,
On 02/26/2014 02:36 PM, Laurent Pinchart wrote:
> Hi Suman,
>
> On Wednesday 26 February 2014 14:23:03 Suman Anna wrote:
>>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
>>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
>>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
>>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
>>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
>>>>>>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>>
>>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
>>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a
>>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
>>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
>>>>>>>>
>>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error back]
>>>>>>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 ++++++++++++
>>>>>>>> 1 file changed, 28 insertions(+)
>>>>>>>> create mode 100644
>>>>>>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>
>>>>>>>> diff --git
>>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file
>>>>>>>> mode 100644
>>>>>>>> index 0000000..116492d
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>> @@ -0,0 +1,28 @@
>>>>>>>> +OMAP2+ IOMMU
>>>>>>>> +
>>>>>>>> +Required properties:
>>>>>>>> +- compatible : Should be one of,
>>>>>>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
>>>>>>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
>>>>>>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances
>>>>>>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU instance
>>>>>>>> +- reg : Address space for the configuration registers
>>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
>>>>>>>> +- dma-window : IOVA start address and length
>>>>>>>
>>>>>>> Isn't the dma window more of a system configuration property than a
>>>>>>> hardware property ? How do you expect it to be set?
>>>>>>
>>>>>> We are setting it based on the addressable range for the MMU.
>>>>>
>>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
>>>>> support the full 4GB VA space. Why do you need to restrict it ?
>>>>
>>>> I should have rephrased it better when I said addressable range. While
>>>> the MMUs are capable of programming the full 4GB space, there are some
>>>> address ranges that are private from the processor view. This window is
>>>> currently used to set the range for the omap-iovmm driver (which only
>>>> OMAP3 ISP is using atm), and there is no point in allowing the
>>>> omap-iovmm driver the full range when the processor could never
>>>> reach/access those addresses.
>>>
>>> But the IOMMU VA space is from a device point of view, not from a CPU
>>> point of view. Could you point me to where those private ranges are
>>> documented, in order to understand the problem correctly ?
>>
>> Yes, they are indeed from the device perspective. I meant DSP and/or IPU
>> by processor.
>>
>> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP
>> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external
>> addressable range starts from 0x11000000.
>
> OK, so it looks more like a property of the IOMMU master than a property of
> the IOMMU itself. It would be better to express it as such, but I wonder how
> that could be done, and if it would be worth it in this case.
This property is currently solely used to configure the range for the
omap-iovmm module, which were supplied through platform data in the
non-DT case. I am wondering if the way to go here is to use
iommu_domain_set_attr() and use the domain geometry values.
regards
Suman
>
> As not all masters (the OMAP3 ISP doesn't for instance) have restrictions
> regarding the VA range they can address, should this property be at least made
> optional ?
>
>>>>>> We are reusing the existing defined property and it allows us to get
>>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP
>>>>>> iommu platform data.
>>>>>>
>>>>>>>> +Optional properties:
>>>>>>>> +- ti,#tlb-entries : Number of entries in the translation look-aside
>>>>>>>> buffer. + Should be either 8 or 32 (default: 32)
>>>>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports
>>>>>>>> throwing
>>>>>>>> + back a bus error response on MMU faults.
>>>>>>>
>>>>>>> Do these features vary per IOMMU instance or per IOMMU model ? In the
>>>>>>> latter case they could be inferred from the compatible string by the
>>>>>>> driver without requiring them to be explicit in DT (whether you want
>>>>>>> to do so is left to you though).
>>>>>>
>>>>>> Well, these are fixed features given an IOMMU instance, like the OMAP3
>>>>>> ISP is the only one that has 8 TLB entries, all the remaining ones have
>>>>>> 32, and the IPU iommu instances are the only ones that support the bus
>>>>>> error response back. I have no preference to any particular way, and
>>>>>> sure the driver can infer these easily based on unique compatible
>>>>>> strings per subsystem per SoC. I just happened to go with defining
>>>>>> compatible strings per SoC, with the optional properties
>>>>>> differentiating the fixed behavior between different IOMMU instances on
>>>>>> that SoC. This is where I was looking for some inputs/guidance from the
>>>>>> DT bindings maintainers on what is the preferred method.
>>>>>
>>>>> I think you've made the right choice. I wasn't sure whether those
>>>>> parameters varied across IOMMU instances of compatible devices (from a
>>>>> compatible string point of view) or were constant. As they vary they
>>>>> should be expressed in DT.
>>>>
>>>> Yeah, I wasn't sure if these qualify as features (as per
>>>> Documentation/devicetree/bindings/ABI.txt section II.2).
>>>>
>>>> regards
>>>> Suman
>>>>
>>>>>>>> +Example:
>>>>>>>> + /* OMAP3 ISP MMU */
>>>>>>>> + mmu_isp: mmu@480bd400 {
>>>>>>>> + compatible = "ti,omap2-iommu";
>>>>>>>> + reg = <0x480bd400 0x80>;
>>>>>>>> + interrupts = <24>;
>>>>>>>> + ti,hwmods = "mmu_isp";
>>>>>>>> + ti,#tlb-entries = <8>;
>>>>>>>> + dma-window = <0 0xfffff000>;
>>>>>>>> + };
>
^ permalink raw reply
* Re: [PATCH] phy: micrel: add of configuration for LED mode
From: Florian Fainelli @ 2014-02-26 22:20 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-kernel, devicetree@vger.kernel.org, netdev
In-Reply-To: <1393415280-10227-1-git-send-email-ben.dooks@codethink.co.uk>
Hi,
2014-02-26 3:48 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
[snip]
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 5a8993b..0c9e434 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -148,15 +148,52 @@ static int ks8737_config_intr(struct phy_device *phydev)
> return rc < 0 ? rc : 0;
> }
>
> +static int kszphy_setup_led(struct phy_device *phydev,
> + unsigned int reg, unsigned int shift)
> +{
> +
> + struct device *dev = &phydev->dev;
> + struct device_node *of_node = dev->of_node;
> + int rc, temp;
> + u32 val;
> +
> + if (!of_node && dev->parent->of_node)
> + of_node = dev->parent->of_node;
> +
> + if (of_property_read_u32(of_node, "micrel,led-mode", &val))
> + return 0;
This breaks non-OF configuration because of_read_property_read_u32()
will return -ENOSYS, so you skip the LED register configuration
entirely, is that intended?
> +
> + temp = phy_read(phydev, reg);
> + if (temp < 0)
> + return temp;
> +
> + temp &= 3 << shift;
The compiler cannot verify that we are not overflowing, you might want
to make sure that shift <= 14 (just in case)
> + temp |= val << shift;
> + rc = phy_write(phydev, reg, temp);
> +
> + return rc < 0 ? rc : 0;
You could have;
return phy_write(phydev, reg, temp);
> +}
> +
> static int kszphy_config_init(struct phy_device *phydev)
> {
> return 0;
> }
>
> +static int kszphy_config_init_led8041(struct phy_device *phydev)
> +{
> + /* single led control, register 0x1e bits 15..14 */
> + return kszphy_setup_led(phydev, 0x1e, 14);
> +}
> +
> static int ksz8021_config_init(struct phy_device *phydev)
> {
> - int rc;
> const u16 val = KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE;
> + int rc;
> +
> + rc = kszphy_setup_led(phydev, 0x1f, 4);
> + if (rc)
> + dev_err(&phydev->dev, "failed to set led mode\n");
> +
> phy_write(phydev, MII_KSZPHY_OMSO, val);
> rc = ksz_config_flags(phydev);
> return rc < 0 ? rc : 0;
> @@ -166,6 +203,10 @@ static int ks8051_config_init(struct phy_device *phydev)
> {
> int rc;
>
> + rc = kszphy_setup_led(phydev, 0x1f, 4);
> + if (rc)
> + dev_err(&phydev->dev, "failed to set led mode\n");
> +
> rc = ksz_config_flags(phydev);
> return rc < 0 ? rc : 0;
> }
> @@ -327,7 +368,7 @@ static struct phy_driver ksphy_driver[] = {
> .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause
> | SUPPORTED_Asym_Pause),
> .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
> - .config_init = kszphy_config_init,
> + .config_init = kszphy_config_init_led8041,
> .config_aneg = genphy_config_aneg,
> .read_status = genphy_read_status,
> .ack_interrupt = kszphy_ack_interrupt,
> @@ -342,7 +383,7 @@ static struct phy_driver ksphy_driver[] = {
> .features = PHY_BASIC_FEATURES |
> SUPPORTED_Pause | SUPPORTED_Asym_Pause,
> .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
> - .config_init = kszphy_config_init,
> + .config_init = kszphy_config_init_led8041,
> .config_aneg = genphy_config_aneg,
> .read_status = genphy_read_status,
> .ack_interrupt = kszphy_ack_interrupt,
> @@ -371,7 +412,7 @@ static struct phy_driver ksphy_driver[] = {
> .phy_id_mask = 0x00ffffff,
> .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
> .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
> - .config_init = kszphy_config_init,
> + .config_init = kszphy_config_init_led8041,
> .config_aneg = genphy_config_aneg,
> .read_status = genphy_read_status,
> .ack_interrupt = kszphy_ack_interrupt,
> --
> 1.8.5.3
>
--
Florian
^ permalink raw reply
* Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings
From: Suman Anna @ 2014-02-26 22:28 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Tony Lindgren,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kumar Gala,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Florian Vaussard
In-Reply-To: <530E682D.9070005-l0cyMroinI0@public.gmane.org>
On 02/26/2014 04:18 PM, Suman Anna wrote:
> Hi Laurent,
>
> On 02/26/2014 02:36 PM, Laurent Pinchart wrote:
>> Hi Suman,
>>
>> On Wednesday 26 February 2014 14:23:03 Suman Anna wrote:
>>>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
>>>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
>>>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
>>>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
>>>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
>>>>>>>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>>>
>>>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
>>>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a
>>>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
>>>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error
>>>>>>>>> back]
>>>>>>>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28
>>>>>>>>> ++++++++++++
>>>>>>>>> 1 file changed, 28 insertions(+)
>>>>>>>>> create mode 100644
>>>>>>>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>>
>>>>>>>>> diff --git
>>>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new
>>>>>>>>> file
>>>>>>>>> mode 100644
>>>>>>>>> index 0000000..116492d
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>> @@ -0,0 +1,28 @@
>>>>>>>>> +OMAP2+ IOMMU
>>>>>>>>> +
>>>>>>>>> +Required properties:
>>>>>>>>> +- compatible : Should be one of,
>>>>>>>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
>>>>>>>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
>>>>>>>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances
>>>>>>>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU
>>>>>>>>> instance
>>>>>>>>> +- reg : Address space for the configuration registers
>>>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
>>>>>>>>> +- dma-window : IOVA start address and length
>>>>>>>>
>>>>>>>> Isn't the dma window more of a system configuration property than a
>>>>>>>> hardware property ? How do you expect it to be set?
>>>>>>>
>>>>>>> We are setting it based on the addressable range for the MMU.
>>>>>>
>>>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
>>>>>> support the full 4GB VA space. Why do you need to restrict it ?
>>>>>
>>>>> I should have rephrased it better when I said addressable range. While
>>>>> the MMUs are capable of programming the full 4GB space, there are some
>>>>> address ranges that are private from the processor view. This
>>>>> window is
>>>>> currently used to set the range for the omap-iovmm driver (which only
>>>>> OMAP3 ISP is using atm), and there is no point in allowing the
>>>>> omap-iovmm driver the full range when the processor could never
>>>>> reach/access those addresses.
>>>>
>>>> But the IOMMU VA space is from a device point of view, not from a CPU
>>>> point of view. Could you point me to where those private ranges are
>>>> documented, in order to understand the problem correctly ?
>>>
>>> Yes, they are indeed from the device perspective. I meant DSP and/or IPU
>>> by processor.
>>>
>>> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP
>>> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external
>>> addressable range starts from 0x11000000.
>>
>> OK, so it looks more like a property of the IOMMU master than a
>> property of
>> the IOMMU itself. It would be better to express it as such, but I
>> wonder how
>> that could be done, and if it would be worth it in this case.
>
> This property is currently solely used to configure the range for the
> omap-iovmm module, which were supplied through platform data in the
> non-DT case. I am wondering if the way to go here is to use
> iommu_domain_set_attr() and use the domain geometry values.
The other option is to supply these as driver match data, and switching
the compatible strings to identify the MMU instance precisely.
regards
Suman
>>
>> As not all masters (the OMAP3 ISP doesn't for instance) have restrictions
>> regarding the VA range they can address, should this property be at
>> least made
>> optional ?
>>
>>>>>>> We are reusing the existing defined property and it allows us to get
>>>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP
>>>>>>> iommu platform data.
>>>>>>>
>>>>>>>>> +Optional properties:
>>>>>>>>> +- ti,#tlb-entries : Number of entries in the translation
>>>>>>>>> look-aside
>>>>>>>>> buffer. + Should be either 8 or 32 (default:
>>>>>>>>> 32)
>>>>>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports
>>>>>>>>> throwing
>>>>>>>>> + back a bus error response on MMU faults.
>>>>>>>>
>>>>>>>> Do these features vary per IOMMU instance or per IOMMU model ?
>>>>>>>> In the
>>>>>>>> latter case they could be inferred from the compatible string by
>>>>>>>> the
>>>>>>>> driver without requiring them to be explicit in DT (whether you
>>>>>>>> want
>>>>>>>> to do so is left to you though).
>>>>>>>
>>>>>>> Well, these are fixed features given an IOMMU instance, like the
>>>>>>> OMAP3
>>>>>>> ISP is the only one that has 8 TLB entries, all the remaining
>>>>>>> ones have
>>>>>>> 32, and the IPU iommu instances are the only ones that support
>>>>>>> the bus
>>>>>>> error response back. I have no preference to any particular way, and
>>>>>>> sure the driver can infer these easily based on unique compatible
>>>>>>> strings per subsystem per SoC. I just happened to go with defining
>>>>>>> compatible strings per SoC, with the optional properties
>>>>>>> differentiating the fixed behavior between different IOMMU
>>>>>>> instances on
>>>>>>> that SoC. This is where I was looking for some inputs/guidance
>>>>>>> from the
>>>>>>> DT bindings maintainers on what is the preferred method.
>>>>>>
>>>>>> I think you've made the right choice. I wasn't sure whether those
>>>>>> parameters varied across IOMMU instances of compatible devices
>>>>>> (from a
>>>>>> compatible string point of view) or were constant. As they vary they
>>>>>> should be expressed in DT.
>>>>>
>>>>> Yeah, I wasn't sure if these qualify as features (as per
>>>>> Documentation/devicetree/bindings/ABI.txt section II.2).
>>>>>
>>>>> regards
>>>>> Suman
>>>>>
>>>>>>>>> +Example:
>>>>>>>>> + /* OMAP3 ISP MMU */
>>>>>>>>> + mmu_isp: mmu@480bd400 {
>>>>>>>>> + compatible = "ti,omap2-iommu";
>>>>>>>>> + reg = <0x480bd400 0x80>;
>>>>>>>>> + interrupts = <24>;
>>>>>>>>> + ti,hwmods = "mmu_isp";
>>>>>>>>> + ti,#tlb-entries = <8>;
>>>>>>>>> + dma-window = <0 0xfffff000>;
>>>>>>>>> + };
>>
>
^ permalink raw reply
* Re: [PATCH v2 3/6] spi: sh-msiof: Add support for R-Car H2 and M2
From: Geert Uytterhoeven @ 2014-02-26 22:36 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mark Brown, Takashi Yoshii, Magnus Damm, linux-spi, Linux-sh list,
linux-kernel@vger.kernel.org, Geert Uytterhoeven,
devicetree@vger.kernel.org
In-Reply-To: <38730952.JbtPf2mapG@avalon>
Hi Laurent,
On Wed, Feb 26, 2014 at 11:16 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt
>> b/Documentation/devicetree/bindings/spi/sh-msiof.txt index
>> eae3c8c9300e..1f0cb33763a1 100644
>> --- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
>> +++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
>> @@ -1,8 +1,13 @@
>> Renesas MSIOF spi controller
>>
>> Required properties:
>> -- compatible : "renesas,sh-msiof" for SuperH, or
>> +- compatible : "renesas,msiof-<soctype>" for SoCs,
>> + "renesas,sh-msiof" for SuperH, or
>> "renesas,sh-mobile-msiof" for SH Mobile series.
>> + Examples with soctypes are:
>> + "renesas,msiof-sh7724" (SH)
>
> Given that the driver doesn't handle the "renesas,msiof-sh7724" compatible
> string this might not be a good example. Furthermore SuperH doesn't have DT
> support. I would thus drop the "renesas,sh-msiof" compatible string from patch
> 1/6 and wouldn't mention sh7724 here. I very much doubt that someone would
> have developed DT support for SuperH on the side and shipped products that
> would be broken by this change :-)
Right. Originally I had 'or "renesas,sh-msiof" as fallback', but the fallbacks
have been dropped, so it indeed won't work with just "renesas,msiof-sh7724".
I'll fix that.
Thanks for your review!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings
From: Laurent Pinchart @ 2014-02-26 22:43 UTC (permalink / raw)
To: Suman Anna
Cc: Joerg Roedel, Tony Lindgren, Florian Vaussard, iommu, devicetree,
linux-omap, linux-arm-kernel, Mark Rutland, Kumar Gala,
Rob Herring
In-Reply-To: <530E6A78.9060503@ti.com>
Hi Suman,
On Wednesday 26 February 2014 16:28:08 Suman Anna wrote:
> On 02/26/2014 04:18 PM, Suman Anna wrote:
> > On 02/26/2014 02:36 PM, Laurent Pinchart wrote:
> >> On Wednesday 26 February 2014 14:23:03 Suman Anna wrote:
> >>>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
> >>>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
> >>>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
> >>>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
> >>>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
> >>>>>>>>> From: Florian Vaussard <florian.vaussard@epfl.ch>
> >>>>>>>>>
> >>>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
> >>>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a
> >>>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
> >>>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> >>>>>>>>> [s-anna@ti.com: split bindings document, add dra7 and bus error
> >>>>>>>>> back]
> >>>>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
> >>>>>>>>> ---
> >>>>>>>>>
> >>>>>>>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 +++++++++
> >>>>>>>>> 1 file changed, 28 insertions(+)
> >>>>>>>>> create mode 100644
> >>>>>>>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>>>>>
> >>>>>>>>> diff --git
> >>>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new
> >>>>>>>>> file
> >>>>>>>>> mode 100644
> >>>>>>>>> index 0000000..116492d
> >>>>>>>>> --- /dev/null
> >>>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>>>>> @@ -0,0 +1,28 @@
> >>>>>>>>> +OMAP2+ IOMMU
> >>>>>>>>> +
> >>>>>>>>> +Required properties:
> >>>>>>>>> +- compatible : Should be one of,
> >>>>>>>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
> >>>>>>>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
> >>>>>>>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances
> >>>>>>>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU
> >>>>>>>>> instance
> >>>>>>>>> +- reg : Address space for the configuration registers
> >>>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
> >>>>>>>>> +- dma-window : IOVA start address and length
> >>>>>>>>
> >>>>>>>> Isn't the dma window more of a system configuration property than a
> >>>>>>>> hardware property ? How do you expect it to be set?
> >>>>>>>
> >>>>>>> We are setting it based on the addressable range for the MMU.
> >>>>>>
> >>>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
> >>>>>> support the full 4GB VA space. Why do you need to restrict it ?
> >>>>>
> >>>>> I should have rephrased it better when I said addressable range. While
> >>>>> the MMUs are capable of programming the full 4GB space, there are some
> >>>>> address ranges that are private from the processor view. This
> >>>>> window is currently used to set the range for the omap-iovmm driver
> >>>>> (which only OMAP3 ISP is using atm), and there is no point in allowing
> >>>>> the omap-iovmm driver the full range when the processor could never
> >>>>> reach/access those addresses.
> >>>>
> >>>> But the IOMMU VA space is from a device point of view, not from a CPU
> >>>> point of view. Could you point me to where those private ranges are
> >>>> documented, in order to understand the problem correctly ?
> >>>
> >>> Yes, they are indeed from the device perspective. I meant DSP and/or IPU
> >>> by processor.
> >>>
> >>> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP
> >>> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external
> >>> addressable range starts from 0x11000000.
> >>
> >> OK, so it looks more like a property of the IOMMU master than a
> >> property of the IOMMU itself. It would be better to express it as such,
> >> but I wonder how that could be done, and if it would be worth it in this
> >> case.
> >
> > This property is currently solely used to configure the range for the
> > omap-iovmm module, which were supplied through platform data in the
> > non-DT case.
If I'm not mistaken omap-iovmm is something we want to get rid of. I know that
the OMAP3 ISP driver is the only user of that module, and I've started working
on fixing that, but I'm currently lacking time to complete the work.
Now, if we get rid of omap-iovmm, does that mean that the dma-window property
won't need to be specified anymore ? If so, given that the only omap-iovmm
user is the OMAP3 ISP driver, I would propose to drop the property and just
hardcode the value.
Please let me know if there's something I've missed.
> > I am wondering if the way to go here is to use iommu_domain_set_attr() and
> > use the domain geometry values.
>
> The other option is to supply these as driver match data, and switching
> the compatible strings to identify the MMU instance precisely.
>
> regards
> Suman
>
> >> As not all masters (the OMAP3 ISP doesn't for instance) have restrictions
> >> regarding the VA range they can address, should this property be at
> >> least made
> >> optional ?
> >>
> >>>>>>> We are reusing the existing defined property and it allows us to get
> >>>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP
> >>>>>>> iommu platform data.
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCHv1 5/6] HSI: Introduce OMAP SSI driver
From: Sebastian Reichel @ 2014-02-26 22:49 UTC (permalink / raw)
To: Mark Rutland
Cc: Linus Walleij, Shubhrajyoti Datta, Carlos Chinea, Tony Lindgren,
grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org, Pawel Moll,
Stephen Warren, Ian Campbell, Rob Landley,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Pali Rohár,
Ивайло Димитров,
Joni Lapilainen, Aaro Koskinen
In-Reply-To: <20140224155132.GK28555-NuALmloUBlrZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4841 bytes --]
Hi Mark,
On Mon, Feb 24, 2014 at 03:51:32PM +0000, Mark Rutland wrote:
> > + irq = platform_get_resource_byname(pd, IORESOURCE_IRQ, "gdd_mpu");
> > + if (!irq) {
> > + dev_err(&pd->dev, "GDD IRQ resource missing\n");
> > + err = -ENXIO;
> > + goto out_err;
> > + }
> > + omap_ssi->gdd_irq = irq->start;
>
> You can use platform_get_irq_byname here.
Right. Will be changed in PATCHv2.
> > +static inline int ssi_of_get_available_child_count(const struct device_node *np)
> > +{
> > + struct device_node *child;
> > + int num = 0;
> > +
> > + for_each_child_of_node(np, child)
> > + if (of_device_is_available(child))
> > + num++;
> > +
> > + return num;
> > +}
>
> You can find of_get_available_child_count in <linux/of.h>.
That did not exist when I started with the DT conversion of the
driver.
> That said, this seems to be trying to count the number of ports,
> which should all be compatible with "ti,omap3-ssi-port", no?
>
> So maybe you should count all available child nodes compatible with
> that.
I updated the function to check the compatible string
and use for_each_available_child_of_node(), which has
also been added after I wrote this function.
> > +static int __init ssi_probe(struct platform_device *pd)
> > +{
> > + struct device_node *np = pd->dev.of_node;
> > + struct hsi_controller *ssi;
> > + int err;
> > + int num_ports;
> > +
> > + if (!np) {
> > + dev_err(&pd->dev, "missing device tree data\n");
> > + return -EINVAL;
> > + }
> > +
> > + num_ports = ssi_of_get_available_child_count(np);
> > +
> > + ssi = hsi_alloc_controller(num_ports, GFP_KERNEL);
> > + if (!ssi) {
> > + dev_err(&pd->dev, "No memory for controller\n");
> > + return -ENOMEM;
> > + }
> > +
> > + platform_set_drvdata(pd, ssi);
> > +
> > + err = ssi_add_controller(ssi, pd);
> > + if (err < 0)
> > + goto out1;
> > +
> > + pm_runtime_irq_safe(&pd->dev);
> > + pm_runtime_enable(&pd->dev);
> > +
> > + err = ssi_hw_init(ssi);
> > + if (err < 0)
> > + goto out2;
> > +#ifdef CONFIG_DEBUG_FS
> > + err = ssi_debug_add_ctrl(ssi);
> > + if (err < 0)
> > + goto out2;
> > +#endif
> > +
> > + err = of_platform_populate(pd->dev.of_node, NULL, NULL, &pd->dev);
>
> I'm not keen on doing this because it allows arbitrary devices which are
> not ssi ports to be placed in the ssi host controller node that will be
> probed, which is nonsensical and something I'd like to avoid by
> construction.
>
> Is there any reason the ports have to be platform devices at all?
not strictly, but I get system resources via platform_get_resource_byname.
> If so, is there no way we can register them directly and skip any other
> devices?
I set the second parameter (matches) of the of_platform_populate()
call to a table containing the ssi-port compatible value.
> > +static int __exit ssi_remove(struct platform_device *pd)
> > +{
> > + struct hsi_controller *ssi = platform_get_drvdata(pd);
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > + ssi_debug_remove_ctrl(ssi);
> > +#endif
> > + ssi_remove_controller(ssi);
> > + platform_set_drvdata(pd, NULL);
> > +
> > + pm_runtime_disable(&pd->dev);
> > +
> > + /* cleanup of of_platform_populate() call */
> > + device_for_each_child(&pd->dev, NULL, ssi_remove_ports);
>
> This would certainly be broken for a non ssi port device.
I never intended to support other subdevices, but this only
unregisters each subdevice, so its probably safe...
> > +static int omap_ssi_port_runtime_suspend(struct device *dev)
> > +{
> > + struct hsi_port *port = dev_get_drvdata(dev);
> > + struct omap_ssi_port *omap_port = hsi_port_drvdata(port);
> > + struct hsi_controller *ssi = to_hsi_controller(port->device.parent);
> > + struct omap_ssi_controller *omap_ssi = hsi_controller_drvdata(ssi);
> > +
> > + dev_dbg(dev, "port runtime suspend!\n");
> > +
> > + ssi_set_port_mode(omap_port, SSI_MODE_SLEEP);
> > + if (omap_ssi->get_loss)
> > + omap_port->loss_count =
> > + (*omap_ssi->get_loss)(ssi->device.parent);
>
> You don't need to do (*struct->func)(args) when invoking a function
> pointer. You can jsut have struct->func(args) as we do elsewhere. This
> can be:
>
> omap_ssi->get_loss(ssi->device.parent)
>
> This should be fixed up in the other sites too.
Thanks, fixed everywhere.
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings
From: Suman Anna @ 2014-02-26 23:14 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Tony Lindgren,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kumar Gala,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Florian Vaussard
In-Reply-To: <2109933.259zooPLHZ@avalon>
Hi Laurent,
>
> On Wednesday 26 February 2014 16:28:08 Suman Anna wrote:
>> On 02/26/2014 04:18 PM, Suman Anna wrote:
>>> On 02/26/2014 02:36 PM, Laurent Pinchart wrote:
>>>> On Wednesday 26 February 2014 14:23:03 Suman Anna wrote:
>>>>>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
>>>>>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
>>>>>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
>>>>>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
>>>>>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
>>>>>>>>>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>>>>>
>>>>>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
>>>>>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a
>>>>>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
>>>>>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>>>>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error
>>>>>>>>>>> back]
>>>>>>>>>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>>>>>>>>>>> ---
>>>>>>>>>>>
>>>>>>>>>>> .../devicetree/bindings/iommu/ti,omap-iommu.txt | 28 +++++++++
>>>>>>>>>>> 1 file changed, 28 insertions(+)
>>>>>>>>>>> create mode 100644
>>>>>>>>>>> Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>>>>
>>>>>>>>>>> diff --git
>>>>>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new
>>>>>>>>>>> file
>>>>>>>>>>> mode 100644
>>>>>>>>>>> index 0000000..116492d
>>>>>>>>>>> --- /dev/null
>>>>>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>>>> @@ -0,0 +1,28 @@
>>>>>>>>>>> +OMAP2+ IOMMU
>>>>>>>>>>> +
>>>>>>>>>>> +Required properties:
>>>>>>>>>>> +- compatible : Should be one of,
>>>>>>>>>>> + "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
>>>>>>>>>>> + "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
>>>>>>>>>>> + "ti,dra7-iommu" for DRA7xx IOMMU instances
>>>>>>>>>>> +- ti,hwmods : Name of the hwmod associated with the IOMMU
>>>>>>>>>>> instance
>>>>>>>>>>> +- reg : Address space for the configuration registers
>>>>>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
>>>>>>>>>>> +- dma-window : IOVA start address and length
>>>>>>>>>>
>>>>>>>>>> Isn't the dma window more of a system configuration property than a
>>>>>>>>>> hardware property ? How do you expect it to be set?
>>>>>>>>>
>>>>>>>>> We are setting it based on the addressable range for the MMU.
>>>>>>>>
>>>>>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
>>>>>>>> support the full 4GB VA space. Why do you need to restrict it ?
>>>>>>>
>>>>>>> I should have rephrased it better when I said addressable range. While
>>>>>>> the MMUs are capable of programming the full 4GB space, there are some
>>>>>>> address ranges that are private from the processor view. This
>>>>>>> window is currently used to set the range for the omap-iovmm driver
>>>>>>> (which only OMAP3 ISP is using atm), and there is no point in allowing
>>>>>>> the omap-iovmm driver the full range when the processor could never
>>>>>>> reach/access those addresses.
>>>>>>
>>>>>> But the IOMMU VA space is from a device point of view, not from a CPU
>>>>>> point of view. Could you point me to where those private ranges are
>>>>>> documented, in order to understand the problem correctly ?
>>>>>
>>>>> Yes, they are indeed from the device perspective. I meant DSP and/or IPU
>>>>> by processor.
>>>>>
>>>>> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP
>>>>> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external
>>>>> addressable range starts from 0x11000000.
>>>>
>>>> OK, so it looks more like a property of the IOMMU master than a
>>>> property of the IOMMU itself. It would be better to express it as such,
>>>> but I wonder how that could be done, and if it would be worth it in this
>>>> case.
>>>
>>> This property is currently solely used to configure the range for the
>>> omap-iovmm module, which were supplied through platform data in the
>>> non-DT case.
>
> If I'm not mistaken omap-iovmm is something we want to get rid of. I know that
> the OMAP3 ISP driver is the only user of that module, and I've started working
> on fixing that, but I'm currently lacking time to complete the work.
>
> Now, if we get rid of omap-iovmm, does that mean that the dma-window property
> won't need to be specified anymore ? If so, given that the only omap-iovmm
> user is the OMAP3 ISP driver, I would propose to drop the property and just
> hardcode the value.
Yeah, none of our OMAP4+ stacks use omap-iovmm, or similar dynamic
reservation scheme at the moment. I am perfectly fine with dropping the
property and hard-coding it in the driver with a note. DSP/Bridge has a
similar usage (in dmm.c), but that code is localized within the driver.
Thanks for all the comments.
regards
Suman
>
> Please let me know if there's something I've missed.
>
>>> I am wondering if the way to go here is to use iommu_domain_set_attr() and
>>> use the domain geometry values.
>>
>> The other option is to supply these as driver match data, and switching
>> the compatible strings to identify the MMU instance precisely.
>>
>> regards
>> Suman
>>
>>>> As not all masters (the OMAP3 ISP doesn't for instance) have restrictions
>>>> regarding the VA range they can address, should this property be at
>>>> least made
>>>> optional ?
>>>>
>>>>>>>>> We are reusing the existing defined property and it allows us to get
>>>>>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP
>>>>>>>>> iommu platform data.
>
> [snip]
>
^ permalink raw reply
* Re: [PATCH] clk: hisilicon: add hi3620_mmc_clks
From: Mike Turquette @ 2014-02-27 0:10 UTC (permalink / raw)
To: Chris Ball, Arnd Bergmann, Jaehoon Chung, Seungwon Jeon,
Haojian Zhuang, brooke.wangzhigang
Cc: linux-mmc, linux-arm-kernel, patches, devicetree, Zhangfei Gao
In-Reply-To: <1389605852-10160-1-git-send-email-zhangfei.gao@linaro.org>
Quoting Zhangfei Gao (2014-01-13 01:37:32)
> Suggest by Arnd: abstract mmc tuning as clock behavior,
> also because different soc have different tuning method and registers.
> hi3620_mmc_clks is added to handle mmc clock specifically on hi3620.
>
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
Taken into clk-next (again! This one got lost when migrating my early
branch from 3.14-rcX to the new clk-next)
Regards,
Mike
> ---
> .../bindings/arm/hisilicon/hisilicon.txt | 14 +
> .../devicetree/bindings/clock/hi3620-clock.txt | 1 +
> drivers/clk/hisilicon/clk-hi3620.c | 274 ++++++++++++++++++++
> include/dt-bindings/clock/hi3620-clock.h | 5 +
> 4 files changed, 294 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> index 8c7a4653508d..df0a452b8526 100644
> --- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> @@ -30,3 +30,17 @@ Example:
> resume-offset = <0x308>;
> reboot-offset = <0x4>;
> };
> +
> +PCTRL: Peripheral misc control register
> +
> +Required Properties:
> +- compatible: "hisilicon,pctrl"
> +- reg: Address and size of pctrl.
> +
> +Example:
> +
> + /* for Hi3620 */
> + pctrl: pctrl@fca09000 {
> + compatible = "hisilicon,pctrl";
> + reg = <0xfca09000 0x1000>;
> + };
> diff --git a/Documentation/devicetree/bindings/clock/hi3620-clock.txt b/Documentation/devicetree/bindings/clock/hi3620-clock.txt
> index 4b71ab41be53..dad6269f52c5 100644
> --- a/Documentation/devicetree/bindings/clock/hi3620-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/hi3620-clock.txt
> @@ -7,6 +7,7 @@ Required Properties:
>
> - compatible: should be one of the following.
> - "hisilicon,hi3620-clock" - controller compatible with Hi3620 SoC.
> + - "hisilicon,hi3620-mmc-clock" - controller specific for Hi3620 mmc.
>
> - reg: physical base address of the controller and length of memory mapped
> region.
> diff --git a/drivers/clk/hisilicon/clk-hi3620.c b/drivers/clk/hisilicon/clk-hi3620.c
> index f24ad6a3a797..38faa469d288 100644
> --- a/drivers/clk/hisilicon/clk-hi3620.c
> +++ b/drivers/clk/hisilicon/clk-hi3620.c
> @@ -240,3 +240,277 @@ static void __init hi3620_clk_init(struct device_node *np)
> base);
> }
> CLK_OF_DECLARE(hi3620_clk, "hisilicon,hi3620-clock", hi3620_clk_init);
> +
> +struct hisi_mmc_clock {
> + unsigned int id;
> + const char *name;
> + const char *parent_name;
> + unsigned long flags;
> + u32 clken_reg;
> + u32 clken_bit;
> + u32 div_reg;
> + u32 div_off;
> + u32 div_bits;
> + u32 drv_reg;
> + u32 drv_off;
> + u32 drv_bits;
> + u32 sam_reg;
> + u32 sam_off;
> + u32 sam_bits;
> +};
> +
> +struct clk_mmc {
> + struct clk_hw hw;
> + u32 id;
> + void __iomem *clken_reg;
> + u32 clken_bit;
> + void __iomem *div_reg;
> + u32 div_off;
> + u32 div_bits;
> + void __iomem *drv_reg;
> + u32 drv_off;
> + u32 drv_bits;
> + void __iomem *sam_reg;
> + u32 sam_off;
> + u32 sam_bits;
> +};
> +
> +#define to_mmc(_hw) container_of(_hw, struct clk_mmc, hw)
> +
> +static struct hisi_mmc_clock hi3620_mmc_clks[] __initdata = {
> + { HI3620_SD_CIUCLK, "sd_bclk1", "sd_clk", CLK_SET_RATE_PARENT, 0x1f8, 0, 0x1f8, 1, 3, 0x1f8, 4, 4, 0x1f8, 8, 4},
> + { HI3620_MMC_CIUCLK1, "mmc_bclk1", "mmc_clk1", CLK_SET_RATE_PARENT, 0x1f8, 12, 0x1f8, 13, 3, 0x1f8, 16, 4, 0x1f8, 20, 4},
> + { HI3620_MMC_CIUCLK2, "mmc_bclk2", "mmc_clk2", CLK_SET_RATE_PARENT, 0x1f8, 24, 0x1f8, 25, 3, 0x1f8, 28, 4, 0x1fc, 0, 4},
> + { HI3620_MMC_CIUCLK3, "mmc_bclk3", "mmc_clk3", CLK_SET_RATE_PARENT, 0x1fc, 4, 0x1fc, 5, 3, 0x1fc, 8, 4, 0x1fc, 12, 4},
> +};
> +
> +static unsigned long mmc_clk_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + switch (parent_rate) {
> + case 26000000:
> + return 13000000;
> + case 180000000:
> + return 25000000;
> + case 360000000:
> + return 50000000;
> + case 720000000:
> + return 100000000;
> + case 1440000000:
> + return 180000000;
> + default:
> + return parent_rate;
> + }
> +}
> +
> +static long mmc_clk_determine_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *best_parent_rate,
> + struct clk **best_parent_p)
> +{
> + struct clk_mmc *mclk = to_mmc(hw);
> + unsigned long best = 0;
> +
> + if ((rate <= 13000000) && (mclk->id == HI3620_MMC_CIUCLK1)) {
> + rate = 13000000;
> + best = 26000000;
> + } else if (rate <= 26000000) {
> + rate = 25000000;
> + best = 180000000;
> + } else if (rate <= 52000000) {
> + rate = 50000000;
> + best = 360000000;
> + } else if (rate <= 100000000) {
> + rate = 100000000;
> + best = 720000000;
> + } else {
> + /* max is 180M */
> + rate = 180000000;
> + best = 1440000000;
> + }
> + *best_parent_rate = best;
> + return rate;
> +}
> +
> +static u32 mmc_clk_delay(u32 val, u32 para, u32 off, u32 len)
> +{
> + u32 i;
> +
> + if (para >= 0) {
> + for (i = 0; i < len; i++) {
> + if (para % 2)
> + val |= 1 << (off + i);
> + else
> + val &= ~(1 << (off + i));
> + para = para >> 1;
> + }
> + }
> + return val;
> +}
> +
> +static int mmc_clk_set_timing(struct clk_hw *hw, unsigned long rate)
> +{
> + struct clk_mmc *mclk = to_mmc(hw);
> + unsigned long flags;
> + u32 sam, drv, div, val;
> + static DEFINE_SPINLOCK(mmc_clk_lock);
> +
> + switch (rate) {
> + case 13000000:
> + sam = 3;
> + drv = 1;
> + div = 1;
> + break;
> + case 25000000:
> + sam = 13;
> + drv = 6;
> + div = 6;
> + break;
> + case 50000000:
> + sam = 3;
> + drv = 6;
> + div = 6;
> + break;
> + case 100000000:
> + sam = 6;
> + drv = 4;
> + div = 6;
> + break;
> + case 180000000:
> + sam = 6;
> + drv = 4;
> + div = 7;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + spin_lock_irqsave(&mmc_clk_lock, flags);
> +
> + val = readl_relaxed(mclk->clken_reg);
> + val &= ~(1 << mclk->clken_bit);
> + writel_relaxed(val, mclk->clken_reg);
> +
> + val = readl_relaxed(mclk->sam_reg);
> + val = mmc_clk_delay(val, sam, mclk->sam_off, mclk->sam_bits);
> + writel_relaxed(val, mclk->sam_reg);
> +
> + val = readl_relaxed(mclk->drv_reg);
> + val = mmc_clk_delay(val, drv, mclk->drv_off, mclk->drv_bits);
> + writel_relaxed(val, mclk->drv_reg);
> +
> + val = readl_relaxed(mclk->div_reg);
> + val = mmc_clk_delay(val, div, mclk->div_off, mclk->div_bits);
> + writel_relaxed(val, mclk->div_reg);
> +
> + val = readl_relaxed(mclk->clken_reg);
> + val |= 1 << mclk->clken_bit;
> + writel_relaxed(val, mclk->clken_reg);
> +
> + spin_unlock_irqrestore(&mmc_clk_lock, flags);
> +
> + return 0;
> +}
> +
> +static int mmc_clk_prepare(struct clk_hw *hw)
> +{
> + struct clk_mmc *mclk = to_mmc(hw);
> + unsigned long rate;
> +
> + if (mclk->id == HI3620_MMC_CIUCLK1)
> + rate = 13000000;
> + else
> + rate = 25000000;
> +
> + return mmc_clk_set_timing(hw, rate);
> +}
> +
> +static int mmc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + return mmc_clk_set_timing(hw, rate);
> +}
> +
> +static struct clk_ops clk_mmc_ops = {
> + .prepare = mmc_clk_prepare,
> + .determine_rate = mmc_clk_determine_rate,
> + .set_rate = mmc_clk_set_rate,
> + .recalc_rate = mmc_clk_recalc_rate,
> +};
> +
> +static struct clk *hisi_register_clk_mmc(struct hisi_mmc_clock *mmc_clk,
> + void __iomem *base, struct device_node *np)
> +{
> + struct clk_mmc *mclk;
> + struct clk *clk;
> + struct clk_init_data init;
> +
> + mclk = kzalloc(sizeof(*mclk), GFP_KERNEL);
> + if (!mclk) {
> + pr_err("%s: fail to allocate mmc clk\n", __func__);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + init.name = mmc_clk->name;
> + init.ops = &clk_mmc_ops;
> + init.flags = mmc_clk->flags | CLK_IS_BASIC;
> + init.parent_names = (mmc_clk->parent_name ? &mmc_clk->parent_name : NULL);
> + init.num_parents = (mmc_clk->parent_name ? 1 : 0);
> + mclk->hw.init = &init;
> +
> + mclk->id = mmc_clk->id;
> + mclk->clken_reg = base + mmc_clk->clken_reg;
> + mclk->clken_bit = mmc_clk->clken_bit;
> + mclk->div_reg = base + mmc_clk->div_reg;
> + mclk->div_off = mmc_clk->div_off;
> + mclk->div_bits = mmc_clk->div_bits;
> + mclk->drv_reg = base + mmc_clk->drv_reg;
> + mclk->drv_off = mmc_clk->drv_off;
> + mclk->drv_bits = mmc_clk->drv_bits;
> + mclk->sam_reg = base + mmc_clk->sam_reg;
> + mclk->sam_off = mmc_clk->sam_off;
> + mclk->sam_bits = mmc_clk->sam_bits;
> +
> + clk = clk_register(NULL, &mclk->hw);
> + if (WARN_ON(IS_ERR(clk)))
> + kfree(mclk);
> + return clk;
> +}
> +
> +static void __init hi3620_mmc_clk_init(struct device_node *node)
> +{
> + void __iomem *base;
> + int i, num = ARRAY_SIZE(hi3620_mmc_clks);
> + struct clk_onecell_data *clk_data;
> +
> + if (!node) {
> + pr_err("failed to find pctrl node in DTS\n");
> + return;
> + }
> +
> + base = of_iomap(node, 0);
> + if (!base) {
> + pr_err("failed to map pctrl\n");
> + return;
> + }
> +
> + clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
> + if (WARN_ON(!clk_data))
> + return;
> +
> + clk_data->clks = kzalloc(sizeof(struct clk *) * num, GFP_KERNEL);
> + if (!clk_data->clks) {
> + pr_err("%s: fail to allocate mmc clk\n", __func__);
> + return;
> + }
> +
> + for (i = 0; i < num; i++) {
> + struct hisi_mmc_clock *mmc_clk = &hi3620_mmc_clks[i];
> + clk_data->clks[mmc_clk->id] =
> + hisi_register_clk_mmc(mmc_clk, base, node);
> + }
> +
> + clk_data->clk_num = num;
> + of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> +}
> +
> +CLK_OF_DECLARE(hi3620_mmc_clk, "hisilicon,hi3620-mmc-clock", hi3620_mmc_clk_init);
> diff --git a/include/dt-bindings/clock/hi3620-clock.h b/include/dt-bindings/clock/hi3620-clock.h
> index 6eaa6a45e110..21b9d0e2eb0c 100644
> --- a/include/dt-bindings/clock/hi3620-clock.h
> +++ b/include/dt-bindings/clock/hi3620-clock.h
> @@ -147,6 +147,11 @@
> #define HI3620_MMC_CLK3 217
> #define HI3620_MCU_CLK 218
>
> +#define HI3620_SD_CIUCLK 0
> +#define HI3620_MMC_CIUCLK1 1
> +#define HI3620_MMC_CIUCLK2 2
> +#define HI3620_MMC_CIUCLK3 3
> +
> #define HI3620_NR_CLKS 219
>
> #endif /* __DTS_HI3620_CLOCK_H */
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCH] phy-rcar-gen2-usb: add device tree support
From: Sergei Shtylyov @ 2014-02-27 0:12 UTC (permalink / raw)
To: balbi, linux-usb, robh+dt, pawel.moll, mark.rutland,
ijc+devicetree, galak, grant.likely, devicetree
Cc: gregkh, linux-sh, valentine.barshak, rob, linux-doc
Add support of the device tree probing for the Renesas R-Car generation 2 SoCs
documenting the device tree binding as necessary.
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
This patch is against the 'next' branch of Felipe Balbi's 'usb.git' repo.
Documentation/devicetree/bindings/usb/rcar-gen2-phy.txt | 29 +++++++
drivers/usb/phy/phy-rcar-gen2-usb.c | 64 ++++++++++++++--
2 files changed, 85 insertions(+), 8 deletions(-)
Index: usb/Documentation/devicetree/bindings/usb/rcar-gen2-phy.txt
===================================================================
--- /dev/null
+++ usb/Documentation/devicetree/bindings/usb/rcar-gen2-phy.txt
@@ -0,0 +1,29 @@
+* Renesas R-Car generation 2 USB PHY
+
+This file provides information on what the device node for the R-Car generation
+2 USB PHY contains.
+
+Required properties:
+- compatible: "renesas,usb-phy-r8a7790" if the device is a part of R8A7790 SoC.
+ "renesas,usb-phy-r8a7791" if the device is a part of R8A7791 SoC.
+- reg: offset and length of the register block.
+- clocks: clock phandle and specifier pair.
+- clock-names: string, clock input name, must be "usbhs".
+
+Optional properties:
+- renesas,channel0-pci: boolean, specify when USB channel 0 should be connected
+ to PCI EHCI/OHCI; otherwise, it will be connected to the
+ USBHS controller.
+- renesas,channel2-pci: boolean, specify when USB channel 2 should be connected
+ to PCI EHCI/OHCI; otherwise, it will be connected to the
+ USBSS controller (xHCI).
+
+Example (Lager board):
+
+ usb-phy@e6590100 {
+ compatible = "renesas,usb-phy-r8a7790";
+ reg = <0 0xe6590100 0 0x100>;
+ clocks = <&mstp7_clks R8A7790_CLK_HSUSB>;
+ clock-names = "usbhs";
+ renesas,channel2-pci;
+ };
Index: usb/drivers/usb/phy/phy-rcar-gen2-usb.c
===================================================================
--- usb.orig/drivers/usb/phy/phy-rcar-gen2-usb.c
+++ usb/drivers/usb/phy/phy-rcar-gen2-usb.c
@@ -1,8 +1,8 @@
/*
* Renesas R-Car Gen2 USB phy driver
*
- * Copyright (C) 2013 Renesas Solutions Corp.
- * Copyright (C) 2013 Cogent Embedded, Inc.
+ * Copyright (C) 2013-2014 Renesas Solutions Corp.
+ * Copyright (C) 2013-2014 Cogent Embedded, Inc.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -13,6 +13,7 @@
#include <linux/delay.h>
#include <linux/io.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/platform_data/usb-rcar-gen2-phy.h>
#include <linux/platform_device.h>
#include <linux/spinlock.h>
@@ -167,9 +168,41 @@ out:
spin_unlock_irqrestore(&priv->lock, flags);
}
+#ifdef CONFIG_OF
+static struct rcar_gen2_phy_platform_data *
+rcar_gen2_usb_phy_parse_dt(struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ struct rcar_gen2_phy_platform_data *pdata;
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return NULL;
+
+ pdata->chan0_pci = of_property_read_bool(np, "renesas,channel0-pci");
+ pdata->chan2_pci = of_property_read_bool(np, "renesas,channel2-pci");
+
+ return pdata;
+}
+
+static const struct of_device_id rcar_gen2_usb_phy_match_table[] = {
+ { .compatible = "renesas,usb-phy-r8a7790" },
+ { .compatible = "renesas,usb-phy-r8a7791" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, rcar_gen2_usb_phy_match_table);
+#else
+static inline struct rcar_gen2_phy_platform_data *
+rcar_gen2_usb_phy_parse_dt(struct device *dev)
+{
+ return NULL;
+}
+#endif
+
static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
struct rcar_gen2_phy_platform_data *pdata;
struct rcar_gen2_usb_phy_priv *priv;
struct resource *res;
@@ -177,13 +210,19 @@ static int rcar_gen2_usb_phy_probe(struc
struct clk *clk;
int retval;
- pdata = dev_get_platdata(dev);
+ if (np)
+ pdata = rcar_gen2_usb_phy_parse_dt(dev);
+ else
+ pdata = dev_get_platdata(dev);
if (!pdata) {
dev_err(dev, "No platform data\n");
return -EINVAL;
}
- clk = devm_clk_get(dev, "usbhs");
+ if (np)
+ clk = of_clk_get_by_name(np, "usbhs");
+ else
+ clk = clk_get(dev, "usbhs");
if (IS_ERR(clk)) {
dev_err(dev, "Can't get the clock\n");
return PTR_ERR(clk);
@@ -191,13 +230,16 @@ static int rcar_gen2_usb_phy_probe(struc
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ if (IS_ERR(base)) {
+ retval = PTR_ERR(base);
+ goto error;
+ }
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv) {
dev_err(dev, "Memory allocation failed\n");
- return -ENOMEM;
+ retval = -ENOMEM;
+ goto error;
}
spin_lock_init(&priv->lock);
@@ -216,12 +258,16 @@ static int rcar_gen2_usb_phy_probe(struc
retval = usb_add_phy_dev(&priv->phy);
if (retval < 0) {
dev_err(dev, "Failed to add USB phy\n");
- return retval;
+ goto error;
}
platform_set_drvdata(pdev, priv);
return retval;
+
+error:
+ clk_put(clk);
+ return retval;
}
static int rcar_gen2_usb_phy_remove(struct platform_device *pdev)
@@ -229,6 +275,7 @@ static int rcar_gen2_usb_phy_remove(stru
struct rcar_gen2_usb_phy_priv *priv = platform_get_drvdata(pdev);
usb_remove_phy(&priv->phy);
+ clk_put(priv->clk);
return 0;
}
@@ -236,6 +283,7 @@ static int rcar_gen2_usb_phy_remove(stru
static struct platform_driver rcar_gen2_usb_phy_driver = {
.driver = {
.name = "usb_phy_rcar_gen2",
+ .of_match_table = of_match_ptr(rcar_gen2_usb_phy_match_table),
},
.probe = rcar_gen2_usb_phy_probe,
.remove = rcar_gen2_usb_phy_remove,
^ permalink raw reply
* Re: [PATCHv1 3/6] HSI: hsi-char: add Device Tree support
From: Sebastian Reichel @ 2014-02-27 0:24 UTC (permalink / raw)
To: Mark Rutland
Cc: Linus Walleij, Shubhrajyoti Datta, Carlos Chinea, Tony Lindgren,
grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org, Pawel Moll,
Stephen Warren, Ian Campbell, Rob Landley,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Pali Rohár,
Ивайло Димитров,
Joni Lapilainen, Aaro Koskinen
In-Reply-To: <20140224151301.GJ28555-NuALmloUBlrZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1636 bytes --]
On Mon, Feb 24, 2014 at 03:13:01PM +0000, Mark Rutland wrote:
> On Sun, Feb 23, 2014 at 11:49:58PM +0000, Sebastian Reichel wrote:
> > Add of_match_table to hsi_char driver, so that it can
> > be referenced from Device Tree.
> >
> > Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
> > ---
> > drivers/hsi/clients/hsi_char.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/hsi/clients/hsi_char.c b/drivers/hsi/clients/hsi_char.c
> > index e61e5f9..7f64bed 100644
> > --- a/drivers/hsi/clients/hsi_char.c
> > +++ b/drivers/hsi/clients/hsi_char.c
> > @@ -42,6 +42,7 @@
> > #include <linux/stat.h>
> > #include <linux/hsi/hsi.h>
> > #include <linux/hsi/hsi_char.h>
> > +#include <linux/of_device.h>
> >
> > #define HSC_DEVS 16 /* Num of channels */
> > #define HSC_MSGS 4
> > @@ -758,12 +759,22 @@ static int hsc_remove(struct device *dev)
> > return 0;
> > }
> >
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id hsi_char_of_match[] = {
> > + { .compatible = "ssi-char", },
>
> This string is undocumented.
>
> > + { .compatible = "hsi-char", },
>
> I'm not sure either string makes sense though; this feels like a binding
> for the sake of the driver rather than describing the device and
> allowing the driver to pick it up if it makes sense to do so.
>
> What exactly is a "ssi-char" device or a "hsi-char" device?
mh. I guess I will update the hsi framework, so that it simply loads
the client driver for each registered port. This is indeed only a
binding to load the driver.
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v7 5/9] ARM: Enable erratum 798181 for Broadcom Brahma-B15
From: Gregory Fong @ 2014-02-27 0:38 UTC (permalink / raw)
To: Will Deacon
Cc: Marc Carino, Christian Daudt, Arnd Bergmann, Olof Johansson,
Florian Fainelli, Matt Porter, Russell King,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org
In-Reply-To: <20140226104702.GA8961-MRww78TxoiP5vMa5CHWGZ34zcgK1vI+I0E9HWUfgJXw@public.gmane.org>
Hi Will,
On Wed, Feb 26, 2014 at 2:47 AM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Wed, Feb 26, 2014 at 10:29:08AM +0000, Marc Carino wrote:
>> From: Gregory Fong <gregory.0xf0-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> Broadcom Brahma-B15 (r0p0..r0p2) is also affected by Cortex-A15
>> erratum 798181, so enable the workaround for Brahma-B15.
>
> Really... *exactly* the same erratum? That sounds pretty unlikely, so I'd
> really like to be sure that the workaround we have indeed solves your
> problem (issuing a dummy TLBI to 0x0 + dsb, then followed by a dmb + clrex
> on each core). The point is that the workaround doesn't simply perform
> non-shareable invalidation on each core using IPIs.
Yes, it should be the same. I've sent you more detail off-list.
Regards,
Gregory
--
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: [PATCHv3 1/3] mmc: dw_mmc-socfpga: Remove the SOCFPGA specific platform for dw_mmc
From: Chris Ball @ 2014-02-27 2:32 UTC (permalink / raw)
To: Dinh Nguyen
Cc: Seungwon Jeon, dinguyen, linux-mmc, devicetree,
'Rob Herring', 'Pawel Moll',
'Mark Rutland', 'Ian Campbell',
'Kumar Gala', 'Jaehoon Chung'
In-Reply-To: <530CCBBB.9010503@gmail.com>
Hi,
On Tue, Feb 25 2014, Dinh Nguyen wrote:
>>> From: Dinh Nguyen <dinguyen@altera.com>
>>>
>>> It turns now that the only really platform specific code that is needed for
>>> SOCFPGA is using the SDMMC_CMD_USE_HOLD_REG in the prepare_command function.
>>> Since the Rockchip already has this functionality, re-use the code that is
>>> already in dw_mmc-pltfm.c.
>>
>> Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
>
> If you're ok with this patch series, can you please apply them for 3.15?
Thanks, pushed to mmc-next for 3.15.
- Chris.
--
Chris Ball <chris@printf.net> <http://printf.net/>
^ permalink raw reply
* Re: [RFC PATCH 1/6] PM / Voltagedomain: Add generic clk notifier handler for regulator based dynamic voltage scaling
From: Nishanth Menon @ 2014-02-27 2:34 UTC (permalink / raw)
To: Mike Turquette, Rafael J. Wysocki, Viresh Kumar, MyungJoo Ham,
Mark Brown
Cc: devicetree, linux-pm, linux-doc, linux-kernel, cpufreq,
linux-omap, linux-arm-kernel
In-Reply-To: <530D0394.4020208@ti.com>
On 14:56-20140225, Nishanth Menon wrote:
> On 02/24/2014 11:51 PM, Mike Turquette wrote:
> > Quoting Nishanth Menon (2014-02-18 12:32:18)
[...]
> > I'm not sure about trying to capture the "voltdm" as a core concept. It
> > feels a bit unwieldy to me.
>
> Considering it is a simple collation of regulators and SoC specific
> "magic" which have to be operated in tandem to clock operation, Why
> does it seem unwieldy? Usage of multiple voltage planes in a single
> voltage domain concept does not seem unique to TI processors either:
> For example, imx6q-cpufreq.c uses 3 regulators (arm, pu, soc),
> s5pv210-cpufreq.c uses two regulators (vddarm, vddint), ideally OMAP
> implementation would use two (vdd_mpu, vbb_mpu).
>
> > I have wondered about making an abstract
> > "performance domain" which is the dvfs analogue to generic power
> > domains. This a reasonable split since gpd are good for idle power
> > savings (e.g. clock gate, power gate, sleep state, etc) and "perf
> > domains" would be good for active power savings (dvfs).
> >
> > Having a generic container for performance domains might make a good
> > place to stuff all of this glue logic that we keep running into (e.g.
> > CPU and GPU max frequencies that are related), and it might make another
> > nice knob for the thermal folks to use.
>
> This sounds like one level higher abstraction that we are speaking of
> here? I was'nt intending to solve the bigger picture problem here -
> just an abstraction level that might allow reusablity for multiple
> SoCs. In fact, having an abstraction away for voltage domain(which may
> consist of multiple regulators and any SoC specific magic) purely
> allows us to move towards a direction you mention here.
>
> >
> > For the case of the OMAP voltage domains, it would be a place to stuff
> > all of the VC/VP -> ABB -> Smart Reflex AVS stuff.
> >
>
> Unfortunately, I dont completely comprehend objection we have to this
> approach (other than an higher level abstraction is needed) and if we
> do have an objection, what is the alternate approach should be for
> representing hardware which this series attempts to present.
I think the following is around the lines of your thought direction -
if Rafael or others have comments on the following approach, it'd be a
good starting point for me to progress.
-->8--
>From 62e50b9f920495db88e5594aa6bceb52e83a443d Mon Sep 17 00:00:00 2001
From: Nishanth Menon <nm@ti.com>
Date: Wed, 26 Feb 2014 10:59:59 -0600
Subject: [PATCH] PM / Runtime: introduce active power management callbacks
for pm_domain
dev_pm_domain currently handles just device idle power management
using the generic pm_runtime_get|put and related family of functions.
Logically with appropriate pm_domain hooks this can translate to
hardware specific clock and related operations. Given that pm_domains
may contain this information, this provides an opportunity to extend
current pm_runtime do dynamic power operations as well.
What this means for drivers is as follows:
Today, drivers(with some level of complexity) do:
pm_runtime_get_sync(dev);
clk = clk_get(dev, "name");
old_rate = clk_get_rate(clk);
...
clk_set_rate(clk, new_rate);
...
clk_put(clk);
pm_runtime_get_sync(dev);
Instead, on pm_domains that can handle this as part of
pm_domain->active_ops functions, They can now do the following:
pm_runtime_get_sync(dev);
old_rate = pm_runtime_get_rate(dev);
...
pm_runtime_set_rate(dev, new_rate);
...
pm_runtime_put_sync(dev);
Obviously, this'd work for devices that handle a single main
functional clock, but this could reduce complexity of drivers having
to deal with power management details to have pm_runtime as the main
point of interface.
CAVEAT: For power domains that are capable of handling multiple
clocks (example on OMAP, where there are the concepts of interface,
functional and optional clocks per block), appropriate handling will
be necessary from pm_domain callbacks. So, the question about which
clock rate is being controlled or returned to is entirely upto the
pm_domain implementation.
On the otherhand, we can debate about defining and querying ACPI style
"Performance state" instead of frequencies and wrap P-states inside
or the other way around.. but given that majority of drivers using
pm_runtime would rather be interested in frequencies and my naieve
belief that we can index P-states with frequencies, kind of influenced
my choice here of proposing frequencies as base query parameter..
ofcourse, debate is still open here.
Yes, we can still debate if providing yet another wrapper on top of
clock APIs makes sense at all as well.
Nyet-signed-off-by: Nishanth Menon <nm@ti.com>
---
drivers/base/power/runtime.c | 101 ++++++++++++++++++++++++++++++++++++++++++
include/linux/pm.h | 25 +++++++++--
include/linux/pm_runtime.h | 21 +++++++++
3 files changed, 143 insertions(+), 4 deletions(-)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 72e00e6..ef230b4 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1401,3 +1401,104 @@ void pm_runtime_remove(struct device *dev)
if (dev->power.irq_safe && dev->parent)
pm_runtime_put(dev->parent);
}
+
+/**
+ * pm_runtime_get_rate() - Returns the device operational frequency
+ * @dev: Device to handle
+ * @rate: Returns rate in Hz.
+ *
+ * Returns appropriate error value in case of error conditions, else
+ * returns 0 and rate is updated. The pm_domain logic does all the necessary
+ * operation (which may consider magic hardware stuff) to provide the rate.
+ *
+ * NOTE: the rate returned is a snapshot and in many cases just a bypass
+ * to clk api to set the rate.
+ */
+int pm_runtime_get_rate(struct device *dev, unsigned long *rate)
+{
+ unsigned long flags;
+ int error = -ENOSYS;
+
+ if (!rate || !dev)
+ return -EINVAL;
+
+ spin_lock_irqsave(&dev->power.lock, flags);
+ if (!pm_runtime_active(dev)) {
+ error = -EINVAL;
+ goto out;
+ }
+
+ if (dev->pm_domain && dev->pm_domain->active_ops.get_rate)
+ error = dev->pm_domain->active_ops.get_rate(dev, rate);
+out:
+ spin_unlock_irqrestore(&dev->power.lock, flags);
+
+ return error;
+}
+
+/**
+ * pm_runtime_set_rate() - Set a specific rate for the device operation
+ * @dev: Device to handle
+ * @rate: Rate to set in Hz
+ *
+ * Returns appropriate error value in case of error conditions, else
+ * returns 0. The pm_domain logic does all the necessary operation (which
+ * may include voltage scale operations or other magic hardware stuff) to
+ * achieve the operation. It is guarenteed that the requested rate is achieved
+ * on returning from this function if return value is 0.
+ */
+int pm_runtime_set_rate(struct device *dev, unsigned long rate)
+{
+ unsigned long flags;
+ int error = -ENOSYS;
+
+ if (!rate || !dev)
+ return -EINVAL;
+
+ spin_lock_irqsave(&dev->power.lock, flags);
+ if (!pm_runtime_active(dev)) {
+ error = -EINVAL;
+ goto out;
+ }
+
+ if (dev->pm_domain && dev->pm_domain->active_ops.set_rate)
+ error = dev->pm_domain->active_ops.set_rate(dev, rate);
+out:
+ spin_unlock_irqrestore(&dev->power.lock, flags);
+
+ return error;
+}
+
+/**
+ * pm_runtime_get_transition_latency() - determine transition latency`
+ * @dev: Device to handle
+ * @from_rate: Transition from which rate
+ * @to_rate: Transition to which rate
+ *
+ * Returns appropriate error value in case of error conditions, else
+ * returns the latency in uSecs for transition between two given rates
+ */
+int pm_runtime_get_transition_latency(struct device *dev,
+ unsigned long from_rate,
+ unsigned long to_rate)
+{
+ unsigned long flags;
+ int error = -ENOSYS;
+
+ if (!from_rate || !to_rate || !dev)
+ return -EINVAL;
+
+ spin_lock_irqsave(&dev->power.lock, flags);
+ if (!pm_runtime_active(dev)) {
+ error = -EINVAL;
+ goto out;
+ }
+
+ if (dev->pm_domain && dev->pm_domain->active_ops.get_transition_latency)
+ error = dev->pm_domain->active_ops.get_transition_latency(dev,
+ from_rate, to_rate);
+out:
+ spin_unlock_irqrestore(&dev->power.lock, flags);
+
+ return error;
+}
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 8c6583a..f907b27 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -299,6 +299,20 @@ struct dev_pm_ops {
int (*runtime_idle)(struct device *dev);
};
+/**
+ * struct dev_pm_active_ops - Active power management operations
+ * @get_rate: get the current operational frequency
+ * @set_rate: set the current operational frequency
+ * @get_transition_latency: get the transition latency in uSeconds
+ */
+struct dev_pm_active_ops {
+ int (*get_rate)(struct device *dev, unsigned long *rate);
+ int (*set_rate)(struct device *dev, unsigned long rate);
+ int (*get_transition_latency)(struct device *dev,
+ unsigned long from_rate,
+ unsigned long to_rate);
+};
+
#ifdef CONFIG_PM_SLEEP
#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
.suspend = suspend_fn, \
@@ -589,13 +603,16 @@ extern void update_pm_runtime_accounting(struct device *dev);
extern int dev_pm_get_subsys_data(struct device *dev);
extern int dev_pm_put_subsys_data(struct device *dev);
-/*
- * Power domains provide callbacks that are executed during system suspend,
- * hibernation, system resume and during runtime PM transitions along with
- * subsystem-level and driver-level callbacks.
+/**
+ * struct dev_pm_domain - power domain information
+ * @ops: Power domains provide callbacks that are executed during system
+ * suspend, hibernation, system resume and during runtime PM transitions
+ * along with subsystem-level and driver-level callbacks.
+ * @active_ops: Active operational callbacks
*/
struct dev_pm_domain {
struct dev_pm_ops ops;
+ struct dev_pm_active_ops active_ops;
};
/*
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 16c9a62..731a6e4 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -112,6 +112,11 @@ static inline void pm_runtime_mark_last_busy(struct device *dev)
ACCESS_ONCE(dev->power.last_busy) = jiffies;
}
+extern int pm_runtime_get_rate(struct device *dev, unsigned long *rate);
+extern int pm_runtime_set_rate(struct device *dev, unsigned long rate);
+extern int pm_runtime_get_transition_latency(struct device *dev,
+ unsigned long from_rate,
+ unsigned long to_rate);
#else /* !CONFIG_PM_RUNTIME */
static inline int __pm_runtime_idle(struct device *dev, int rpmflags)
@@ -162,6 +167,22 @@ static inline unsigned long pm_runtime_autosuspend_expiration(
static inline void pm_runtime_set_memalloc_noio(struct device *dev,
bool enable){}
+static inline int pm_runtime_get_rate(struct device *dev, unsigned long *rate)
+{
+ return -ENOSYS;
+}
+
+static inline int pm_runtime_set_rate(struct device *dev, unsigned long rate)
+{
+ return -ENOSYS;
+}
+
+static inline int pm_runtime_get_transition_latency(struct device *dev,
+ unsigned long from_rate,
+ unsigned long to_rate)
+{
+ return -ENOSYS;
+}
#endif /* !CONFIG_PM_RUNTIME */
static inline int pm_runtime_idle(struct device *dev)
--
1.7.9.5
--
Regards,
Nishanth Menon
^ permalink raw reply related
* Re: [RFC PATCH 1/6] PM / Voltagedomain: Add generic clk notifier handler for regulator based dynamic voltage scaling
From: Mike Turquette @ 2014-02-27 5:00 UTC (permalink / raw)
To: Nishanth Menon, Rafael J. Wysocki, Viresh Kumar, MyungJoo Ham,
Mark Brown
Cc: devicetree, linux-doc, linux-kernel, cpufreq, linux-pm,
linux-arm-kernel, linux-omap
In-Reply-To: <20140227023455.GA15712@kahuna>
Quoting Nishanth Menon (2014-02-26 18:34:55)
> +/**
> + * pm_runtime_get_rate() - Returns the device operational frequency
> + * @dev: Device to handle
> + * @rate: Returns rate in Hz.
> + *
> + * Returns appropriate error value in case of error conditions, else
> + * returns 0 and rate is updated. The pm_domain logic does all the necessary
> + * operation (which may consider magic hardware stuff) to provide the rate.
> + *
> + * NOTE: the rate returned is a snapshot and in many cases just a bypass
> + * to clk api to set the rate.
> + */
> +int pm_runtime_get_rate(struct device *dev, unsigned long *rate)
Instead of "rate", how about we use "level" and leave it undefined as to
what that means? It would be equally valid for level to represent a
clock rate, or an opp from a table of opp's, or a p-state, or some value
passed to a PM microcontroller.
Code that is tightly coupled to the hardware would simply know what
value to use with no extra sugar.
Generic code would need to get the various supported "levels" populated
at run time, but a DT binding could do that, or a query to the ACPI
tables, or whatever.
> +{
> + unsigned long flags;
> + int error = -ENOSYS;
> +
> + if (!rate || !dev)
> + return -EINVAL;
> +
> + spin_lock_irqsave(&dev->power.lock, flags);
> + if (!pm_runtime_active(dev)) {
> + error = -EINVAL;
> + goto out;
> + }
> +
> + if (dev->pm_domain && dev->pm_domain->active_ops.get_rate)
> + error = dev->pm_domain->active_ops.get_rate(dev, rate);
> +out:
> + spin_unlock_irqrestore(&dev->power.lock, flags);
> +
> + return error;
> +}
> +
> +/**
> + * pm_runtime_set_rate() - Set a specific rate for the device operation
> + * @dev: Device to handle
> + * @rate: Rate to set in Hz
> + *
> + * Returns appropriate error value in case of error conditions, else
> + * returns 0. The pm_domain logic does all the necessary operation (which
> + * may include voltage scale operations or other magic hardware stuff) to
> + * achieve the operation. It is guarenteed that the requested rate is achieved
> + * on returning from this function if return value is 0.
> + */
> +int pm_runtime_set_rate(struct device *dev, unsigned long rate)
Additionally I wonder if the function signature should include a way to
specify the sub-unit of a device that we are operating on? This is a way
to tackle the issues you raised regarding multiple clocks per device,
etc. Two approaches come to mind:
int pm_runtime_set_rate(struct device *dev, int index,
unsigned long rate);
Where index is a sub-unit of struct device *dev. The second approach is
to create a publicly declared structure representing the sub-unit. Some
variations on that theme:
int pm_runtime_set_rate(struct perf_domain *perfdm, unsigned long rate);
or,
int pm_runtime_set_rate(struct generic_power_domain *gpd,
unsigned long rate);
or whatever that sub-unit looks like. The gpd thing might be a total
layering violation, I don't know. Or perhaps it's a decent idea but it
shouldn't be as a PM runtime call. Again, I dunno.
Regards,
Mike
^ permalink raw reply
* Re: [PATCH RESEND v10 3/4] PHY: add APM X-Gene SoC 15Gbps Multi-purpose PHY driver
From: Kishon Vijay Abraham I @ 2014-02-27 6:02 UTC (permalink / raw)
To: Loc Ho
Cc: devicetree@vger.kernel.org, Suman Tripathi, Arnd Bergmann,
Jon Masters, patches@apm.com, linux-kernel, Olof Johansson,
Don Dutile, Tejun Heo, Tuan Phan,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAPw-ZT=eLbxt3wPL7hrZFtxTP9vGxbRyBY2Yoh77pq9ypphCaw@mail.gmail.com>
On Thursday 27 February 2014 02:15 AM, Loc Ho wrote:
> Hi,
>
>>>
>>> +config PHY_XGENE
>>> + tristate "APM X-Gene 15Gbps PHY support"
>>> + depends on ARM64 || COMPILE_TEST
>>> + select GENERIC_PHY
>>
>>
>> depends on HAS_IOMEM and CONFIG_OF..
>
> I will make it depends as "HAS_IOMEM && OF && (ARM64 || COMPILE_TEST)
>
>>>
>>> +/* PLL Clock Macro Unit (CMU) CSR accessing from SDS indirectly */
>>> +#define CMU_REG0 0x00000
>>> +#define CMU_REG0_PLL_REF_SEL_MASK 0x00002000
>>> +#define CMU_REG0_PLL_REF_SEL_SET(dst, src) \
>>> + (((dst) & ~0x00002000) | (((u32)(src) << 0xd) & 0x00002000))
>>
>>
>> using decimals for shift value would be better. No strong feeling though.
>
> I will change to integer value.
>
>>> +/*
>>> + * For chip earlier than A3 version, enable this flag.
>>> + * To enable, pass boot argument phy_xgene.preA3Chip=1
>>> + */
>>> +static int preA3Chip;
>>> +MODULE_PARM_DESC(preA3Chip, "Enable pre-A3 chip support (1=enable 0=disable)");
>>> +module_param_named(preA3Chip, preA3Chip, int, 0444);
>>
>>
>> Do we need to have module param for this? I mean we can differentiate between
>> different chip versions in dt data only.
>
> This is only required for the short term. Once all the pre-A3 system
> are replaced, there isn't an need for this. For those who still has an
> pre-A3 silicon system, this would provide an short term solution for
> them. DT isn't quite correct here. This is an global thing. I guess I
> can OR all node. If it is still better to put in the DT, let me know
> and I will move it.
>
>>> +
>>> +static void sds_wr(void __iomem *csr_base, u32 indirect_cmd_reg,
>>> + u32 indirect_data_reg, u32 addr, u32 data)
>>> +{
>>> + u32 val;
>>> + u32 cmd;
>>> +
>>> + cmd = CFG_IND_WR_CMD_MASK | CFG_IND_CMD_DONE_MASK;
>>> + cmd = CFG_IND_ADDR_SET(cmd, addr);
>>
>>
>> This looks hacky. If 'CFG_IND_WR_CMD_MASK | CFG_IND_CMD_DONE_MASK' should be set then it should be part of the second argument. From the macro 'CFG_IND_ADDR_SET' the first argument should be more like the current value present in the register right? I feel the macro (CFG_IND_ADDR_SET) is not used in the way it is intended to.
>
> The macro XXX_SET is intended to update an field within the register.
> The update field is returned. The first assignment lines are setting
> another field. Those two lines can be written as:
>
> cmd = 0;
> cmd |= CFG_IND_WR_CMD_MASK; ==> Set the CMD bit
> cmd |= CFG_IND_CMD_DONE_MASK; ==> Set the DONE bit
> cmd = CFG_IND_ADDR_SET(cmd, addr); ===> Set the field ADDR
#define CFG_IND_ADDR_SET(dst, src) \
(((dst) & ~0x003ffff0) | (((u32)(src)<<4) & 0x003ffff0))
From this macro the first argument should be the present value in that
register. Here you reset the address bits and write the new address bits.
IMO the first argument should be the value in 'csr_base +
indirect_cmd_reg'. So it resets the address bits in 'csr_base +
indirect_cmd_reg' and write down the new address bits.
Thanks
Kishon
^ 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