* Re: [PATCH v2 11/18] dt-bindings: pinctrl: Add bindings for Cirrus Logic Madera codecs
From: Linus Walleij @ 2017-04-25 9:35 UTC (permalink / raw)
To: Richard Fitzgerald
Cc: Alexandre Courbot, alsa-devel@alsa-project.org, Jason Cooper,
devicetree@vger.kernel.org,
open list:WOLFSON MICROELECTRONICS DRIVERS,
linux-kernel@vger.kernel.org, Rob Herring,
linux-gpio@vger.kernel.org, Mark Brown, Thomas Gleixner,
Lee Jones
In-Reply-To: <1493050124-5970-12-git-send-email-rf@opensource.wolfsonmicro.com>
On Mon, Apr 24, 2017 at 6:08 PM, Richard Fitzgerald
<rf@opensource.wolfsonmicro.com> wrote:
> This is the binding description of the pinctrl driver for Cirru Logic
> Madera codecs. The binding uses the generic pinctrl binding so the main
> purpose here is to describe the device-specific names for groups and
> functions.
>
> Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> ---
> Changes since V1:
> - split out from main pinctrl patch into a separate patch
> - fixed typo in pinctrl-names property description
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C
From: Benjamin Herrenschmidt @ 2017-04-25 9:34 UTC (permalink / raw)
To: Ryan Chen, Brendan Higgins
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Joel Stanley, Vladimir Zapolskiy,
Kachalov Anton, Cédric Le Goater, linux-i2c@vger.kernel.org,
devicetree@vger.kernel.org, Linux Kernel Mailing List,
OpenBMC Maillist
In-Reply-To: <7d991733d3ea4591b74baf93300f0224@TWMBX01.aspeed.com>
On Tue, 2017-04-25 at 08:50 +0000, Ryan Chen wrote:
> Hello All,
> ASPEED_I2CD_M_SDA_DRIVE_1T_EN,
> ASPEED_I2CD_SDA_DRIVE_1T_EN is specific for some case usage.
> For example, if i2c bus is use on "high speed" and
> "single slave and master" and i2c bus is too long. It need drive SDA
> or SCL less lunacy. It would enable it.
> Otherwise, don’t enable it. especially in multi-master.
> It can’t be enable.
That smells like a specific enough use case that we should probably
cover with a device-tree property, something like an empty
"sda-extra-drive" property (empty properties are typically used
for booleans, their presence means "true").
Thanks Ryan. Can you shed some light on the meaning of the high-speed
bit as well please ? Does it force to a specific speed (ignoring the
divisor) or we can still play with the clock high/low counts ?
Cheers,
Ben.
>
>
>
> Best Regards,
> Ryan
>
> 信驊科技股份有限公司
> ASPEED Technology Inc.
> 2F,No.15,Industry East Road 4.,Hsinchu Science Park, Hsinchu City
> 30077, Taiwan
> Tel: 886-3-578-9568 #857
> Fax: 886-3-578-9586
> ************* Email Confidentiality Notice ********************
> DISCLAIMER:
> This message (and any attachments) may contain legally privileged
> and/or other confidential information. If you have received it in
> error, please notify the sender by reply e-mail and immediately
> delete the e-mail and any attachments without copying or disclosing
> the contents. Thank you.
>
>
> -----Original Message-----
> From: Brendan Higgins [mailto:brendanhiggins@google.com]
> Sent: Tuesday, April 25, 2017 4:32 PM
> To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Wolfram Sang <wsa@the-dreams.de>; Rob Herring <robh+dt@kernel.org
> >; Mark Rutland <mark.rutland@arm.com>; Thomas Gleixner <tglx@linutro
> nix.de>; Jason Cooper <jason@lakedaemon.net>; Marc Zyngier <marc.zyng
> ier@arm.com>; Joel Stanley <joel@jms.id.au>; Vladimir Zapolskiy <vz@m
> leia.com>; Kachalov Anton <mouse@mayc.ru>; Cédric Le Goater <clg@kaod
> .org>; linux-i2c@vger.kernel.org; devicetree@vger.kernel.org; Linux
> Kernel Mailing List <linux-kernel@vger.kernel.org>; OpenBMC Maillist
> <openbmc@lists.ozlabs.org>; Ryan Chen <ryan_chen@aspeedtech.com>
> Subject: Re: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C
>
> Adding Ryan.
>
> On Mon, Apr 24, 2017 at 7:19 PM, Benjamin Herrenschmidt <benh@kernel.
> crashing.org> wrote:
> > On Mon, 2017-04-24 at 11:56 -0700, Brendan Higgins wrote:
> > > > > +struct aspeed_i2c_bus {
> > > > > + struct i2c_adapter adap;
> > > > > + struct device *dev;
> > > > > + void __iomem *base;
> > > > > + /* Synchronizes I/O mem access to base. */
> > > > > + spinlock_t lock;
> > > >
> > > > I am not entirely convinced we need that lock. The i2c core
> > > > will
> > > > take a mutex protecting all operations on the bus. So we only
> > > > need
> > > > to synchronize between our "xfer" code and our interrupt
> > > > handler.
> > >
> > > You are right if both having slave and master active at the same
> > > time
> > > was not possible; however, it is.
> >
> > Right, I somewhat forgot about the slave case.
> >
> > ...
> >
> > > > Some of those error states probably also warrant a reset of
> > > > the
> > > > controller, I think aspeed does that in the SDK.
> > >
> > > For timeout and cmd_err, I do not see any argument against it;
> > > it
> > > sounds like we are in a very messed up, very unknown state, so
> > > full
> > > reset is probably the best last resort.
> >
> > Yup.
> >
> > > For SDA staying pulled down, I
> > > think we can say with reasonable confidence that some device on
> > > our
> > > bus is behaving very badly and I am not convinced that resetting
> > > the
> > > controller is likely to do anything to help;
> >
> > Right. Hammering with STOPs and pray ...
>
> I think sending recovery mode sends stops as a part of the recovery
> algorithm it executes.
>
> >
> > > that being said, I really
> > > do not have any good ideas to address that. So maybe praying and
> > > resetting the controller is *the most reasonable thing to do.* I
> > > would like to know what you think we should do in that case.
> >
> > Well, there's a (small ?) chance that it's a controller bug
> > asserting
> > the line so ... but there's little we can do if not.
>
> True.
>
> >
> > > While I was thinking about this I also realized that the SDA
> > > line
> > > check after recovery happens in the else branch, but SCL line
> > > check
> > > does not happen after we attempt to STOP if SCL is hung. If we
> > > decide
> > > to make special note SDA being hung by a device that won't let
> > > go, we
> > > might want to make a special note that SCL is hung by a device
> > > that
> > > won't let go. Just a thought.
> >
> > Maybe. Or just "unrecoverable error"... hopefully these don't
> > happen
> > too often ... We had cases of a TPM misbehaving like that.
>
> Yeah, definitely should print something out.
>
> >
> > > > > +out:
> > >
> > > ...
> > > > What about I2C_M_NOSTART ?
> > > >
> > > > Not that I've ever seen it used... ;-)
> > >
> > > Right now I am not doing any of the protocol mangling options,
> > > but I
> > > can add them in if you think it is important for initial support.
> >
> > No, not important, we can add that later if it ever becomes useful.
> >
> > ...
> >
> > > > In general, you always ACK all interrupts first. Then you
> > > > handle
> > > > the bits you have harvested.
> > > >
> > >
> > > The documentation says to ACK the interrupt after handling in the
> > > RX
> > > case:
> > >
> > > <<<
> > > S/W needs to clear this status bit to allow next data receiving.
> > > > > >
> > >
> > > I will double check with Ryan to make sure TX works the same way.
> > >
> > > > > + if (irq_status & ASPEED_I2CD_INTR_ERROR ||
> > > > > + (!bus->msgs && bus->master_state !=
> > > > > ASPEED_I2C_MASTER_STOP)) {
> > >
> > > ...
> > > >
> > > > I would set master_state to "RECOVERY" (new state ?) and
> > > > ensure
> > > > those things are caught if they happen outside of a recovery.
> >
> > I replied privately ... as long as we ack before we start a new
> > command we should be ok but we shouldn't ack after.
> >
> > Your latest patch still does that. It will do things like start a
> > STOP
> > command *then* ack the status bits. I'm pretty sure that's bogus.
> >
> > That way it's a lot simpler to simply move the
> >
> > writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
> >
> > To either right after the readl of the status reg at the beginning
> > of
> > aspeed_i2c_master_irq().
> >
> > I would be very surprised if that didn't work properly and wasn't
> > much
> > safer than what you are currently doing.
>
> I think I tried your way and it worked. In anycase, Ryan will be able
> to clarify for us.
>
> >
> > > Let me know if you still think we need a "RECOVERY" state.
> >
> > The way you just switch to stop state and store the error for
> > later
> > should work I think.
> >
> > > >
> > > > > + if (bus->master_state == ASPEED_I2C_MASTER_START) {
> > >
> > > ...
> > > >
> > > > > + dev_dbg(bus->dev,
> > > > > + "no slave present at %02x",
> > > > > msg-
> > > > > > addr);
> > > > >
> > > > > + status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> > > > > + bus->cmd_err = -EIO;
> > > > > + do_stop(bus);
> > > > > + goto out_no_complete;
> > > > > + } else {
> > > > > + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> > > > > + if (msg->flags & I2C_M_RD)
> > > > > + bus->master_state =
> > > > > ASPEED_I2C_MASTER_RX;
> > > > > + else
> > > > > + bus->master_state =
> > > > > ASPEED_I2C_MASTER_TX_FIRST;
> > > >
> > > > What about the SMBUS_QUICK case ? (0-len transfer). Do we need
> > > > to
> > > > handle this here ? A quick look at the TX_FIRST case makes me
> > > > think
> > > > we are ok there but I'm not sure about the RX case.
> > >
> > > I did not think that there is an SMBUS_QUICK RX. Could you point
> > > me
> > > to an example?
> >
> > Not so much an RX, it's more like you are sending a 1-bit data in
> > the
> > place of the Rd/Wr bit. So you have a read with a lenght of 0, I
> > don't
> > think in that case you should set ASPEED_I2CD_M_RX_CMD in
> > __aspeed_i2c_do_start
>
> Forget what I said, I was just not thinking about the fact that SMBus
> emulation causes the data bit to be encoded as the R/W flag. I see
> what you are saying; you are correct.
>
> >
> > > > I'm not sure the RX case is tight also. What completion does
> > > > the HW
> > > > give you for the address cycle ? Won't you get that before it
> > > > has
> > > > received the first character ? IE. You fall through to the
> > > > read
> > > > case of the state machine with the read potentially not
> > > > complete
> > > > yet no ?
> > >
> > > ...
> > > > > + case ASPEED_I2C_MASTER_RX:
> > > > > + if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
> > > > > + dev_err(bus->dev, "master failed to
> > > > > RX");
> > > > > + goto out_complete;
> > > > > + }
> > > >
> > > > See my comment above for a bog standard i2c_read. Aren't you
> > > > getting the completion for the address before the read is even
> > > > started ?
> > >
> > > In practice no, but it is probably best to be safe :-)
> >
> > Yup :)
> > > >
> > > > > + status_ack |= ASPEED_I2CD_INTR_RX_DONE;
> > > > > +
> > > > > + recv_byte = aspeed_i2c_read(bus,
> > > > > ASPEED_I2C_BYTE_BUF_REG) >> 8;
> > > > > + msg->buf[bus->buf_index++] = recv_byte;
> > > > > +
> > > > > + if (msg->flags & I2C_M_RECV_LEN &&
> > > > > + recv_byte <= I2C_SMBUS_BLOCK_MAX) {
> > > > > + msg->len = recv_byte +
> > > > > + ((msg->flags &
> > > > > I2C_CLIENT_PEC) ? 2 : 1);
> > >
> > > ...
> > > > > + return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
> > > > > + & ASPEED_I2CD_TIME_SCL_HIGH_MASK)
> > > > > + | ((clk_low <<
> > > > > ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
> > > > > + & ASPEED_I2CD_TIME_SCL_LOW_MASK)
> > > > > + | (base_clk &
> > > > > ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
> > > > > +}
> > > >
> > > > As I think I mentioned earlier, the AST2500 has a slightly
> > > > different register layout which support larger values for high
> > > > and
> > > > low, thus allowing a finer granularity.
> > >
> > > I am developing against the 2500.
> >
> > Yes but we'd like the driver to work with both :-)
>
> Right, I thought you were making an assertion about the 2500, if you
> are making an assertion about the 2400, I do not know and do not have
> one handy.
>
> >
> > > > BTW. In case you haven't, I would suggest you copy/paste the
> > > > above
> > > > in a userspace app and run it for all frequency divisors and
> > > > see if
> > > > your results match the aspeed table :)
> > >
> > > Good call.
> >
> > If you end up doing that, can you shoot it my way ? I can take care
> > of
> > making sure it's all good for the 2400.
>
> Will do.
>
> >
> > > > > +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
> > > > > + struct platform_device *pdev) {
> > > > > + u32 clk_freq, divisor;
> > > > > + struct clk *pclk;
> > > > > + int ret;
> > > > > +
> > > > > + pclk = devm_clk_get(&pdev->dev, NULL);
> > > > > + if (IS_ERR(pclk)) {
> > > > > + dev_err(&pdev->dev, "clk_get failed\n");
> > > > > + return PTR_ERR(pclk);
> > > > > + }
> > > > > + ret = of_property_read_u32(pdev->dev.of_node,
> > > > > + "clock-frequency",
> > > > > &clk_freq);
> > > >
> > > > See my previous comment about calling that 'bus-frequency'
> > > > rather
> > > > than 'clock-frequency'.
> > > >
> > > > > + if (ret < 0) {
> > > > > + dev_err(&pdev->dev,
> > > > > + "Could not read clock-frequency
> > > > > property\n");
> > > > > + clk_freq = 100000;
> > > > > + }
> > > > > + divisor = clk_get_rate(pclk) / clk_freq;
> > > > > + /* We just need the clock rate, we don't actually use
> > > > > the
> > > > > clk object. */
> > > > > + devm_clk_put(&pdev->dev, pclk);
> > > > > +
> > > > > + /* Set AC Timing */
> > > > > + if (clk_freq / 1000 > 1000) {
> > > > > + aspeed_i2c_write(bus, aspeed_i2c_read(bus,
> > > > > + ASPEED_I2
> > > > > C_FU
> > > > > N_CTRL_REG) |
> > > > > + ASPEED_I2CD_M_HIGH_SPEED_EN |
> > > > > + ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
> > > > > + ASPEED_I2CD_SDA_DRIVE_1T_EN,
> > > > > + ASPEED_I2C_FUN_CTRL_REG);
> > > > > +
> > > > > + aspeed_i2c_write(bus, 0x3,
> > > > > ASPEED_I2C_AC_TIMING_REG2);
> > > > > + aspeed_i2c_write(bus,
> > > > > aspeed_i2c_get_clk_reg_val(divisor),
> > > > > + ASPEED_I2C_AC_TIMING_REG1);
> > > >
> > > > I already discussed by doubts about the above. I can try to
> > > > scope
> > > > it with the EVB if you don't get to it. For now I'd rather take
> > > > the
> > > > code out.
> > > >
> > > > We should ask aspeed from what frequency the "1T" stuff is
> > > > useful.
> > >
> > > Will do, I will try to rope Ryan in on the next review; it will
> > > be
> > > good for him to get used to working with upstream anyway.
> >
> > Yup. However, for the sake of getting something upstream (and in
> > OpenBMC 4.10 kernel) asap, I would suggest just dropping support
> > for
> > those fast speeds for now, we can add them back later.
>
> Alright, that's fine. Still, Ryan, could you provide some context on
> this?
>
> >
> > > >
> > > > > + } else {
> > > > > + aspeed_i2c_write(bus,
> > > > > aspeed_i2c_get_clk_reg_val(divisor),
> > > > > + ASPEED_I2C_AC_TIMING_REG1);
> > > > > + aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
> > > > > + ASPEED_I2C_AC_TIMING_REG2);
> > > > > + }
> > >
> > > ...
> > > > > + spin_lock_init(&bus->lock);
> > > > > + init_completion(&bus->cmd_complete);
> > > > > + bus->adap.owner = THIS_MODULE;
> > > > > + bus->adap.retries = 0;
> > > > > + bus->adap.timeout = 5 * HZ;
> > > > > + bus->adap.algo = &aspeed_i2c_algo;
> > > > > + bus->adap.algo_data = bus;
> > > > > + bus->adap.dev.parent = &pdev->dev;
> > > > > + bus->adap.dev.of_node = pdev->dev.of_node;
> > > > > + snprintf(bus->adap.name, sizeof(bus->adap.name),
> > > > > "Aspeed
> > > > > i2c");
> > > >
> > > > Another trivial one, should we put some kind of bus number in
> > > > that
> > > > string ?
> > >
> > > Whoops, looks like I missed this one; I will get to it in the
> > > next
> > > revision.
> >
> > Ok. I noticed you missed that in v7, so I assume you mean v8 :-)
>
> Yep, I will get it in v8.
>
> >
> > > >
> > > > > + bus->dev = &pdev->dev;
> > > > > +
> > > > > + /* reset device: disable master & slave functions */
> > > > > + aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
> > >
> > > ...
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe
> > > devicetree"
> > > in
> > > the body of a message to majordomo@vger.kernel.org More
> > > majordomo
> > > info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 09/10] arm64: allwinner: a64: enable AXP803 regulators for Pine64
From: Icenowy Zheng @ 2017-04-25 9:28 UTC (permalink / raw)
To: Andre Przywara, Thomas Gleixner, Rob Herring, Maxime Ripard,
Chen-Yu Tsai, Lee Jones, Liam Girdwood
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <8c7864f1-a0d7-b87f-f9d6-5201cc773609-5wv7dgnIgG8@public.gmane.org>
于 2017年4月25日 GMT+08:00 下午5:24:13, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> 写到:
>Hi,
>
>On 24/04/17 17:01, Icenowy Zheng wrote:
>> Add support of AXP803 regulators in the Pine64 device tree, in order
>to
>> enable many future functionalities, e.g. Wi-Fi.
>
>In general that's quite some code to just achieve some device power
>plane switching, but that's another discussion, I guess ;-)
>
>To me this patch here like a lot of churn to be duplicated in each
>board's DT.
>Can't we provide some sane defaults, either in axp803.dtsi or in an
>extra file (allwinner-ref-axp803.dtsi?), and only overwrite them in a
>board's DT if a board deviates?
For critical regulators you're right, however, peripherals' power seems to vary a lot, e.g. Wi-Fi and HDMI.
The DLDO1 for HDMI and DLDO4 for Wi-Fi design is only adopted by Pine64; according to BPi M64 schematics, Wi-Fi is DLDO2 and Wi-Fi IO voltage is DLDO4 (on Pine64 it's ELDO1 which is forced to be 1.8v because it's connected to CPVDD); on Orange Pi Win {,Plus} Wi-Fi is ALDO1, Wi-Fi IO is DLDO4 and HDMI is DLDO3.
>Because it seems like many boards are actually based on some Allwinner
>reference design and use very similar, if not identical settings.
>
>Also one thing below ...
>
>>
>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>> ---
>> .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 109
>+++++++++++++++++++++
>> 1 file changed, 109 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
>b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
>> index 3e1b44292534..abc1879e91f2 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
>> @@ -106,6 +106,115 @@
>> };
>> };
>>
>> +#include "axp803.dtsi"
>> +
>> +®_aldo1 {
>> + regulator-min-microvolt = <2800000>;
>> + regulator-max-microvolt = <2800000>;
>> + regulator-name = "vcc-csi";
>> +};
>> +
>> +®_aldo2 {
>> + regulator-always-on;
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-name = "vcc-pl";
>> +};
>> +
>> +®_aldo3 {
>> + regulator-always-on;
>> + regulator-min-microvolt = <2700000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-name = "vcc-pll-avcc";
>> +};
>> +
>> +®_dc1sw {
>> + regulator-name = "vcc-phy";
>> +};
>> +
>> +®_dcdc1 {
>> + regulator-always-on;
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-name = "vcc-3v3";
>> +};
>> +
>> +®_dcdc2 {
>> + regulator-always-on;
>> + regulator-min-microvolt = <1000000>;
>> + regulator-max-microvolt = <1300000>;
>> + regulator-name = "vdd-cpux";
>> +};
>> +
>> +/* DCDC3 is polyphased with DCDC2 */
>> +
>> +®_dcdc5 {
>> + regulator-always-on;
>> + regulator-min-microvolt = <1500000>;
>> + regulator-max-microvolt = <1500000>;
>
>On the Pine64 there is DDR3L DRAM, running at 1.35V.
>
>Cheers,
>Andre.
>
>
>> + regulator-name = "vcc-dram";
>> +};
>> +
>> +®_dcdc6 {
>> + regulator-always-on;
>> + regulator-min-microvolt = <1100000>;
>> + regulator-max-microvolt = <1100000>;
>> + regulator-name = "vdd-sys";
>> +};
>> +
>> +®_dldo1 {
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-name = "vcc-hdmi";
>> +};
>> +
>> +®_dldo2 {
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-name = "vcc-mipi";
>> +};
>> +
>> +®_dldo3 {
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-name = "avdd-csi";
>> +};
>> +
>> +®_dldo4 {
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-name = "vcc-wifi";
>> +};
>> +
>> +®_eldo1 {
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + regulator-name = "cpvdd";
>> +};
>> +
>> +®_eldo3 {
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + regulator-name = "vdd-1v8-csi";
>> +};
>> +
>> +®_fldo1 {
>> + regulator-min-microvolt = <1200000>;
>> + regulator-max-microvolt = <1200000>;
>> + regulator-name = "vcc-1v2-hsic";
>> +};
>> +
>> +®_fldo2 {
>> + regulator-always-on;
>> + regulator-min-microvolt = <1100000>;
>> + regulator-max-microvolt = <1100000>;
>> + regulator-name = "vdd-cpus";
>> +};
>> +
>> +®_rtc_ldo {
>> + regulator-name = "vcc-rtc";
>> +};
>> +
>> &uart0 {
>> pinctrl-names = "default";
>> pinctrl-0 = <&uart0_pins_a>;
>>
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v4 09/10] arm64: allwinner: a64: enable AXP803 regulators for Pine64
From: Andre Przywara @ 2017-04-25 9:24 UTC (permalink / raw)
To: icenowy-h8G6r0blFSE, Thomas Gleixner, Rob Herring, Maxime Ripard,
Chen-Yu Tsai, Lee Jones, Liam Girdwood
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20170424160103.9447-10-icenowy-h8G6r0blFSE@public.gmane.org>
Hi,
On 24/04/17 17:01, Icenowy Zheng wrote:
> Add support of AXP803 regulators in the Pine64 device tree, in order to
> enable many future functionalities, e.g. Wi-Fi.
In general that's quite some code to just achieve some device power
plane switching, but that's another discussion, I guess ;-)
To me this patch here like a lot of churn to be duplicated in each
board's DT.
Can't we provide some sane defaults, either in axp803.dtsi or in an
extra file (allwinner-ref-axp803.dtsi?), and only overwrite them in a
board's DT if a board deviates?
Because it seems like many boards are actually based on some Allwinner
reference design and use very similar, if not identical settings.
Also one thing below ...
>
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
> .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 109 +++++++++++++++++++++
> 1 file changed, 109 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> index 3e1b44292534..abc1879e91f2 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> @@ -106,6 +106,115 @@
> };
> };
>
> +#include "axp803.dtsi"
> +
> +®_aldo1 {
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> + regulator-name = "vcc-csi";
> +};
> +
> +®_aldo2 {
> + regulator-always-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-pl";
> +};
> +
> +®_aldo3 {
> + regulator-always-on;
> + regulator-min-microvolt = <2700000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-pll-avcc";
> +};
> +
> +®_dc1sw {
> + regulator-name = "vcc-phy";
> +};
> +
> +®_dcdc1 {
> + regulator-always-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-3v3";
> +};
> +
> +®_dcdc2 {
> + regulator-always-on;
> + regulator-min-microvolt = <1000000>;
> + regulator-max-microvolt = <1300000>;
> + regulator-name = "vdd-cpux";
> +};
> +
> +/* DCDC3 is polyphased with DCDC2 */
> +
> +®_dcdc5 {
> + regulator-always-on;
> + regulator-min-microvolt = <1500000>;
> + regulator-max-microvolt = <1500000>;
On the Pine64 there is DDR3L DRAM, running at 1.35V.
Cheers,
Andre.
> + regulator-name = "vcc-dram";
> +};
> +
> +®_dcdc6 {
> + regulator-always-on;
> + regulator-min-microvolt = <1100000>;
> + regulator-max-microvolt = <1100000>;
> + regulator-name = "vdd-sys";
> +};
> +
> +®_dldo1 {
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-hdmi";
> +};
> +
> +®_dldo2 {
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-mipi";
> +};
> +
> +®_dldo3 {
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "avdd-csi";
> +};
> +
> +®_dldo4 {
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "vcc-wifi";
> +};
> +
> +®_eldo1 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-name = "cpvdd";
> +};
> +
> +®_eldo3 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-name = "vdd-1v8-csi";
> +};
> +
> +®_fldo1 {
> + regulator-min-microvolt = <1200000>;
> + regulator-max-microvolt = <1200000>;
> + regulator-name = "vcc-1v2-hsic";
> +};
> +
> +®_fldo2 {
> + regulator-always-on;
> + regulator-min-microvolt = <1100000>;
> + regulator-max-microvolt = <1100000>;
> + regulator-name = "vdd-cpus";
> +};
> +
> +®_rtc_ldo {
> + regulator-name = "vcc-rtc";
> +};
> +
> &uart0 {
> pinctrl-names = "default";
> pinctrl-0 = <&uart0_pins_a>;
>
^ permalink raw reply
* Re: [PATCH v3 0/7] V4L2 fwnode support
From: Hans Verkuil @ 2017-04-25 8:51 UTC (permalink / raw)
To: Sakari Ailus, linux-media; +Cc: linux-acpi, devicetree, laurent.pinchart
In-Reply-To: <1491829376-14791-1-git-send-email-sakari.ailus@linux.intel.com>
On 10/04/17 15:02, Sakari Ailus wrote:
> Hello everyone,
>
> This patchset adds support for fwnode to V4L2. Besides OF, also ACPI based
> systems can be supported this way. By using V4L2 fwnode, the individual
> drivers do not need to be aware of the underlying firmware implementation.
> The patchset also removes specific V4L2 OF support and converts the
> affected drivers to use V4L2 fwnode.
Successfully tested with my Atmel sama5d3 board + ov2640 sensor.
Tested-by: Hans Verkuil <hans.verkuil@cisco.com>
Regards,
Hans
>
> The patchset depends on another patchset here:
>
> <URL:http://www.spinics.net/lists/linux-acpi/msg72973.html>
>
> A git tree with the dependencies can be found here:
>
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi-merge>
>
> v1 of the set can be found here:
>
> <URL:http://www.spinics.net/lists/linux-media/msg111073.html>
>
> and v2 here:
>
> <URL:http://www.spinics.net/lists/linux-media/msg114110.html>
>
> changes since v2:
>
> - Use EXPORT_SYMBOL_GPL() instead of EXPORT_SYMBOL().
>
> - Alphabetically order the topics under V4L2 core kAPI documentation.
>
> - Prefer "fwnode" variable name for struct fwnode_handle pointers instead
> of "fwn". Similarly, use "vep" for struct v4l2_fwnode_endpoint instead
> of "vfwn".
>
> - Convert existing users of OF matching to fwnode matching.
>
> - Remove OF matching support as well as compatibility between OF and
> fwnode matching.
>
> - Use of_node_cmp() to determine whether two nodes match in case both of
> them are OF nodes. There is thus no functional difference between
> existing OF matching in v1.
>
> - Continue to use struct device_node.full_name on fwnodes that are known
> to be OF nodes instead of omitting such debug information. Drivers that
> can actually use fwnode need a new interface to provide this in fwnode
> framework. This is out of scope of the patchset.
>
> - Remove linux/of.h header inclusion in
> drivers/media/v4l2-core/v4l2-flash-led-class.c.
>
> - Improved line wrapping primarily in
> drivers/media/v4l2-core/v4l2-fwnode.c.
>
> - Rewrap KernelDoc documentation for V4L2 fwnode API up to 80 characters
> per line (new patch).
>
> - Fix KernelDoc documentation, there were a few locations where the
> argument had been changed but the documentation was not updated
> accordingly.
>
> - Fix punctuation and wording in V4L2 fwnode documentation.
>
> - Drop patch "v4l: media/drv-intf/soc_mediabus.h: include dependent header
> file". It is no longer needed.
>
> - Fix obtaining port parent in v4l2_fwnode_parse_link() on ACPI.
>
> - Include newly OF-supported atmel-isi to V4L2 OF -> fwnode conversion.
>
> - Add that the v4l2-fwnode.c has origins in v4l2-of.c to the commit
> message and the file header.
>
> changes since v1:
>
> - Use existing dev_fwnode() instead of device_fwnode_handle() added by the
> ACPI graph patchset,
>
> - Fix too long line of ^'s in ReST documentation and
>
> - Drop the patch rearranging the header files. It'd better go in
> separately, if at all.
>
> Sakari Ailus (7):
> v4l: fwnode: Support generic fwnode for parsing standardised
> properties
> v4l: async: Add fwnode match support
> v4l: flash led class: Use fwnode_handle instead of device_node in init
> v4l: Switch from V4L2 OF not V4L2 fwnode API
> docs-rst: media: Sort topic list alphabetically
> docs-rst: media: Switch documentation to V4L2 fwnode API
> v4l: Remove V4L2 OF framework in favour of V4L2 fwnode framework
>
> Documentation/media/kapi/v4l2-core.rst | 20 +-
> Documentation/media/kapi/v4l2-fwnode.rst | 3 +
> Documentation/media/kapi/v4l2-of.rst | 3 -
> drivers/leds/leds-aat1290.c | 5 +-
> drivers/leds/leds-max77693.c | 5 +-
> drivers/media/i2c/Kconfig | 9 +
> drivers/media/i2c/adv7604.c | 7 +-
> drivers/media/i2c/mt9v032.c | 7 +-
> drivers/media/i2c/ov2659.c | 8 +-
> drivers/media/i2c/s5c73m3/s5c73m3-core.c | 7 +-
> drivers/media/i2c/s5k5baf.c | 6 +-
> drivers/media/i2c/smiapp/Kconfig | 1 +
> drivers/media/i2c/smiapp/smiapp-core.c | 29 ++-
> drivers/media/i2c/tc358743.c | 11 +-
> drivers/media/i2c/tvp514x.c | 6 +-
> drivers/media/i2c/tvp5150.c | 7 +-
> drivers/media/i2c/tvp7002.c | 6 +-
> drivers/media/platform/Kconfig | 3 +
> drivers/media/platform/am437x/Kconfig | 1 +
> drivers/media/platform/am437x/am437x-vpfe.c | 15 +-
> drivers/media/platform/atmel/Kconfig | 1 +
> drivers/media/platform/atmel/atmel-isc.c | 13 +-
> drivers/media/platform/exynos4-is/Kconfig | 2 +
> drivers/media/platform/exynos4-is/media-dev.c | 13 +-
> drivers/media/platform/exynos4-is/mipi-csis.c | 6 +-
> drivers/media/platform/omap3isp/isp.c | 49 ++--
> drivers/media/platform/pxa_camera.c | 11 +-
> drivers/media/platform/rcar-vin/Kconfig | 1 +
> drivers/media/platform/rcar-vin/rcar-core.c | 23 +-
> drivers/media/platform/soc_camera/Kconfig | 1 +
> drivers/media/platform/soc_camera/atmel-isi.c | 7 +-
> drivers/media/platform/soc_camera/soc_camera.c | 7 +-
> drivers/media/platform/ti-vpe/cal.c | 15 +-
> drivers/media/platform/xilinx/Kconfig | 1 +
> drivers/media/platform/xilinx/xilinx-vipp.c | 63 +++--
> drivers/media/v4l2-core/Kconfig | 3 +
> drivers/media/v4l2-core/Makefile | 4 +-
> drivers/media/v4l2-core/v4l2-async.c | 21 +-
> drivers/media/v4l2-core/v4l2-flash-led-class.c | 12 +-
> drivers/media/v4l2-core/v4l2-fwnode.c | 342 +++++++++++++++++++++++++
> drivers/media/v4l2-core/v4l2-of.c | 327 -----------------------
> include/media/v4l2-async.h | 8 +-
> include/media/v4l2-flash-led-class.h | 4 +-
> include/media/v4l2-fwnode.h | 104 ++++++++
> include/media/v4l2-of.h | 128 ---------
> include/media/v4l2-subdev.h | 3 +
> 46 files changed, 690 insertions(+), 638 deletions(-)
> create mode 100644 Documentation/media/kapi/v4l2-fwnode.rst
> delete mode 100644 Documentation/media/kapi/v4l2-of.rst
> create mode 100644 drivers/media/v4l2-core/v4l2-fwnode.c
> delete mode 100644 drivers/media/v4l2-core/v4l2-of.c
> create mode 100644 include/media/v4l2-fwnode.h
> delete mode 100644 include/media/v4l2-of.h
>
^ permalink raw reply
* Re: [PATCH v3 0/4] of: remove *phandle properties from expanded device tree
From: Frank Rowand @ 2017-04-25 8:51 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493110049-30165-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 04/25/17 01:47, frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>
> Remove "phandle" and "linux,phandle" properties from the internal
> device tree. The phandle will still be in the struct device_node
> phandle field.
>
> This is to resolve the issue found by Stephen Boyd [1] when he changed
> the type of struct property.value from void * to const void *. As
> a result of the type change, the overlay code had compile errors
> where the resolver updates phandle values.
>
> [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
>
> Patch 1 is the phandle related changes.
>
> Patches 2 - 4 are minor fixups for issues that became visible
> while implementing patch 1.
>
> Changes from v1:
^^^^^^^^
Changes from v2
> - patch 1: Remove check in __of_add_phandle_sysfs() that would not
> add a sysfs entry if IS_ENABLED(CONFIG_PPC_PSERIES)
>
> Changes from v1:
> - Remove phandle properties in of_attach_node(), before attaching
> the node to the live tree.
> - Add the phandle sysfs entry for the node in of_attach_node().
> - When creating an overlay changeset, duplicate the node phandle in
> __of_node_dup().
>
> Frank Rowand (4):
> of: remove *phandle properties from expanded device tree
> of: make __of_attach_node() static
> of: be consistent in form of file mode
> of: detect invalid phandle in overlay
>
> drivers/of/base.c | 50 +++++++++++++++++++++++++++++++++++++++----
> drivers/of/dynamic.c | 56 ++++++++++++++++++++++++++++++++++++-------------
> drivers/of/fdt.c | 40 +++++++++++++++++++++--------------
> drivers/of/of_private.h | 2 +-
> drivers/of/overlay.c | 8 ++++---
> drivers/of/resolver.c | 23 +-------------------
> include/linux/of.h | 1 +
> 7 files changed, 120 insertions(+), 60 deletions(-)
>
--
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 v6 4/5] i2c: aspeed: added driver for Aspeed I2C
From: Ryan Chen @ 2017-04-25 8:50 UTC (permalink / raw)
To: Brendan Higgins, Benjamin Herrenschmidt
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Joel Stanley, Vladimir Zapolskiy,
Kachalov Anton, Cédric Le Goater, linux-i2c@vger.kernel.org,
devicetree@vger.kernel.org, Linux Kernel Mailing List,
OpenBMC Maillist
In-Reply-To: <CAFd5g44ioW0PdFVOhY-ep8TXBNg6qC7E5yjOVf3MEdY9bPgEDA@mail.gmail.com>
Hello All,
ASPEED_I2CD_M_SDA_DRIVE_1T_EN, ASPEED_I2CD_SDA_DRIVE_1T_EN is specific for some case usage.
For example, if i2c bus is use on "high speed" and "single slave and master" and i2c bus is too long. It need drive SDA or SCL less lunacy. It would enable it.
Otherwise, don’t enable it. especially in multi-master. It can’t be enable.
Best Regards,
Ryan
信驊科技股份有限公司
ASPEED Technology Inc.
2F,No.15,Industry East Road 4.,Hsinchu Science Park, Hsinchu City 30077, Taiwan
Tel: 886-3-578-9568 #857
Fax: 886-3-578-9586
************* Email Confidentiality Notice ********************
DISCLAIMER:
This message (and any attachments) may contain legally privileged and/or other confidential information. If you have received it in error, please notify the sender by reply e-mail and immediately delete the e-mail and any attachments without copying or disclosing the contents. Thank you.
-----Original Message-----
From: Brendan Higgins [mailto:brendanhiggins@google.com]
Sent: Tuesday, April 25, 2017 4:32 PM
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Wolfram Sang <wsa@the-dreams.de>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Thomas Gleixner <tglx@linutronix.de>; Jason Cooper <jason@lakedaemon.net>; Marc Zyngier <marc.zyngier@arm.com>; Joel Stanley <joel@jms.id.au>; Vladimir Zapolskiy <vz@mleia.com>; Kachalov Anton <mouse@mayc.ru>; Cédric Le Goater <clg@kaod.org>; linux-i2c@vger.kernel.org; devicetree@vger.kernel.org; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Ryan Chen <ryan_chen@aspeedtech.com>
Subject: Re: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C
Adding Ryan.
On Mon, Apr 24, 2017 at 7:19 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Mon, 2017-04-24 at 11:56 -0700, Brendan Higgins wrote:
>> > > +struct aspeed_i2c_bus {
>> > > + struct i2c_adapter adap;
>> > > + struct device *dev;
>> > > + void __iomem *base;
>> > > + /* Synchronizes I/O mem access to base. */
>> > > + spinlock_t lock;
>> >
>> > I am not entirely convinced we need that lock. The i2c core will
>> > take a mutex protecting all operations on the bus. So we only need
>> > to synchronize between our "xfer" code and our interrupt handler.
>>
>> You are right if both having slave and master active at the same time
>> was not possible; however, it is.
>
> Right, I somewhat forgot about the slave case.
>
> ...
>
>> > Some of those error states probably also warrant a reset of the
>> > controller, I think aspeed does that in the SDK.
>>
>> For timeout and cmd_err, I do not see any argument against it; it
>> sounds like we are in a very messed up, very unknown state, so full
>> reset is probably the best last resort.
>
> Yup.
>
>> For SDA staying pulled down, I
>> think we can say with reasonable confidence that some device on our
>> bus is behaving very badly and I am not convinced that resetting the
>> controller is likely to do anything to help;
>
> Right. Hammering with STOPs and pray ...
I think sending recovery mode sends stops as a part of the recovery algorithm it executes.
>
>> that being said, I really
>> do not have any good ideas to address that. So maybe praying and
>> resetting the controller is *the most reasonable thing to do.* I
>> would like to know what you think we should do in that case.
>
> Well, there's a (small ?) chance that it's a controller bug asserting
> the line so ... but there's little we can do if not.
True.
>
>> While I was thinking about this I also realized that the SDA line
>> check after recovery happens in the else branch, but SCL line check
>> does not happen after we attempt to STOP if SCL is hung. If we decide
>> to make special note SDA being hung by a device that won't let go, we
>> might want to make a special note that SCL is hung by a device that
>> won't let go. Just a thought.
>
> Maybe. Or just "unrecoverable error"... hopefully these don't happen
> too often ... We had cases of a TPM misbehaving like that.
Yeah, definitely should print something out.
>
>> > > +out:
>>
>> ...
>> > What about I2C_M_NOSTART ?
>> >
>> > Not that I've ever seen it used... ;-)
>>
>> Right now I am not doing any of the protocol mangling options, but I
>> can add them in if you think it is important for initial support.
>
> No, not important, we can add that later if it ever becomes useful.
>
> ...
>
>> > In general, you always ACK all interrupts first. Then you handle
>> > the bits you have harvested.
>> >
>>
>> The documentation says to ACK the interrupt after handling in the RX
>> case:
>>
>> <<<
>> S/W needs to clear this status bit to allow next data receiving.
>> > > >
>>
>> I will double check with Ryan to make sure TX works the same way.
>>
>> > > + if (irq_status & ASPEED_I2CD_INTR_ERROR ||
>> > > + (!bus->msgs && bus->master_state !=
>> > > ASPEED_I2C_MASTER_STOP)) {
>>
>> ...
>> >
>> > I would set master_state to "RECOVERY" (new state ?) and ensure
>> > those things are caught if they happen outside of a recovery.
>
> I replied privately ... as long as we ack before we start a new
> command we should be ok but we shouldn't ack after.
>
> Your latest patch still does that. It will do things like start a STOP
> command *then* ack the status bits. I'm pretty sure that's bogus.
>
> That way it's a lot simpler to simply move the
>
> writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
>
> To either right after the readl of the status reg at the beginning of
> aspeed_i2c_master_irq().
>
> I would be very surprised if that didn't work properly and wasn't much
> safer than what you are currently doing.
I think I tried your way and it worked. In anycase, Ryan will be able to clarify for us.
>
>> Let me know if you still think we need a "RECOVERY" state.
>
> The way you just switch to stop state and store the error for later
> should work I think.
>
>> >
>> > > + if (bus->master_state == ASPEED_I2C_MASTER_START) {
>>
>> ...
>> >
>> > > + dev_dbg(bus->dev,
>> > > + "no slave present at %02x", msg-
>> > > >addr);
>> > > + status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> > > + bus->cmd_err = -EIO;
>> > > + do_stop(bus);
>> > > + goto out_no_complete;
>> > > + } else {
>> > > + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> > > + if (msg->flags & I2C_M_RD)
>> > > + bus->master_state =
>> > > ASPEED_I2C_MASTER_RX;
>> > > + else
>> > > + bus->master_state =
>> > > ASPEED_I2C_MASTER_TX_FIRST;
>> >
>> > What about the SMBUS_QUICK case ? (0-len transfer). Do we need to
>> > handle this here ? A quick look at the TX_FIRST case makes me think
>> > we are ok there but I'm not sure about the RX case.
>>
>> I did not think that there is an SMBUS_QUICK RX. Could you point me
>> to an example?
>
> Not so much an RX, it's more like you are sending a 1-bit data in the
> place of the Rd/Wr bit. So you have a read with a lenght of 0, I don't
> think in that case you should set ASPEED_I2CD_M_RX_CMD in
> __aspeed_i2c_do_start
Forget what I said, I was just not thinking about the fact that SMBus emulation causes the data bit to be encoded as the R/W flag. I see what you are saying; you are correct.
>
>> > I'm not sure the RX case is tight also. What completion does the HW
>> > give you for the address cycle ? Won't you get that before it has
>> > received the first character ? IE. You fall through to the read
>> > case of the state machine with the read potentially not complete
>> > yet no ?
>>
>> ...
>> > > + case ASPEED_I2C_MASTER_RX:
>> > > + if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
>> > > + dev_err(bus->dev, "master failed to RX");
>> > > + goto out_complete;
>> > > + }
>> >
>> > See my comment above for a bog standard i2c_read. Aren't you
>> > getting the completion for the address before the read is even
>> > started ?
>>
>> In practice no, but it is probably best to be safe :-)
>
> Yup :)
>> >
>> > > + status_ack |= ASPEED_I2CD_INTR_RX_DONE;
>> > > +
>> > > + recv_byte = aspeed_i2c_read(bus,
>> > > ASPEED_I2C_BYTE_BUF_REG) >> 8;
>> > > + msg->buf[bus->buf_index++] = recv_byte;
>> > > +
>> > > + if (msg->flags & I2C_M_RECV_LEN &&
>> > > + recv_byte <= I2C_SMBUS_BLOCK_MAX) {
>> > > + msg->len = recv_byte +
>> > > + ((msg->flags &
>> > > I2C_CLIENT_PEC) ? 2 : 1);
>>
>> ...
>> > > + return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
>> > > + & ASPEED_I2CD_TIME_SCL_HIGH_MASK)
>> > > + | ((clk_low <<
>> > > ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
>> > > + & ASPEED_I2CD_TIME_SCL_LOW_MASK)
>> > > + | (base_clk &
>> > > ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
>> > > +}
>> >
>> > As I think I mentioned earlier, the AST2500 has a slightly
>> > different register layout which support larger values for high and
>> > low, thus allowing a finer granularity.
>>
>> I am developing against the 2500.
>
> Yes but we'd like the driver to work with both :-)
Right, I thought you were making an assertion about the 2500, if you are making an assertion about the 2400, I do not know and do not have one handy.
>
>> > BTW. In case you haven't, I would suggest you copy/paste the above
>> > in a userspace app and run it for all frequency divisors and see if
>> > your results match the aspeed table :)
>>
>> Good call.
>
> If you end up doing that, can you shoot it my way ? I can take care of
> making sure it's all good for the 2400.
Will do.
>
>> > > +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
>> > > + struct platform_device *pdev) {
>> > > + u32 clk_freq, divisor;
>> > > + struct clk *pclk;
>> > > + int ret;
>> > > +
>> > > + pclk = devm_clk_get(&pdev->dev, NULL);
>> > > + if (IS_ERR(pclk)) {
>> > > + dev_err(&pdev->dev, "clk_get failed\n");
>> > > + return PTR_ERR(pclk);
>> > > + }
>> > > + ret = of_property_read_u32(pdev->dev.of_node,
>> > > + "clock-frequency", &clk_freq);
>> >
>> > See my previous comment about calling that 'bus-frequency' rather
>> > than 'clock-frequency'.
>> >
>> > > + if (ret < 0) {
>> > > + dev_err(&pdev->dev,
>> > > + "Could not read clock-frequency
>> > > property\n");
>> > > + clk_freq = 100000;
>> > > + }
>> > > + divisor = clk_get_rate(pclk) / clk_freq;
>> > > + /* We just need the clock rate, we don't actually use the
>> > > clk object. */
>> > > + devm_clk_put(&pdev->dev, pclk);
>> > > +
>> > > + /* Set AC Timing */
>> > > + if (clk_freq / 1000 > 1000) {
>> > > + aspeed_i2c_write(bus, aspeed_i2c_read(bus,
>> > > + ASPEED_I2C_FU
>> > > N_CTRL_REG) |
>> > > + ASPEED_I2CD_M_HIGH_SPEED_EN |
>> > > + ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
>> > > + ASPEED_I2CD_SDA_DRIVE_1T_EN,
>> > > + ASPEED_I2C_FUN_CTRL_REG);
>> > > +
>> > > + aspeed_i2c_write(bus, 0x3,
>> > > ASPEED_I2C_AC_TIMING_REG2);
>> > > + aspeed_i2c_write(bus,
>> > > aspeed_i2c_get_clk_reg_val(divisor),
>> > > + ASPEED_I2C_AC_TIMING_REG1);
>> >
>> > I already discussed by doubts about the above. I can try to scope
>> > it with the EVB if you don't get to it. For now I'd rather take the
>> > code out.
>> >
>> > We should ask aspeed from what frequency the "1T" stuff is useful.
>>
>> Will do, I will try to rope Ryan in on the next review; it will be
>> good for him to get used to working with upstream anyway.
>
> Yup. However, for the sake of getting something upstream (and in
> OpenBMC 4.10 kernel) asap, I would suggest just dropping support for
> those fast speeds for now, we can add them back later.
Alright, that's fine. Still, Ryan, could you provide some context on this?
>
>> >
>> > > + } else {
>> > > + aspeed_i2c_write(bus,
>> > > aspeed_i2c_get_clk_reg_val(divisor),
>> > > + ASPEED_I2C_AC_TIMING_REG1);
>> > > + aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
>> > > + ASPEED_I2C_AC_TIMING_REG2);
>> > > + }
>>
>> ...
>> > > + spin_lock_init(&bus->lock);
>> > > + init_completion(&bus->cmd_complete);
>> > > + bus->adap.owner = THIS_MODULE;
>> > > + bus->adap.retries = 0;
>> > > + bus->adap.timeout = 5 * HZ;
>> > > + bus->adap.algo = &aspeed_i2c_algo;
>> > > + bus->adap.algo_data = bus;
>> > > + bus->adap.dev.parent = &pdev->dev;
>> > > + bus->adap.dev.of_node = pdev->dev.of_node;
>> > > + snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed
>> > > i2c");
>> >
>> > Another trivial one, should we put some kind of bus number in that
>> > string ?
>>
>> Whoops, looks like I missed this one; I will get to it in the next
>> revision.
>
> Ok. I noticed you missed that in v7, so I assume you mean v8 :-)
Yep, I will get it in v8.
>
>> >
>> > > + bus->dev = &pdev->dev;
>> > > +
>> > > + /* reset device: disable master & slave functions */
>> > > + aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
>>
>> ...
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree"
>> in
>> the body of a message to majordomo@vger.kernel.org More majordomo
>> info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* 33027 devicetree
From: arywhite007-/E1597aS9LQAvxtiuMwx3w @ 2017-04-25 8:50 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: 09469566543158.zip --]
[-- Type: application/zip, Size: 2600 bytes --]
^ permalink raw reply
* [PATCH v3 4/4] of: detect invalid phandle in overlay
From: frowand.list @ 2017-04-25 8:47 UTC (permalink / raw)
To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel
In-Reply-To: <1493110049-30165-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@sony.com>
Overlays are not allowed to modify phandle values of previously existing
nodes because there is no information available to allow fixup up
properties that use the previously existing phandle.
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
drivers/of/overlay.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index ca0b85f5deb1..20ab49d2f7a4 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -130,6 +130,10 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
/* NOTE: Multiple mods of created nodes not supported */
tchild = of_get_child_by_name(target, cname);
if (tchild != NULL) {
+ /* new overlay phandle value conflicts with existing value */
+ if (child->phandle)
+ return -EINVAL;
+
/* apply overlay recursively */
ret = of_overlay_apply_one(ov, tchild, child);
of_node_put(tchild);
--
Frank Rowand <frank.rowand@sony.com>
^ permalink raw reply related
* [PATCH v3 3/4] of: be consistent in form of file mode
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-25 8:47 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493110049-30165-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
checkpatch whined about using S_IRUGO instead of octal equivalent
when adding phandle sysfs code, so used octal in that patch.
Change other instances of the S_* constants in the same file to
the octal form.
Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
drivers/of/base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 41473b1e2445..9fe42346925b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -168,7 +168,7 @@ int __of_add_property_sysfs(struct device_node *np, struct property *pp)
sysfs_bin_attr_init(&pp->attr);
pp->attr.attr.name = safe_name(&np->kobj, pp->name);
- pp->attr.attr.mode = secure ? S_IRUSR : S_IRUGO;
+ pp->attr.attr.mode = secure ? 0400 : 0444;
pp->attr.size = secure ? 0 : pp->length;
pp->attr.read = of_node_property_read;
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 2/4] of: make __of_attach_node() static
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-25 8:47 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493110049-30165-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
__of_attach_node() is not used outside of drivers/of/dynamic.c. Make
it static and remove it from drivers/of/of_private.h.
Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
drivers/of/dynamic.c | 2 +-
drivers/of/of_private.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 59545b8fbf46..0b9cf6d5914c 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -216,7 +216,7 @@ int of_property_notify(int action, struct device_node *np,
return of_reconfig_notify(action, &pr);
}
-void __of_attach_node(struct device_node *np)
+static void __of_attach_node(struct device_node *np)
{
np->child = NULL;
np->sibling = np->parent->child;
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 33f11a5384f3..b1ff70e1fdda 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -79,7 +79,6 @@ extern int __of_update_property(struct device_node *np,
extern void __of_update_property_sysfs(struct device_node *np,
struct property *newprop, struct property *oldprop);
-extern void __of_attach_node(struct device_node *np);
extern int __of_attach_node_sysfs(struct device_node *np);
extern void __of_detach_node(struct device_node *np);
extern void __of_detach_node_sysfs(struct device_node *np);
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 1/4] of: remove *phandle properties from expanded device tree
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-25 8:47 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493110049-30165-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
Remove "phandle", "linux,phandle", and "ibm,phandle" properties from
the internal device tree. The phandle will still be in the struct
device_node phandle field.
This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *. As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.
[1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
- Add sysfs infrastructure to report np->phandle, as if it was a property.
- Do not create "phandle" "ibm,phandle", and "linux,phandle" properties
in the expanded device tree.
- Remove phandle properties in of_attach_node(), for nodes dynamically
attached to the live tree. Add the phandle sysfs entry for these nodes.
- When creating an overlay changeset, duplicate the node phandle in
__of_node_dup().
- Remove no longer needed checks to exclude "phandle" and "linux,phandle"
properties in several locations.
- A side effect of these changes is that the obsolete "linux,phandle" and
"ibm,phandle" properties will no longer appear in /proc/device-tree (they
will appear as "phandle").
Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
drivers/of/base.c | 48 ++++++++++++++++++++++++++++++++++++++++---
drivers/of/dynamic.c | 54 +++++++++++++++++++++++++++++++++++++------------
drivers/of/fdt.c | 40 +++++++++++++++++++++---------------
drivers/of/of_private.h | 1 +
drivers/of/overlay.c | 4 +---
drivers/of/resolver.c | 23 +--------------------
include/linux/of.h | 1 +
7 files changed, 114 insertions(+), 57 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d7c4629a3a2d..41473b1e2445 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -116,6 +116,19 @@ static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length);
}
+static ssize_t of_node_phandle_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t offset, size_t count)
+{
+ phandle phandle;
+ struct device_node *np;
+
+ np = container_of(bin_attr, struct device_node, attr_phandle);
+ phandle = cpu_to_be32(np->phandle);
+ return memory_read_from_buffer(buf, count, &offset, &phandle,
+ sizeof(phandle));
+}
+
/* always return newly allocated name, caller must free after use */
static const char *safe_name(struct kobject *kobj, const char *orig_name)
{
@@ -164,6 +177,35 @@ int __of_add_property_sysfs(struct device_node *np, struct property *pp)
return rc;
}
+/*
+ * In the imported device tree (fdt), phandle is a property. In the
+ * internal data structure it is instead stored in the struct device_node.
+ * Make phandle visible in sysfs as if it was a property.
+ */
+int __of_add_phandle_sysfs(struct device_node *np)
+{
+ int rc;
+
+ if (!IS_ENABLED(CONFIG_SYSFS))
+ return 0;
+
+ if (!of_kset || !of_node_is_attached(np))
+ return 0;
+
+ if (!np->phandle || np->phandle == 0xffffffff)
+ return 0;
+
+ sysfs_bin_attr_init(&pp->attr);
+ np->attr_phandle.attr.name = "phandle";
+ np->attr_phandle.attr.mode = 0444;
+ np->attr_phandle.size = sizeof(np->phandle);
+ np->attr_phandle.read = of_node_phandle_read;
+
+ rc = sysfs_create_bin_file(&np->kobj, &np->attr_phandle);
+ WARN(rc, "error adding attribute phandle to node %s\n", np->full_name);
+ return rc;
+}
+
int __of_attach_node_sysfs(struct device_node *np)
{
const char *name;
@@ -193,6 +235,8 @@ int __of_attach_node_sysfs(struct device_node *np)
if (rc)
return rc;
+ __of_add_phandle_sysfs(np);
+
for_each_property_of_node(np, pp)
__of_add_property_sysfs(np, pp);
@@ -2097,9 +2141,7 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
int id, len;
/* Skip those we do not want to proceed */
- if (!strcmp(pp->name, "name") ||
- !strcmp(pp->name, "phandle") ||
- !strcmp(pp->name, "linux,phandle"))
+ if (!strcmp(pp->name, "name"))
continue;
np = of_find_node_by_path(pp->value);
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 888fdbc09992..59545b8fbf46 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -218,19 +218,6 @@ int of_property_notify(int action, struct device_node *np,
void __of_attach_node(struct device_node *np)
{
- const __be32 *phandle;
- int sz;
-
- np->name = __of_get_property(np, "name", NULL) ? : "<NULL>";
- np->type = __of_get_property(np, "device_type", NULL) ? : "<NULL>";
-
- phandle = __of_get_property(np, "phandle", &sz);
- if (!phandle)
- phandle = __of_get_property(np, "linux,phandle", &sz);
- if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle)
- phandle = __of_get_property(np, "ibm,phandle", &sz);
- np->phandle = (phandle && (sz >= 4)) ? be32_to_cpup(phandle) : 0;
-
np->child = NULL;
np->sibling = np->parent->child;
np->parent->child = np;
@@ -243,11 +230,47 @@ void __of_attach_node(struct device_node *np)
int of_attach_node(struct device_node *np)
{
struct of_reconfig_data rd;
+ struct property *prev;
+ struct property *prop;
+ struct property *save_next;
unsigned long flags;
+ const __be32 *phandle;
+ int sz;
memset(&rd, 0, sizeof(rd));
rd.dn = np;
+ np->name = __of_get_property(np, "name", NULL) ? : "<NULL>";
+ np->type = __of_get_property(np, "device_type", NULL) ? : "<NULL>";
+
+ phandle = __of_get_property(np, "phandle", &sz);
+ if (!phandle)
+ phandle = __of_get_property(np, "linux,phandle", &sz);
+ if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle)
+ phandle = __of_get_property(np, "ibm,phandle", &sz);
+ np->phandle = (phandle && (sz >= 4)) ? be32_to_cpup(phandle) : 0;
+
+ /* remove phandle properties from node */
+ prev = NULL;
+ for (prop = np->properties; prop != NULL; ) {
+ save_next = prop->next;
+ if (!strcmp(prop->name, "phandle") ||
+ !strcmp(prop->name, "ibm,phandle") ||
+ !strcmp(prop->name, "linux,phandle")) {
+ if (prev)
+ prev->next = prop->next;
+ else
+ np->properties = prop->next;
+ prop->next = np->deadprops;
+ np->deadprops = prop;
+ } else {
+ prev = prop;
+ }
+ prop = save_next;
+ }
+
+ __of_add_phandle_sysfs(np);
+
mutex_lock(&of_mutex);
raw_spin_lock_irqsave(&devtree_lock, flags);
__of_attach_node(np);
@@ -429,6 +452,7 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
/* Iterate over and duplicate all properties */
if (np) {
struct property *pp, *new_pp;
+ node->phandle = np->phandle;
for_each_property_of_node(np, pp) {
new_pp = __of_prop_dup(pp, GFP_KERNEL);
if (!new_pp)
@@ -441,6 +465,10 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
}
}
}
+
+ node->name = __of_get_property(node, "name", NULL) ? : "<NULL>";
+ node->type = __of_get_property(node, "device_type", NULL) ? : "<NULL>";
+
return node;
err_prop:
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index e5ce4b59e162..270f31b0c259 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -181,6 +181,8 @@ static void populate_properties(const void *blob,
const __be32 *val;
const char *pname;
u32 sz;
+ int prop_is_phandle = 0;
+ int prop_is_ibm_phandle = 0;
val = fdt_getprop_by_offset(blob, cur, &pname, &sz);
if (!val) {
@@ -196,11 +198,6 @@ static void populate_properties(const void *blob,
if (!strcmp(pname, "name"))
has_name = true;
- pp = unflatten_dt_alloc(mem, sizeof(struct property),
- __alignof__(struct property));
- if (dryrun)
- continue;
-
/* We accept flattened tree phandles either in
* ePAPR-style "phandle" properties, or the
* legacy "linux,phandle" properties. If both
@@ -208,23 +205,34 @@ static void populate_properties(const void *blob,
* will get weird. Don't do that.
*/
if (!strcmp(pname, "phandle") ||
- !strcmp(pname, "linux,phandle")) {
- if (!np->phandle)
- np->phandle = be32_to_cpup(val);
- }
+ !strcmp(pname, "linux,phandle"))
+ prop_is_phandle = 1;
/* And we process the "ibm,phandle" property
* used in pSeries dynamic device tree
* stuff
*/
- if (!strcmp(pname, "ibm,phandle"))
- np->phandle = be32_to_cpup(val);
+ if (!strcmp(pname, "ibm,phandle")) {
+ prop_is_phandle = 1;
+ prop_is_ibm_phandle = 1;
+ }
+
+ if (!prop_is_phandle)
+ pp = unflatten_dt_alloc(mem, sizeof(struct property),
+ __alignof__(struct property));
- pp->name = (char *)pname;
- pp->length = sz;
- pp->value = (__be32 *)val;
- *pprev = pp;
- pprev = &pp->next;
+ if (dryrun)
+ continue;
+
+ if (!prop_is_phandle) {
+ pp->name = (char *)pname;
+ pp->length = sz;
+ pp->value = (__be32 *)val;
+ *pprev = pp;
+ pprev = &pp->next;
+ } else if (prop_is_ibm_phandle || !np->phandle) {
+ np->phandle = be32_to_cpup(val);
+ }
}
/* With version 0x10 we may not have the name property,
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 18bbb4517e25..33f11a5384f3 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -47,6 +47,7 @@ extern int of_property_notify(int action, struct device_node *np,
extern void of_node_release(struct kobject *kobj);
extern int __of_changeset_apply(struct of_changeset *ocs);
extern int __of_changeset_revert(struct of_changeset *ocs);
+extern int __of_add_phandle_sysfs(struct device_node *np);
#else /* CONFIG_OF_DYNAMIC */
static inline int of_property_notify(int action, struct device_node *np,
struct property *prop, struct property *old_prop)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 7827786718d8..ca0b85f5deb1 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -101,9 +101,7 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
tprop = of_find_property(target, prop->name, NULL);
/* special properties are not meant to be updated (silent NOP) */
- if (of_prop_cmp(prop->name, "name") == 0 ||
- of_prop_cmp(prop->name, "phandle") == 0 ||
- of_prop_cmp(prop->name, "linux,phandle") == 0)
+ if (!of_prop_cmp(prop->name, "name"))
return 0;
propn = __of_prop_dup(prop, GFP_KERNEL);
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 7ae9863cb0a4..624cbaeae0a4 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -71,30 +71,11 @@ static void adjust_overlay_phandles(struct device_node *overlay,
int phandle_delta)
{
struct device_node *child;
- struct property *prop;
- phandle phandle;
/* adjust node's phandle in node */
if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
overlay->phandle += phandle_delta;
- /* copy adjusted phandle into *phandle properties */
- for_each_property_of_node(overlay, prop) {
-
- if (of_prop_cmp(prop->name, "phandle") &&
- of_prop_cmp(prop->name, "linux,phandle"))
- continue;
-
- if (prop->length < 4)
- continue;
-
- phandle = be32_to_cpup(prop->value);
- if (phandle == OF_PHANDLE_ILLEGAL)
- continue;
-
- *(uint32_t *)prop->value = cpu_to_be32(overlay->phandle);
- }
-
for_each_child_of_node(overlay, child)
adjust_overlay_phandles(child, phandle_delta);
}
@@ -197,9 +178,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
for_each_property_of_node(local_fixups, prop_fix) {
/* skip properties added automatically */
- if (!of_prop_cmp(prop_fix->name, "name") ||
- !of_prop_cmp(prop_fix->name, "phandle") ||
- !of_prop_cmp(prop_fix->name, "linux,phandle"))
+ if (!of_prop_cmp(prop_fix->name, "name"))
continue;
if ((prop_fix->length % 4) != 0 || prop_fix->length == 0)
diff --git a/include/linux/of.h b/include/linux/of.h
index 21e6323de0f3..4e86e05853f3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -61,6 +61,7 @@ struct device_node {
struct kobject kobj;
unsigned long _flags;
void *data;
+ struct bin_attribute attr_phandle;
#if defined(CONFIG_SPARC)
const char *path_component_name;
unsigned int unique_id;
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 0/4] of: remove *phandle properties from expanded device tree
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-25 8:47 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
Remove "phandle" and "linux,phandle" properties from the internal
device tree. The phandle will still be in the struct device_node
phandle field.
This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *. As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.
[1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
Patch 1 is the phandle related changes.
Patches 2 - 4 are minor fixups for issues that became visible
while implementing patch 1.
Changes from v1:
- patch 1: Remove check in __of_add_phandle_sysfs() that would not
add a sysfs entry if IS_ENABLED(CONFIG_PPC_PSERIES)
Changes from v1:
- Remove phandle properties in of_attach_node(), before attaching
the node to the live tree.
- Add the phandle sysfs entry for the node in of_attach_node().
- When creating an overlay changeset, duplicate the node phandle in
__of_node_dup().
Frank Rowand (4):
of: remove *phandle properties from expanded device tree
of: make __of_attach_node() static
of: be consistent in form of file mode
of: detect invalid phandle in overlay
drivers/of/base.c | 50 +++++++++++++++++++++++++++++++++++++++----
drivers/of/dynamic.c | 56 ++++++++++++++++++++++++++++++++++++-------------
drivers/of/fdt.c | 40 +++++++++++++++++++++--------------
drivers/of/of_private.h | 2 +-
drivers/of/overlay.c | 8 ++++---
drivers/of/resolver.c | 23 +-------------------
include/linux/of.h | 1 +
7 files changed, 120 insertions(+), 60 deletions(-)
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Re: [PATCH v4 07/10] mfd: axp20x: add axp20x-regulator cell for AXP803
From: Icenowy Zheng @ 2017-04-25 8:37 UTC (permalink / raw)
To: lee.jones-QSEj5FYQhm4dnm+yROfE0A
Cc: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
Liam Girdwood, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20170425075717.l4x3qektdgt6kaiq@dell>
于 2017年4月25日 GMT+08:00 下午3:57:17, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 写到:
>On Tue, 25 Apr 2017, Icenowy Zheng wrote:
>
>> As axp20x-regulator now supports AXP803, add a cell for it.
>>
>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>> Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>> ---
>> Changes in v4:
>> - Added a trailing comma for new cell, for easier further cell
>addition.
>> Changes in v3:
>> - Make the new cell one-liner.
>>
>> drivers/mfd/axp20x.c | 3 ++-
>> drivers/regulator/axp20x-regulator.c | 6 +++---
>
>These 2 changes are orthogonal, thus there is no reason to send them
>bundled into a single patch. Doing so complicates things greatly.
>Please resubmit the two changes separately, so that they may be
>absorbed by our respective subsystems.
Yes. As Chen-Yu said, I wrongly squashed the regulator renaming patch here...
It should be squashed into the regulator patch.
Sorry for it.
>
>> 2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index 1dc6235778eb..917b6ddc4f15 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -848,7 +848,8 @@ static struct mfd_cell axp803_cells[] = {
>> .name = "axp20x-pek",
>> .num_resources = ARRAY_SIZE(axp803_pek_resources),
>> .resources = axp803_pek_resources,
>> - }
>> + },
>> + { .name = "axp20x-regulator" },
>> };
>>
>> static struct mfd_cell axp806_cells[] = {
>> diff --git a/drivers/regulator/axp20x-regulator.c
>b/drivers/regulator/axp20x-regulator.c
>> index 9356ec8a9a1f..e2608fe770b9 100644
>> --- a/drivers/regulator/axp20x-regulator.c
>> +++ b/drivers/regulator/axp20x-regulator.c
>> @@ -311,13 +311,13 @@ static const struct regulator_desc
>axp803_regulators[] = {
>> AXP803_FLDO1_V_OUT, 0x0f, AXP22X_PWR_OUT_CTRL3, BIT(2)),
>> AXP_DESC(AXP803, FLDO2, "fldo2", "fldoin", 700, 1450, 50,
>> AXP803_FLDO2_V_OUT, 0x0f, AXP22X_PWR_OUT_CTRL3, BIT(3)),
>> - AXP_DESC_IO(AXP803, LDO_IO0, "ldo_io0", "ips", 700, 3300, 100,
>> + AXP_DESC_IO(AXP803, LDO_IO0, "ldo-io0", "ips", 700, 3300, 100,
>> AXP22X_LDO_IO0_V_OUT, 0x1f, AXP20X_GPIO0_CTRL, 0x07,
>> AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
>> - AXP_DESC_IO(AXP803, LDO_IO1, "ldo_io1", "ips", 700, 3300, 100,
>> + AXP_DESC_IO(AXP803, LDO_IO1, "ldo-io1", "ips", 700, 3300, 100,
>> AXP22X_LDO_IO1_V_OUT, 0x1f, AXP20X_GPIO1_CTRL, 0x07,
>> AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
>> - AXP_DESC_FIXED(AXP803, RTC_LDO, "rtc_ldo", "ips", 3000),
>> + AXP_DESC_FIXED(AXP803, RTC_LDO, "rtc-ldo", "ips", 3000),
>> };
>>
>> static const struct regulator_linear_range axp806_dcdca_ranges[] = {
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v7 5/5] i2c: aspeed: added slave support for Aspeed I2C driver
From: Brendan Higgins @ 2017-04-25 8:35 UTC (permalink / raw)
To: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Joel Stanley, Vladimir Zapolskiy,
Kachalov Anton, Cédric Le Goater, Benjamin Herrenschmidt
Cc: linux-i2c, devicetree, Linux Kernel Mailing List,
OpenBMC Maillist, Brendan Higgins, Ryan Chen
In-Reply-To: <20170424181818.2754-6-brendanhiggins@google.com>
Adding Ryan (sorry to everyone else for the spam).
^ permalink raw reply
* Re: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C
From: Brendan Higgins @ 2017-04-25 8:32 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Joel Stanley, Vladimir Zapolskiy,
Kachalov Anton, Cédric Le Goater,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
OpenBMC Maillist, Ryan Chen
In-Reply-To: <1493086747.25766.264.camel-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
Adding Ryan.
On Mon, Apr 24, 2017 at 7:19 PM, Benjamin Herrenschmidt
<benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> wrote:
> On Mon, 2017-04-24 at 11:56 -0700, Brendan Higgins wrote:
>> > > +struct aspeed_i2c_bus {
>> > > + struct i2c_adapter adap;
>> > > + struct device *dev;
>> > > + void __iomem *base;
>> > > + /* Synchronizes I/O mem access to base. */
>> > > + spinlock_t lock;
>> >
>> > I am not entirely convinced we need that lock. The i2c core will
>> > take a mutex protecting all operations on the bus. So we only need
>> > to synchronize between our "xfer" code and our interrupt handler.
>>
>> You are right if both having slave and master active at the same time
>> was not possible; however, it is.
>
> Right, I somewhat forgot about the slave case.
>
> ...
>
>> > Some of those error states probably also warrant a reset of the
>> > controller,
>> > I think aspeed does that in the SDK.
>>
>> For timeout and cmd_err, I do not see any argument against it; it
>> sounds like we are in a very messed up, very unknown state, so full
>> reset is probably the best last resort.
>
> Yup.
>
>> For SDA staying pulled down, I
>> think we can say with reasonable confidence that some device on our
>> bus is behaving very badly and I am not convinced that resetting the
>> controller is likely to do anything to help;
>
> Right. Hammering with STOPs and pray ...
I think sending recovery mode sends stops as a part of the recovery
algorithm it executes.
>
>> that being said, I really
>> do not have any good ideas to address that. So maybe praying and
>> resetting the controller is *the most reasonable thing to do.* I
>> would like to know what you think we should do in that case.
>
> Well, there's a (small ?) chance that it's a controller bug asserting
> the line so ... but there's little we can do if not.
True.
>
>> While I was thinking about this I also realized that the SDA line
>> check after recovery happens in the else branch, but SCL line check
>> does not happen after we attempt to STOP if SCL is hung. If we decide
>> to make special note SDA being hung by a device that won't let go, we
>> might want to make a special note that SCL is hung by a device that
>> won't let go. Just a thought.
>
> Maybe. Or just "unrecoverable error"... hopefully these don't happen
> too often ... We had cases of a TPM misbehaving like that.
Yeah, definitely should print something out.
>
>> > > +out:
>>
>> ...
>> > What about I2C_M_NOSTART ?
>> >
>> > Not that I've ever seen it used... ;-)
>>
>> Right now I am not doing any of the protocol mangling options, but I
>> can add them in if you think it is important for initial support.
>
> No, not important, we can add that later if it ever becomes useful.
>
> ...
>
>> > In general, you always ACK all interrupts first. Then you handle
>> > the bits you have harvested.
>> >
>>
>> The documentation says to ACK the interrupt after handling in the RX
>> case:
>>
>> <<<
>> S/W needs to clear this status bit to allow next data receiving.
>> > > >
>>
>> I will double check with Ryan to make sure TX works the same way.
>>
>> > > + if (irq_status & ASPEED_I2CD_INTR_ERROR ||
>> > > + (!bus->msgs && bus->master_state !=
>> > > ASPEED_I2C_MASTER_STOP)) {
>>
>> ...
>> >
>> > I would set master_state to "RECOVERY" (new state ?) and ensure
>> > those things are caught if they happen outside of a recovery.
>
> I replied privately ... as long as we ack before we start a new command
> we should be ok but we shouldn't ack after.
>
> Your latest patch still does that. It will do things like start a STOP
> command *then* ack the status bits. I'm pretty sure that's bogus.
>
> That way it's a lot simpler to simply move the
>
> writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
>
> To either right after the readl of the status reg at the beginning of
> aspeed_i2c_master_irq().
>
> I would be very surprised if that didn't work properly and wasn't much
> safer than what you are currently doing.
I think I tried your way and it worked. In anycase, Ryan will be able
to clarify for us.
>
>> Let me know if you still think we need a "RECOVERY" state.
>
> The way you just switch to stop state and store the error for later
> should work I think.
>
>> >
>> > > + if (bus->master_state == ASPEED_I2C_MASTER_START) {
>>
>> ...
>> >
>> > > + dev_dbg(bus->dev,
>> > > + "no slave present at %02x", msg-
>> > > >addr);
>> > > + status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> > > + bus->cmd_err = -EIO;
>> > > + do_stop(bus);
>> > > + goto out_no_complete;
>> > > + } else {
>> > > + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> > > + if (msg->flags & I2C_M_RD)
>> > > + bus->master_state =
>> > > ASPEED_I2C_MASTER_RX;
>> > > + else
>> > > + bus->master_state =
>> > > ASPEED_I2C_MASTER_TX_FIRST;
>> >
>> > What about the SMBUS_QUICK case ? (0-len transfer). Do we need
>> > to handle this here ? A quick look at the TX_FIRST case makes
>> > me think we are ok there but I'm not sure about the RX case.
>>
>> I did not think that there is an SMBUS_QUICK RX. Could you point me
>> to an example?
>
> Not so much an RX, it's more like you are sending a 1-bit data in
> the place of the Rd/Wr bit. So you have a read with a lenght of 0,
> I don't think in that case you should set ASPEED_I2CD_M_RX_CMD in
> __aspeed_i2c_do_start
Forget what I said, I was just not thinking about the fact that SMBus
emulation causes the data bit to be encoded as the R/W flag. I see
what you are saying; you are correct.
>
>> > I'm not sure the RX case is tight also. What completion does the
>> > HW give you for the address cycle ? Won't you get that before it
>> > has received the first character ? IE. You fall through to
>> > the read case of the state machine with the read potentially
>> > not complete yet no ?
>>
>> ...
>> > > + case ASPEED_I2C_MASTER_RX:
>> > > + if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
>> > > + dev_err(bus->dev, "master failed to RX");
>> > > + goto out_complete;
>> > > + }
>> >
>> > See my comment above for a bog standard i2c_read. Aren't you
>> > getting
>> > the completion for the address before the read is even started ?
>>
>> In practice no, but it is probably best to be safe :-)
>
> Yup :)
>> >
>> > > + status_ack |= ASPEED_I2CD_INTR_RX_DONE;
>> > > +
>> > > + recv_byte = aspeed_i2c_read(bus,
>> > > ASPEED_I2C_BYTE_BUF_REG) >> 8;
>> > > + msg->buf[bus->buf_index++] = recv_byte;
>> > > +
>> > > + if (msg->flags & I2C_M_RECV_LEN &&
>> > > + recv_byte <= I2C_SMBUS_BLOCK_MAX) {
>> > > + msg->len = recv_byte +
>> > > + ((msg->flags &
>> > > I2C_CLIENT_PEC) ? 2 : 1);
>>
>> ...
>> > > + return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
>> > > + & ASPEED_I2CD_TIME_SCL_HIGH_MASK)
>> > > + | ((clk_low <<
>> > > ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
>> > > + & ASPEED_I2CD_TIME_SCL_LOW_MASK)
>> > > + | (base_clk &
>> > > ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
>> > > +}
>> >
>> > As I think I mentioned earlier, the AST2500 has a slightly
>> > different
>> > register layout which support larger values for high and low, thus
>> > allowing a finer granularity.
>>
>> I am developing against the 2500.
>
> Yes but we'd like the driver to work with both :-)
Right, I thought you were making an assertion about the 2500, if you
are making an assertion about the 2400, I do not know and do not have
one handy.
>
>> > BTW. In case you haven't, I would suggest you copy/paste the above
>> > in
>> > a userspace app and run it for all frequency divisors and see if
>> > your
>> > results match the aspeed table :)
>>
>> Good call.
>
> If you end up doing that, can you shoot it my way ? I can take care
> of making sure it's all good for the 2400.
Will do.
>
>> > > +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
>> > > + struct platform_device *pdev)
>> > > +{
>> > > + u32 clk_freq, divisor;
>> > > + struct clk *pclk;
>> > > + int ret;
>> > > +
>> > > + pclk = devm_clk_get(&pdev->dev, NULL);
>> > > + if (IS_ERR(pclk)) {
>> > > + dev_err(&pdev->dev, "clk_get failed\n");
>> > > + return PTR_ERR(pclk);
>> > > + }
>> > > + ret = of_property_read_u32(pdev->dev.of_node,
>> > > + "clock-frequency", &clk_freq);
>> >
>> > See my previous comment about calling that 'bus-frequency' rather
>> > than 'clock-frequency'.
>> >
>> > > + if (ret < 0) {
>> > > + dev_err(&pdev->dev,
>> > > + "Could not read clock-frequency
>> > > property\n");
>> > > + clk_freq = 100000;
>> > > + }
>> > > + divisor = clk_get_rate(pclk) / clk_freq;
>> > > + /* We just need the clock rate, we don't actually use the
>> > > clk object. */
>> > > + devm_clk_put(&pdev->dev, pclk);
>> > > +
>> > > + /* Set AC Timing */
>> > > + if (clk_freq / 1000 > 1000) {
>> > > + aspeed_i2c_write(bus, aspeed_i2c_read(bus,
>> > > + ASPEED_I2C_FU
>> > > N_CTRL_REG) |
>> > > + ASPEED_I2CD_M_HIGH_SPEED_EN |
>> > > + ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
>> > > + ASPEED_I2CD_SDA_DRIVE_1T_EN,
>> > > + ASPEED_I2C_FUN_CTRL_REG);
>> > > +
>> > > + aspeed_i2c_write(bus, 0x3,
>> > > ASPEED_I2C_AC_TIMING_REG2);
>> > > + aspeed_i2c_write(bus,
>> > > aspeed_i2c_get_clk_reg_val(divisor),
>> > > + ASPEED_I2C_AC_TIMING_REG1);
>> >
>> > I already discussed by doubts about the above. I can try to scope
>> > it with the EVB if you don't get to it. For now I'd rather take the
>> > code out.
>> >
>> > We should ask aspeed from what frequency the "1T" stuff is useful.
>>
>> Will do, I will try to rope Ryan in on the next review; it will be
>> good for him to get used to working with upstream anyway.
>
> Yup. However, for the sake of getting something upstream (and in
> OpenBMC 4.10 kernel) asap, I would suggest just dropping support
> for those fast speeds for now, we can add them back later.
Alright, that's fine. Still, Ryan, could you provide some context on this?
>
>> >
>> > > + } else {
>> > > + aspeed_i2c_write(bus,
>> > > aspeed_i2c_get_clk_reg_val(divisor),
>> > > + ASPEED_I2C_AC_TIMING_REG1);
>> > > + aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
>> > > + ASPEED_I2C_AC_TIMING_REG2);
>> > > + }
>>
>> ...
>> > > + spin_lock_init(&bus->lock);
>> > > + init_completion(&bus->cmd_complete);
>> > > + bus->adap.owner = THIS_MODULE;
>> > > + bus->adap.retries = 0;
>> > > + bus->adap.timeout = 5 * HZ;
>> > > + bus->adap.algo = &aspeed_i2c_algo;
>> > > + bus->adap.algo_data = bus;
>> > > + bus->adap.dev.parent = &pdev->dev;
>> > > + bus->adap.dev.of_node = pdev->dev.of_node;
>> > > + snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed
>> > > i2c");
>> >
>> > Another trivial one, should we put some kind of bus number
>> > in that string ?
>>
>> Whoops, looks like I missed this one; I will get to it in the next
>> revision.
>
> Ok. I noticed you missed that in v7, so I assume you mean v8 :-)
Yep, I will get it in v8.
>
>> >
>> > > + bus->dev = &pdev->dev;
>> > > +
>> > > + /* reset device: disable master & slave functions */
>> > > + aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
>>
>> ...
>> --
>> 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
--
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 v9 3/3] backlight arcxcnn add support for ArcticSand devices
From: Lee Jones @ 2017-04-25 8:19 UTC (permalink / raw)
To: Olimpiu Dejeu
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
jingoohan1-Re5JQEeQqe8AvxtiuMwx3w, bdodge-eV7fy4qpoLhpLGFMi4vTTA,
joe-6d6DIl74uiNBDgjK7y7TUQ, medasaro-eV7fy4qpoLhpLGFMi4vTTA,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A,
lkp-ral2JQCrhuEAvxtiuMwx3w, fengguang.wu-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <1490115528-11044-3-git-send-email-olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
On Tue, 21 Mar 2017, Olimpiu Dejeu wrote:
> backlight: Add support for Arctic Sand LED backlight driver chips
> This driver provides support for the Arctic Sand arc2c0608 chip,
> and provides a framework to support future devices.
> Reviewed-by: Daniel Thompson <daniel.thompson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> ---
> v8 => v9:
> - Addressing kbuild test robot WARNING: PTR_ERR_OR_ZERO can be used
> v7 => v8:
> - Version updated to match other patch in set. No other changes.
> v6 => v7:
> - Addressing issues brought up by Daniel Thompson
> v5 => v6:
> - Addressing issues brought up by Daniel Thompson
> v4 => v5:
> - Code style changes per Joe Perches and Jingoo Han
> v3 => v4:
> - Code style changes per Joe Perches and Jingoo Han
> v2 => v3:
> - Renamed variables to comply with conventions on naming
> - Corrected device name in arcxcnn.h
> v1 => v2:
> - Removed "magic numbers" to initialize registers
> - Cleaned up device tree bindings
> - Fixed code style to address comments and pass "checkpatch"
> - Removed unneeded debug and testing code
>
>
> drivers/video/backlight/Kconfig | 7 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/arcxcnn_bl.c | 419 +++++++++++++++++++++++++++++++++++
> 3 files changed, 427 insertions(+)
> create mode 100644 drivers/video/backlight/arcxcnn_bl.c
Applied this (and the correct (v9) version of [PATCH 2/3] too).
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 5ffa4b4..4e1d2ad 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -460,6 +460,13 @@ config BACKLIGHT_BD6107
> help
> If you have a Rohm BD6107 say Y to enable the backlight driver.
>
> +config BACKLIGHT_ARCXCNN
> + tristate "Backlight driver for the Arctic Sands ARCxCnnnn family"
> + depends on I2C
> + help
> + If you have an ARCxCnnnn family backlight say Y to enable
> + the backlight driver.
> +
> endif # BACKLIGHT_CLASS_DEVICE
>
> endif # BACKLIGHT_LCD_SUPPORT
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 16ec534..8905129 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -55,3 +55,4 @@ obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
> obj-$(CONFIG_BACKLIGHT_TOSA) += tosa_bl.o
> obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
> obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
> +obj-$(CONFIG_BACKLIGHT_ARCXCNN) += arcxcnn_bl.o
> diff --git a/drivers/video/backlight/arcxcnn_bl.c b/drivers/video/backlight/arcxcnn_bl.c
> new file mode 100644
> index 0000000..e01b1b0
> --- /dev/null
> +++ b/drivers/video/backlight/arcxcnn_bl.c
> @@ -0,0 +1,419 @@
> +/*
> + * Backlight driver for ArcticSand ARC_X_C_0N_0N Devices
> + *
> + * Copyright 2016 ArcticSand, Inc.
> + * Author : Brian Dodge <bdodge-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +
> +enum arcxcnn_chip_id {
> + ARC2C0608
> +};
> +
> +/**
> + * struct arcxcnn_platform_data
> + * @name : Backlight driver name (NULL will use default)
> + * @initial_brightness : initial value of backlight brightness
> + * @leden : initial LED string enables, upper bit is global on/off
> + * @led_config_0 : fading speed (period between intensity steps)
> + * @led_config_1 : misc settings, see datasheet
> + * @dim_freq : pwm dimming frequency if in pwm mode
> + * @comp_config : misc config, see datasheet
> + * @filter_config : RC/PWM filter config, see datasheet
> + * @trim_config : full scale current trim, see datasheet
> + */
> +struct arcxcnn_platform_data {
> + const char *name;
> + u16 initial_brightness;
> + u8 leden;
> + u8 led_config_0;
> + u8 led_config_1;
> + u8 dim_freq;
> + u8 comp_config;
> + u8 filter_config;
> + u8 trim_config;
> +};
> +
> +#define ARCXCNN_CMD 0x00 /* Command Register */
> +#define ARCXCNN_CMD_STDBY 0x80 /* I2C Standby */
> +#define ARCXCNN_CMD_RESET 0x40 /* Reset */
> +#define ARCXCNN_CMD_BOOST 0x10 /* Boost */
> +#define ARCXCNN_CMD_OVP_MASK 0x0C /* --- Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_XXV 0x0C /* <rsvrd> Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_20V 0x08 /* 20v Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_24V 0x04 /* 24v Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_31V 0x00 /* 31.4v Over Voltage Threshold */
> +#define ARCXCNN_CMD_EXT_COMP 0x01 /* part (0) or full (1) ext. comp */
> +
> +#define ARCXCNN_CONFIG 0x01 /* Configuration */
> +#define ARCXCNN_STATUS1 0x02 /* Status 1 */
> +#define ARCXCNN_STATUS2 0x03 /* Status 2 */
> +#define ARCXCNN_FADECTRL 0x04 /* Fading Control */
> +#define ARCXCNN_ILED_CONFIG 0x05 /* ILED Configuration */
> +#define ARCXCNN_ILED_DIM_PWM 0x00 /* config dim mode pwm */
> +#define ARCXCNN_ILED_DIM_INT 0x04 /* config dim mode internal */
> +#define ARCXCNN_LEDEN 0x06 /* LED Enable Register */
> +#define ARCXCNN_LEDEN_ISETEXT 0x80 /* Full-scale current set extern */
> +#define ARCXCNN_LEDEN_MASK 0x3F /* LED string enables mask */
> +#define ARCXCNN_LEDEN_BITS 0x06 /* Bits of LED string enables */
> +#define ARCXCNN_LEDEN_LED1 0x01
> +#define ARCXCNN_LEDEN_LED2 0x02
> +#define ARCXCNN_LEDEN_LED3 0x04
> +#define ARCXCNN_LEDEN_LED4 0x08
> +#define ARCXCNN_LEDEN_LED5 0x10
> +#define ARCXCNN_LEDEN_LED6 0x20
> +
> +#define ARCXCNN_WLED_ISET_LSB 0x07 /* LED ISET LSB (in upper nibble) */
> +#define ARCXCNN_WLED_ISET_LSB_SHIFT 0x04 /* ISET LSB Left Shift */
> +#define ARCXCNN_WLED_ISET_MSB 0x08 /* LED ISET MSB (8 bits) */
> +
> +#define ARCXCNN_DIMFREQ 0x09
> +#define ARCXCNN_COMP_CONFIG 0x0A
> +#define ARCXCNN_FILT_CONFIG 0x0B
> +#define ARCXCNN_IMAXTUNE 0x0C
> +#define ARCXCNN_ID_MSB 0x1E
> +#define ARCXCNN_ID_LSB 0x1F
> +
> +#define MAX_BRIGHTNESS 4095
> +#define INIT_BRIGHT 60
> +
> +struct arcxcnn {
> + struct i2c_client *client;
> + struct backlight_device *bl;
> + struct device *dev;
> + struct arcxcnn_platform_data *pdata;
> +};
> +
> +static int arcxcnn_update_field(struct arcxcnn *lp, u8 reg, u8 mask, u8 data)
> +{
> + int ret;
> + u8 tmp;
> +
> + ret = i2c_smbus_read_byte_data(lp->client, reg);
> + if (ret < 0) {
> + dev_err(lp->dev, "failed to read 0x%.2x\n", reg);
> + return ret;
> + }
> +
> + tmp = (u8)ret;
> + tmp &= ~mask;
> + tmp |= data & mask;
> +
> + return i2c_smbus_write_byte_data(lp->client, reg, tmp);
> +}
> +
> +static int arcxcnn_set_brightness(struct arcxcnn *lp, u32 brightness)
> +{
> + int ret;
> + u8 val;
> +
> + /* lower nibble of brightness goes in upper nibble of LSB register */
> + val = (brightness & 0xF) << ARCXCNN_WLED_ISET_LSB_SHIFT;
> + ret = i2c_smbus_write_byte_data(lp->client,
> + ARCXCNN_WLED_ISET_LSB, val);
> + if (ret < 0)
> + return ret;
> +
> + /* remaining 8 bits of brightness go in MSB register */
> + val = (brightness >> 4);
> + return i2c_smbus_write_byte_data(lp->client,
> + ARCXCNN_WLED_ISET_MSB, val);
> +}
> +
> +static int arcxcnn_bl_update_status(struct backlight_device *bl)
> +{
> + struct arcxcnn *lp = bl_get_data(bl);
> + u32 brightness = bl->props.brightness;
> + int ret;
> +
> + if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> + brightness = 0;
> +
> + ret = arcxcnn_set_brightness(lp, brightness);
> + if (ret)
> + return ret;
> +
> + /* set power-on/off/save modes */
> + return arcxcnn_update_field(lp, ARCXCNN_CMD, ARCXCNN_CMD_STDBY,
> + (bl->props.power == 0) ? 0 : ARCXCNN_CMD_STDBY);
> +}
> +
> +static const struct backlight_ops arcxcnn_bl_ops = {
> + .options = BL_CORE_SUSPENDRESUME,
> + .update_status = arcxcnn_bl_update_status,
> +};
> +
> +static int arcxcnn_backlight_register(struct arcxcnn *lp)
> +{
> + struct backlight_properties *props;
> + const char *name = lp->pdata->name ? : "arctic_bl";
> +
> + props = devm_kzalloc(lp->dev, sizeof(*props), GFP_KERNEL);
> + if (!props)
> + return -ENOMEM;
> +
> + props->type = BACKLIGHT_PLATFORM;
> + props->max_brightness = MAX_BRIGHTNESS;
> +
> + if (lp->pdata->initial_brightness > props->max_brightness)
> + lp->pdata->initial_brightness = props->max_brightness;
> +
> + props->brightness = lp->pdata->initial_brightness;
> +
> + lp->bl = devm_backlight_device_register(lp->dev, name, lp->dev, lp,
> + &arcxcnn_bl_ops, props);
> + return PTR_ERR_OR_ZERO(lp->bl);
> +}
> +
> +static void arcxcnn_parse_dt(struct arcxcnn *lp)
> +{
> + struct device *dev = lp->dev;
> + struct device_node *node = dev->of_node;
> + u32 prog_val, num_entry, entry, sources[ARCXCNN_LEDEN_BITS];
> + int ret;
> +
> + /* device tree entry isn't required, defaults are OK */
> + if (!node)
> + return;
> +
> + ret = of_property_read_string(node, "label", &lp->pdata->name);
> + if (ret < 0)
> + lp->pdata->name = NULL;
> +
> + ret = of_property_read_u32(node, "default-brightness", &prog_val);
> + if (ret == 0)
> + lp->pdata->initial_brightness = prog_val;
> +
> + ret = of_property_read_u32(node, "arc,led-config-0", &prog_val);
> + if (ret == 0)
> + lp->pdata->led_config_0 = (u8)prog_val;
> +
> + ret = of_property_read_u32(node, "arc,led-config-1", &prog_val);
> + if (ret == 0)
> + lp->pdata->led_config_1 = (u8)prog_val;
> +
> + ret = of_property_read_u32(node, "arc,dim-freq", &prog_val);
> + if (ret == 0)
> + lp->pdata->dim_freq = (u8)prog_val;
> +
> + ret = of_property_read_u32(node, "arc,comp-config", &prog_val);
> + if (ret == 0)
> + lp->pdata->comp_config = (u8)prog_val;
> +
> + ret = of_property_read_u32(node, "arc,filter-config", &prog_val);
> + if (ret == 0)
> + lp->pdata->filter_config = (u8)prog_val;
> +
> + ret = of_property_read_u32(node, "arc,trim-config", &prog_val);
> + if (ret == 0)
> + lp->pdata->trim_config = (u8)prog_val;
> +
> + ret = of_property_count_u32_elems(node, "led-sources");
> + if (ret < 0) {
> + lp->pdata->leden = ARCXCNN_LEDEN_MASK; /* all on is default */
> + } else {
> + num_entry = ret;
> + if (num_entry > ARCXCNN_LEDEN_BITS)
> + num_entry = ARCXCNN_LEDEN_BITS;
> +
> + ret = of_property_read_u32_array(node, "led-sources", sources,
> + num_entry);
> + if (ret < 0) {
> + dev_err(dev, "led-sources node is invalid.\n");
> + return;
> + }
> +
> + lp->pdata->leden = 0;
> +
> + /* for each enable in source, set bit in led enable */
> + for (entry = 0; entry < num_entry; entry++) {
> + u8 onbit = 1 << sources[entry];
> +
> + lp->pdata->leden |= onbit;
> + }
> + }
> +}
> +
> +static int arcxcnn_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> +{
> + struct arcxcnn *lp;
> + int ret;
> +
> + if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> + return -EIO;
> +
> + lp = devm_kzalloc(&cl->dev, sizeof(*lp), GFP_KERNEL);
> + if (!lp)
> + return -ENOMEM;
> +
> + lp->client = cl;
> + lp->dev = &cl->dev;
> + lp->pdata = dev_get_platdata(&cl->dev);
> +
> + /* reset the device */
> + ret = i2c_smbus_write_byte_data(lp->client,
> + ARCXCNN_CMD, ARCXCNN_CMD_RESET);
> + if (ret)
> + goto probe_err;
> +
> + if (!lp->pdata) {
> + lp->pdata = devm_kzalloc(lp->dev,
> + sizeof(*lp->pdata), GFP_KERNEL);
> + if (!lp->pdata)
> + return -ENOMEM;
> +
> + /* Setup defaults based on power-on defaults */
> + lp->pdata->name = NULL;
> + lp->pdata->initial_brightness = INIT_BRIGHT;
> + lp->pdata->leden = ARCXCNN_LEDEN_MASK;
> +
> + lp->pdata->led_config_0 = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_FADECTRL);
> +
> + lp->pdata->led_config_1 = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_ILED_CONFIG);
> + /* insure dim mode is not default pwm */
> + lp->pdata->led_config_1 |= ARCXCNN_ILED_DIM_INT;
> +
> + lp->pdata->dim_freq = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_DIMFREQ);
> +
> + lp->pdata->comp_config = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_COMP_CONFIG);
> +
> + lp->pdata->filter_config = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_FILT_CONFIG);
> +
> + lp->pdata->trim_config = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_IMAXTUNE);
> +
> + if (IS_ENABLED(CONFIG_OF))
> + arcxcnn_parse_dt(lp);
> + }
> +
> + i2c_set_clientdata(cl, lp);
> +
> + /* constrain settings to what is possible */
> + if (lp->pdata->initial_brightness > MAX_BRIGHTNESS)
> + lp->pdata->initial_brightness = MAX_BRIGHTNESS;
> +
> + /* set initial brightness */
> + ret = arcxcnn_set_brightness(lp, lp->pdata->initial_brightness);
> + if (ret)
> + goto probe_err;
> +
> + /* set other register values directly */
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_FADECTRL,
> + lp->pdata->led_config_0);
> + if (ret)
> + goto probe_err;
> +
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_ILED_CONFIG,
> + lp->pdata->led_config_1);
> + if (ret)
> + goto probe_err;
> +
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_DIMFREQ,
> + lp->pdata->dim_freq);
> + if (ret)
> + goto probe_err;
> +
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_COMP_CONFIG,
> + lp->pdata->comp_config);
> + if (ret)
> + goto probe_err;
> +
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_FILT_CONFIG,
> + lp->pdata->filter_config);
> + if (ret)
> + goto probe_err;
> +
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_IMAXTUNE,
> + lp->pdata->trim_config);
> + if (ret)
> + goto probe_err;
> +
> + /* set initial LED Enables */
> + arcxcnn_update_field(lp, ARCXCNN_LEDEN,
> + ARCXCNN_LEDEN_MASK, lp->pdata->leden);
> +
> + ret = arcxcnn_backlight_register(lp);
> + if (ret)
> + goto probe_register_err;
> +
> + backlight_update_status(lp->bl);
> +
> + return 0;
> +
> +probe_register_err:
> + dev_err(lp->dev,
> + "failed to register backlight.\n");
> +
> +probe_err:
> + dev_err(lp->dev,
> + "failure ret: %d\n", ret);
> + return ret;
> +}
> +
> +static int arcxcnn_remove(struct i2c_client *cl)
> +{
> + struct arcxcnn *lp = i2c_get_clientdata(cl);
> +
> + /* disable all strings (ignore errors) */
> + i2c_smbus_write_byte_data(lp->client,
> + ARCXCNN_LEDEN, 0x00);
> + /* reset the device (ignore errors) */
> + i2c_smbus_write_byte_data(lp->client,
> + ARCXCNN_CMD, ARCXCNN_CMD_RESET);
> +
> + lp->bl->props.brightness = 0;
> +
> + backlight_update_status(lp->bl);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id arcxcnn_dt_ids[] = {
> + { .compatible = "arc,arc2c0608" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, arcxcnn_dt_ids);
> +
> +static const struct i2c_device_id arcxcnn_ids[] = {
> + {"arc2c0608", ARC2C0608},
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, arcxcnn_ids);
> +
> +static struct i2c_driver arcxcnn_driver = {
> + .driver = {
> + .name = "arcxcnn_bl",
> + .of_match_table = of_match_ptr(arcxcnn_dt_ids),
> + },
> + .probe = arcxcnn_probe,
> + .remove = arcxcnn_remove,
> + .id_table = arcxcnn_ids,
> +};
> +module_i2c_driver(arcxcnn_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Brian Dodge <bdodge-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>");
> +MODULE_DESCRIPTION("ARCXCNN Backlight driver");
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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 v8 2/3] backlight arcxcnn devicetree bindings for ArcticSand
From: Lee Jones @ 2017-04-25 8:17 UTC (permalink / raw)
To: Olimpiu Dejeu
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
jingoohan1-Re5JQEeQqe8AvxtiuMwx3w, bdodge-eV7fy4qpoLhpLGFMi4vTTA,
joe-6d6DIl74uiNBDgjK7y7TUQ, medasaro-eV7fy4qpoLhpLGFMi4vTTA,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1489607133-7870-2-git-send-email-olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
On Wed, 15 Mar 2017, Olimpiu Dejeu wrote:
> backlight: Add devicetree bindings for the Arctic Sand backlight driver
> This patch provides devicetree bindings for the Arctic Sand
> driver submitted in the previous patch
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Acked-by: Daniel Thompson <daniel.thompson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> ---
> v7 => v8:
> - Version updated to match other patch in set. No other changes.
> v6 => v7:
> - Version updated to match other patch in set. No other changes.
> v5 => v6:
> - Version updated to match other patch in set. No other changes.
> v4 => v5:
> - Added spaces for increased readability per Lee Jones
> v3 => v4:
> - Added spaces for increased readability per Lee Jones
> v2 => v3:
> - Version updated to match other patch in set. No other changes.
> v1 => v2:
> - Version updated to match other patch in set. No other changes.
>
> .../bindings/leds/backlight/arcxcnn_bl.txt | 33 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
Applied, thanks.
> diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
> new file mode 100644
> index 0000000..ecb7731
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
> @@ -0,0 +1,33 @@
> +Binding for ArcticSand arc2c0608 LED driver
> +
> +Required properties:
> +- compatible: should be "arc,arc2c0608"
> +- reg: slave address
> +
> +Optional properties:
> +- default-brightness: brightness value on boot, value from: 0-4095
> +- label: The name of the backlight device
> + See Documentation/devicetree/bindings/leds/common.txt
> +- led-sources: List of enabled channels from 0 to 5.
> + See Documentation/devicetree/bindings/leds/common.txt
> +
> +- arc,led-config-0: setting for register ILED_CONFIG_0
> +- arc,led-config-1: setting for register ILED_CONFIG_1
> +- arc,dim-freq: PWM mode frequence setting (bits [3:0] used)
> +- arc,comp-config: setting for register CONFIG_COMP
> +- arc,filter-config: setting for register FILTER_CONFIG
> +- arc,trim-config: setting for register IMAXTUNE
> +
> +Note: Optional properties not specified will default to values in IC EPROM
> +
> +Example:
> +
> +arc2c0608@30 {
> + compatible = "arc,arc2c0608";
> + reg = <0x30>;
> + default-brightness = <500>;
> + label = "lcd-backlight";
> + linux,default-trigger = "backlight";
> + led-sources = <0 1 2 5>;
> +};
> +
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 2/2] mfd: max7360: Add mfd core device driver
From: Valentin Sitdikov @ 2017-04-25 8:15 UTC (permalink / raw)
To: lee.jones, robh+dt, mark.rutland, devicetree, linux-kernel
Cc: Andrei Dranitca, Valentin Sitdikov
In-Reply-To: <20170425081557.13941-1-valentin_sitdikov@mentor.com>
From: Andrei Dranitca <Andrei_Dranitca@mentor.com>
This patch adds core/irq driver to support MAX7360 i2c chip
which contains keypad, gpio, pwm, gpo and rotary encoder submodules.
Signed-off-by: Valentin Sitdikov <valentin_sitdikov@mentor.com>
Signed-off-by: Andrei Dranitca <Andrei_Dranitca@mentor.com>
---
drivers/mfd/Kconfig | 16 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/max7360.c | 393 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/max7360.h | 130 +++++++++++++++
4 files changed, 540 insertions(+)
create mode 100644 drivers/mfd/max7360.c
create mode 100644 include/linux/mfd/max7360.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 55ecdfb..2227c65 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -688,6 +688,22 @@ config MFD_MAX8998
additional drivers must be enabled in order to use the functionality
of the device.
+config MFD_MAX7360
+ tristate "Maxim Semiconductor MAX7360 support"
+ depends on I2C && OF
+ select MFD_CORE
+ select REGMAP_I2C
+ select IRQ_DOMAIN
+ help
+ Say yes here to add support for Maxim Semiconductor MAX7360.
+ This provides microprocessors with management of up to 64 key switches,
+ with an additional eight LED drivers/GPIOs that feature constant-current,
+ PWM intensity control, and rotary switch control options.
+
+ This driver provides common support for accessing the device,
+ additional drivers must be enabled in order to use the functionality
+ of the device.
+
config MFD_MT6397
tristate "MediaTek MT6397 PMIC Support"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 31ce076..f01f3a1 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -133,6 +133,7 @@ obj-$(CONFIG_MFD_DA9063) += da9063.o
obj-$(CONFIG_MFD_DA9150) += da9150-core.o
obj-$(CONFIG_MFD_MAX14577) += max14577.o
+obj-$(CONFIG_MFD_MAX7360) += max7360.o
obj-$(CONFIG_MFD_MAX77620) += max77620.o
obj-$(CONFIG_MFD_MAX77686) += max77686.o
obj-$(CONFIG_MFD_MAX77693) += max77693.o
diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
new file mode 100644
index 0000000..b4c2fdd
--- /dev/null
+++ b/drivers/mfd/max7360.c
@@ -0,0 +1,393 @@
+/*
+ * Copyright (C) 2017 Mentor Graphics
+ *
+ * Author: Valentin Sitdikov <Valentin.Sitdikov@mentor.com>
+ * Author: Andrei Dranitca <Andrei_Dranitca@mentor.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max7360.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+
+int max7360_request_pin(struct max7360 *max7360, u8 pin)
+{
+ struct i2c_client *client = max7360->i2c;
+ int ret = 0;
+
+ spin_lock(&max7360->lock);
+ if (max7360->gpio_pins & BIT(pin)) {
+ dev_err(&client->dev, "pin %d already requested, mask %x",
+ pin, max7360->gpio_pins);
+ spin_unlock(&max7360->lock);
+ return -EBUSY;
+ }
+ max7360->gpio_pins |= BIT(pin);
+ dev_dbg(&client->dev, "pin %d requested successfully", pin);
+ spin_unlock(&max7360->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(max7360_request_pin);
+
+void max7360_free_pin(struct max7360 *max7360, u8 pin)
+{
+ spin_lock(&max7360->lock);
+ max7360->gpio_pins &= ~BIT(pin);
+ spin_unlock(&max7360->lock);
+}
+EXPORT_SYMBOL_GPL(max7360_free_pin);
+
+static const struct mfd_cell max7360_devices[] = {
+ {
+ .name = "max7360-gpio",
+ .of_compatible = "maxim,max7360-gpio",
+ },
+ {
+ .name = "max7360-keypad",
+ .of_compatible = "maxim,max7360-keypad",
+ },
+ {
+ .name = "max7360-pwm",
+ .of_compatible = "maxim,max7360-pwm",
+ },
+ {
+ .name = "max7360-rotary",
+ .of_compatible = "maxim,max7360-rotary",
+ },
+};
+
+static irqreturn_t max7360_irq(int irq, void *data)
+{
+ struct max7360 *max7360 = data;
+ int virq;
+
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+ handle_nested_irq(virq);
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+ handle_nested_irq(virq);
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+ handle_nested_irq(virq);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqi(int irq, void *data)
+{
+ struct max7360 *max7360 = data;
+ int virq;
+
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+ handle_nested_irq(virq);
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+ handle_nested_irq(virq);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqk(int irq, void *data)
+{
+ struct max7360 *max7360 = data;
+ int virq;
+
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+ handle_nested_irq(virq);
+
+ return IRQ_HANDLED;
+}
+
+static int max7360_irq_map(struct irq_domain *d, unsigned int virq,
+ irq_hw_number_t hwirq)
+{
+ struct max7360 *max7360 = d->host_data;
+
+ irq_set_chip_data(virq, max7360);
+ irq_set_chip_and_handler(virq, &dummy_irq_chip,
+ handle_edge_irq);
+ irq_set_nested_thread(virq, 1);
+ irq_set_noprobe(virq);
+
+ return 0;
+}
+
+static void max7360_irq_unmap(struct irq_domain *d, unsigned int virq)
+{
+ irq_set_chip_and_handler(virq, NULL, NULL);
+ irq_set_chip_data(virq, NULL);
+}
+
+static const struct irq_domain_ops max7360_irq_ops = {
+ .map = max7360_irq_map,
+ .unmap = max7360_irq_unmap,
+ .xlate = irq_domain_xlate_onecell,
+};
+
+static int max7360_irq_init(struct max7360 *max7360, struct device_node *np)
+{
+ int ret;
+
+ max7360->inti = of_irq_get_byname(np, "inti");
+ max7360->intk = of_irq_get_byname(np, "intk");
+
+ if (max7360->inti < 0 || max7360->intk < 0) {
+ max7360->shared_irq = of_irq_get_byname(np, "int-shared");
+
+ if (max7360->shared_irq < 0) {
+ dev_err(max7360->dev, "failed to find IRQ in dts\n");
+ return -EINVAL;
+ }
+
+ ret = request_threaded_irq(max7360->shared_irq, NULL,
+ max7360_irq,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "max7360", max7360);
+ if (ret) {
+ dev_err(max7360->dev, "failed to request IRQ: %d\n",
+ ret);
+ return ret;
+ }
+ } else {
+ max7360->shared_irq = 0;
+ ret = request_threaded_irq(max7360->inti, NULL, max7360_irqi,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "max7360", max7360);
+ if (ret) {
+ dev_err(max7360->dev, "failed to request inti IRQ: %d\n",
+ ret);
+ return ret;
+ }
+
+ ret = request_threaded_irq(max7360->intk, NULL, max7360_irqk,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "max7360", max7360);
+ if (ret) {
+ free_irq(max7360->inti, max7360);
+ dev_err(max7360->dev, "failed to request intk IRQ: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ max7360->domain = irq_domain_add_simple(np, MAX7360_NR_INTERNAL_IRQS,
+ 0, &max7360_irq_ops, max7360);
+
+ if (!max7360->domain) {
+ if (max7360->shared_irq)
+ free_irq(max7360->shared_irq, max7360);
+ else {
+ free_irq(max7360->inti, max7360);
+ free_irq(max7360->intk, max7360);
+ }
+ dev_err(max7360->dev, "Failed to create irqdomain\n");
+ return -ENODEV;
+ }
+
+ irq_create_mapping(max7360->domain, MAX7360_INT_GPIO);
+ irq_create_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+ irq_create_mapping(max7360->domain, MAX7360_INT_ROTARY);
+
+ return 0;
+}
+
+void max7360_fall_deepsleep(struct max7360 *max7360)
+{
+ max7360_write_reg(max7360, MAX7360_REG_SLEEP, MAX7360_AUTOSLEEP_8192);
+}
+EXPORT_SYMBOL_GPL(max7360_fall_deepsleep);
+
+void max7360_take_catnap(struct max7360 *max7360)
+{
+ max7360_write_reg(max7360, MAX7360_REG_SLEEP, MAX7360_AUTOSLEEP_256);
+}
+EXPORT_SYMBOL_GPL(max7360_take_catnap);
+
+static int max7360_chip_init(struct max7360 *max7360)
+{
+ max7360->gpio_pins = MAX7360_MAX_GPIO;
+ max7360->gpo_count = 0;
+ max7360->col_count = MAX7360_COL_GPO_PINS;
+ return 0;
+}
+
+static int max7360_device_init(struct max7360 *max7360)
+{
+ int ret = 0;
+
+ ret = mfd_add_devices(max7360->dev, -1, max7360_devices,
+ ARRAY_SIZE(max7360_devices), NULL,
+ 0, max7360->domain);
+ if (ret)
+ dev_err(max7360->dev, "failed to add child devices\n");
+
+ return ret;
+}
+
+int max7360_request_gpo_pin_count(struct max7360 *max7360, u8 count)
+{
+ if (count > MAX7360_MAX_GPO)
+ return -EINVAL;
+ if (max7360->col_count + count > MAX7360_COL_GPO_PINS) {
+ dev_err(max7360->dev,
+ "trying to request %d pins as gpo while %d pins already used as COL\n",
+ count, max7360->col_count);
+ return -EINVAL;
+ }
+ max7360->gpo_count = count;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(max7360_request_gpo_pin_count);
+
+int max7360_request_col_count(struct max7360 *max7360, u8 count)
+{
+ if (max7360->gpo_count + count > MAX7360_COL_GPO_PINS) {
+ dev_err(max7360->dev,
+ "trying to request %d pins as COL while %d pins already used as gpo\n",
+ count, max7360->gpo_count);
+ return -EINVAL;
+ }
+ max7360->col_count = count;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(max7360_request_col_count);
+
+static const struct regmap_range max7360_volatile_ranges[] = {
+ {
+ .range_min = MAX7360_REG_KEYFIFO,
+ .range_max = MAX7360_REG_KEYFIFO,
+ }, {
+ .range_min = 0x48,
+ .range_max = 0x4a,
+ },
+};
+
+static const struct regmap_access_table max7360_volatile_table = {
+ .yes_ranges = max7360_volatile_ranges,
+ .n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
+};
+
+static const struct regmap_config max7360_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0xff,
+ .volatile_table = &max7360_volatile_table,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int max7360_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ struct device_node *np = i2c->dev.of_node;
+ struct max7360 *max7360;
+
+ int ret;
+
+ max7360 = devm_kzalloc(&i2c->dev, sizeof(struct max7360),
+ GFP_KERNEL);
+ if (!max7360)
+ return -ENOMEM;
+
+ spin_lock_init(&max7360->lock);
+
+ max7360->dev = &i2c->dev;
+ max7360->i2c = i2c;
+
+ i2c_set_clientdata(i2c, max7360);
+
+ max7360->regmap = devm_regmap_init_i2c(i2c, &max7360_regmap_config);
+ ret = max7360_chip_init(max7360);
+ if (ret)
+ return ret;
+
+ ret = max7360_irq_init(max7360, np);
+ if (ret)
+ return ret;
+
+ ret = max7360_device_init(max7360);
+ if (ret) {
+ dev_err(max7360->dev, "failed to add child devices\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int max7360_remove(struct i2c_client *client)
+{
+ struct max7360 *max7360 = i2c_get_clientdata(client);
+
+ mfd_remove_devices(max7360->dev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int max7360_suspend(struct device *dev)
+{
+ return 0;
+}
+
+static int max7360_resume(struct device *dev)
+{
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(max7360_dev_pm_ops, max7360_suspend, max7360_resume);
+
+static const struct of_device_id max7360_match[] = {
+ { .compatible = "maxim,max7360" },
+ { }
+};
+
+MODULE_DEVICE_TABLE(of, max7360_match);
+
+static const struct i2c_device_id max7360_id[] = {
+ { "max7360", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max7360_id);
+
+static struct i2c_driver max7360_driver = {
+ .driver = {
+ .name = "max7360",
+ .pm = &max7360_dev_pm_ops,
+ .of_match_table = max7360_match,
+ },
+ .probe = max7360_probe,
+ .remove = max7360_remove,
+ .id_table = max7360_id,
+};
+
+static int __init max7360_init(void)
+{
+ return i2c_add_driver(&max7360_driver);
+}
+subsys_initcall(max7360_init);
+
+static void __exit max7360_exit(void)
+{
+ i2c_del_driver(&max7360_driver);
+}
+module_exit(max7360_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MAX7360 MFD core driver");
diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
new file mode 100644
index 0000000..d139ddd
--- /dev/null
+++ b/include/linux/mfd/max7360.h
@@ -0,0 +1,130 @@
+#ifndef __LINUX_MFD_MAX7360_H
+#define __LINUX_MFD_MAX7360_H
+#include <linux/regmap.h>
+
+#define MAX7360_MAX_KEY_ROWS 8
+#define MAX7360_MAX_KEY_COLS 8
+#define MAX7360_MAX_KEY_NUM (MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
+#define MAX7360_ROW_SHIFT 3
+
+#define MAX7360_MAX_GPIO 8
+#define MAX7360_MAX_GPO 6
+#define MAX7360_COL_GPO_PINS 8
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_KEYFIFO 0x00
+#define MAX7360_REG_CONFIG 0x01
+#define MAX7360_REG_DEBOUNCE 0x02
+#define MAX7360_REG_INTERRUPT 0x03
+#define MAX7360_REG_PORTS 0x04
+#define MAX7360_REG_KEYREP 0x05
+#define MAX7360_REG_SLEEP 0x06
+
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_GPIOCFG 0x40
+#define MAX7360_REG_GPIOCTRL 0x41
+#define MAX7360_REG_GPIODEB 0x42
+#define MAX7360_REG_GPIOCURR 0x43
+#define MAX7360_REG_GPIOOUTM 0x44
+#define MAX7360_REG_PWMCOM 0x45
+#define MAX7360_REG_RTRCFG 0x46
+#define MAX7360_REG_GPIOIN 0x49
+#define MAX7360_REG_RTR_CNT 0x4A
+#define MAX7360_REG_PWMBASE 0x50
+#define MAX7360_REG_PWMCFG 0x58
+
+
+#define MAX7360_REG_PORTCFGBASE 0x58
+
+/*
+ * Configuration register bits
+ */
+#define MAX7360_CFG_SLEEP (1 << 7)
+#define MAX7360_CFG_INTERRUPT (1 << 5)
+#define MAX7360_CFG_KEY_RELEASE (1 << 3)
+#define MAX7360_CFG_WAKEUP (1 << 1)
+#define MAX7360_CFG_TIMEOUT (1 << 0)
+
+/*
+ * Autosleep register values (ms)
+ */
+#define MAX7360_AUTOSLEEP_8192 0x01
+#define MAX7360_AUTOSLEEP_4096 0x02
+#define MAX7360_AUTOSLEEP_2048 0x03
+#define MAX7360_AUTOSLEEP_1024 0x04
+#define MAX7360_AUTOSLEEP_512 0x05
+#define MAX7360_AUTOSLEEP_256 0x06
+
+#define MAX7360_INT_INTI 0
+#define MAX7360_INT_INTK 1
+
+#define MAX7360_INT_GPIO 0
+#define MAX7360_INT_KEYPAD 1
+#define MAX7360_INT_ROTARY 2
+
+#define MAX7360_NR_INTERNAL_IRQS 3
+
+struct max7360 {
+ spinlock_t lock; /* lock access to the structure */
+ struct device *dev;
+ struct i2c_client *i2c;
+ struct irq_domain *domain;
+ struct regmap *regmap;
+
+ int irq_base;
+ int num_gpio;
+ int shared_irq;
+ int inti;
+ int intk;
+ u8 gpio_pins;
+ u8 col_count;
+ u8 gpo_count;
+};
+
+static inline int max7360_read_reg(struct max7360 *max7360, int reg)
+{
+ unsigned int ival;
+ int ret;
+
+ ret = regmap_read(max7360->regmap, reg, &ival);
+ if (!ret)
+ return ival;
+ return 0;
+}
+
+static inline int max7360_write_reg(struct max7360 *max7360, u8 reg, u8 val)
+{
+ return regmap_write(max7360->regmap, reg, val);
+}
+
+static inline int max7360_set_bits(struct max7360 *max7360, u8 reg,
+ unsigned int bit_mask)
+{
+ return regmap_update_bits(max7360->regmap, reg, bit_mask, bit_mask);
+}
+
+static inline int max7360_clr_bits(struct max7360 *max7360, u8 reg,
+ unsigned int bit_mask)
+{
+ return regmap_update_bits(max7360->regmap, reg, bit_mask, 0);
+}
+
+static inline int max7360_update(struct max7360 *max7360, u8 reg, u8 val,
+ unsigned int bit_mask)
+{
+ return regmap_update_bits(max7360->regmap, reg, bit_mask, val);
+}
+
+int max7360_request_pin(struct max7360 *max7360, u8 pin);
+void max7360_free_pin(struct max7360 *max7360, u8 pin);
+
+void max7360_take_catnap(struct max7360 *max7360);
+void max7360_fall_deepsleep(struct max7360 *max7360);
+
+int max7360_request_gpo_pin_count(struct max7360 *max7360, u8 count);
+int max7360_request_col_count(struct max7360 *max7360, u8 count);
+
+#endif
--
2.9.3
^ permalink raw reply related
* [PATCH 1/2] Add DT bindings documentation for the max7360 mfd driver
From: Valentin Sitdikov @ 2017-04-25 8:15 UTC (permalink / raw)
To: lee.jones-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Valentin Sitdikov, Andrei Dranitca
In-Reply-To: <20170425081557.13941-1-valentin_sitdikov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Valentin Sitdikov <valentin_sitdikov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Andrei Dranitca <Andrei_Dranitca-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
Documentation/devicetree/bindings/mfd/max7360.txt | 72 +++++++++++++++++++++++
1 file changed, 72 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/max7360.txt
diff --git a/Documentation/devicetree/bindings/mfd/max7360.txt b/Documentation/devicetree/bindings/mfd/max7360.txt
new file mode 100644
index 0000000..359073a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max7360.txt
@@ -0,0 +1,72 @@
+* Maxim MAX7360 multi-function device
+
+The Maxim MAX7360 is a multifunction device which includes
+64 key switches, eight LED drivers/GPIOs, PWM intensity control,
+and rotary switch control.
+
+Required properties:
+- compatible: Should be the following: "maxim,max7360"
+- reg: Specifies the i2c slave address of the max7360 block. It can be 0x38, 0x3a, 0x3c or 0x3e IIUC.
+
+Optional properties:
+- interrupt-parent: Specifies the phandle of the interrupt controller to which
+ the interrupts from MAX7360 are routed to.
+- interrupt-names: might be "int-shared" or list of "inti" and "intk"
+- interrupt-controller: Identifies the device as an interrupt controller.
+- #interrupt-cells : Number of cells to encode an interrupt source, shall be 1.
+
+Examples:
+
+Without subnodes:
+ max7360@38 {
+ compatible = "maxim,max7360";
+ reg = <0x38>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "int-shared";
+ interrupt-controller;
+ #interrupt-cells = <0x1>;
+
+ };
+
+With subnodes:
+ max7360@38 {
+ compatible = "maxim,max7360";
+ reg = <0x38>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "int-shared";
+ interrupt-controller;
+ #interrupt-cells = <0x1>;
+
+ max7360_gpio: max7360_gpio@0 {
+ compatible = "maxim,max7360-gpio";
+ gpio-controller;
+ #gpio-cells = <0x2>;
+ interrupt-controller;
+ #interrupt-cells = <0x2>;
+ interrupts = <0>;
+ };
+
+ max7360_keypad {
+ compatible = "maxim,max7360-keypad";
+ maxim,debounce_reg = /bits/ 8 <0xef>;
+ maxim,ports_reg = /bits/ 8 <0xae>;
+ linux,keymap = < MATRIX_KEY(0, 0, KEY_F5)
+ MATRIX_KEY(1, 0, KEY_F4) >;
+ keypad,num-rows = <2>;
+ keypad,num-columns = <1>;
+ interrupts = <1>;
+ };
+
+ max7360_pwm: max7360_pwm {
+ compatible = "maxim,max7360-pwm";
+ #pwm-cells = <0x2>;
+ };
+
+ max7360_rotary_encoder {
+ compatible = "maxim,max7360-rotary";
+ interrupts = <2>;
+ };
+
+ };
--
2.9.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
* [PATCH 0/2] Add max7360 mfd driver and DT documentation
From: Valentin Sitdikov @ 2017-04-25 8:15 UTC (permalink / raw)
To: lee.jones-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Valentin Sitdikov
This series add initial support of mfd core driver for max7360 chip
Andrei Dranitca (1):
mfd: max7360: Add mfd core device driver
Valentin Sitdikov (1):
Add DT bindings documentation for the max7360 mfd driver
Documentation/devicetree/bindings/mfd/max7360.txt | 72 ++++
drivers/mfd/Kconfig | 16 +
drivers/mfd/Makefile | 1 +
drivers/mfd/max7360.c | 393 ++++++++++++++++++++++
include/linux/mfd/max7360.h | 130 +++++++
5 files changed, 612 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/max7360.txt
create mode 100644 drivers/mfd/max7360.c
create mode 100644 include/linux/mfd/max7360.h
--
2.9.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
* Re: [PATCH 1/5 v3] usb: host: add DT bindings for faraday fotg2
From: Linus Walleij @ 2017-04-25 8:12 UTC (permalink / raw)
To: Hans Ulli Kroll
Cc: openwrt-devel, devicetree@vger.kernel.org, Paulius Zaleckas,
Greg Kroah-Hartman, linux-usb@vger.kernel.org, Janos Laube,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <alpine.LNX.2.00.1704241846300.15211@T420s>
On Mon, Apr 24, 2017 at 6:53 PM, Hans Ulli Kroll
<ulli.kroll@googlemail.com> wrote:
> Got NAK'ed from Rob on some ealier round due missing "device mode" on this
> IP. I've blatantly overrided this to a host only driver.
>
> These are the needed changes in DT to support both modes
> Note the -dr at the end of fotg210, to reflect this in an dual role device
OK I understood the discussion such that the compatible should
simply be ""faraday,fotg210" as that is the name of the hardware
IP block. This is the name of the hardware name in the Faraday
page:
http://www.faraday-tech.com/html/Product/IPProduct/InterfaceIP/USB2_0.htm
Any other string implies how it is used, so that was what I understood
as the reason to reject it with the "-hcd" (host controller device) suffix.
> +- dr_mode : indicates the working mode for "fotg210-dr" compatible
> + controllers. Can be "host", "peripheral". Default to
> + "host" if not defined for backward compatibility.
This seems right, so it is part of the generic bindings, correct?
> usb@68000000 {
> - compatible = "cortina,gemini-usb", "faraday,fotg210";
> + compatible = "cortina,gemini-usb", "faraday,fotg210-dr";
But this would be wrong, because the compatible should only
indicate what kind of hardware it is, not how it is going to be used
(whether as host only, slave only or dual-role (OTG).
I hope I didn't get anything wrong here :/
Yours,
Linus Walleij
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
^ permalink raw reply
* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
From: Geert Uytterhoeven @ 2017-04-25 8:09 UTC (permalink / raw)
To: Jiada Wang
Cc: Mark Rutland, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Rob Herring, linux-spi, Mark Brown,
Sascha Hauer, Fabio Estevam, Shawn Guo,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <58FF0127.7070703@mentor.com>
Hi Jiada,
On Tue, Apr 25, 2017 at 9:56 AM, Jiada Wang <jiada_wang@mentor.com> wrote:
> On 04/24/2017 06:10 AM, Geert Uytterhoeven wrote:
>> On Mon, Apr 24, 2017 at 2:48 PM, Jiada Wang<jiada_wang@mentor.com> wrote:
>>> On 04/24/2017 03:55 AM, Geert Uytterhoeven wrote:
>>>> On Fri, Apr 14, 2017 at 7:39 AM, Jiada Wang<jiada_wang@mentor.com>
>>>> wrote:
>>>>> Our use case is to use spidev as an interface to communicate with
>>>>> external
>>>>> SPI master devices.
>>>>> meanwhile the SPI bus controller can also act as master device to send
>>>>> data
>>>>> to other
>>>>> SPI slave devices on the board.
>>>>
>>>> That sounds a bit hackish to me. SPI was never meant to be a
>>>> multi-master
>>>> bus.
>>>> While it can be done, you will need external synchronization (signals)
>>>> to
>>>> avoid conflicts between the SPI masters.
>>>
>>> It doesn't need to be a multi-master bus,
>>> for example A is master device for slave device B.
>>> while B has its own slave device C
>>> for each SPI connection A<=> B, and B<=> C, there is only one master
>>> device.
>>>
>>> and I think from use case point of view, it's very normal,
>>> one CPU upon receives command from external SPI master device,
>>> it writes data to its own slave device (EEPROM) connected to it.
>>
>> So "A<=> B" and "B<=> C" are two distinct SPI buses?
>> Or do they share some signals?
>>
>> Your comment seems to suggest otherwise:
>
> the use case of
> "A (master) <=> B (slave)", "B (master) <=> C(slave)", do share MISO and
> MOSI lines,
> but there is no SS line between A and C. so for each SPI slave device, there
> is only one
> master device.
Do you share CLK, too? Then you need a slave select from B to C.
If you use a separate clock, the slave select from B to C can be optional.
> so I think the question becomes whether the above mentioned hardware setup
> is valid or not.
It's a non-conventional SPI bus setup, but it can work, provided you have
some form of synchronization between A and B.
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: [PATCH RFC 0/5] *** SPI Slave mode support ***
From: Uwe Kleine-König @ 2017-04-25 8:07 UTC (permalink / raw)
To: Jiada Wang
Cc: Mark Rutland, devicetree@vger.kernel.org, Mark Brown,
linux-kernel@vger.kernel.org, linux-spi, Rob Herring,
Geert Uytterhoeven, Sascha Hauer, Fabio Estevam, Shawn Guo,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <58FF0127.7070703@mentor.com>
Hello Jiada,
On Tue, Apr 25, 2017 at 12:56:23AM -0700, Jiada Wang wrote:
> the use case of
> "A (master) <=> B (slave)", "B (master) <=> C(slave)", do share MISO and
> MOSI lines,
> but there is no SS line between A and C. so for each SPI slave device, there
> is only one
> master device.
So you need a mutex to make A not use the bus while B communicates to C.
Otherwise you have two drivers on MOSI (A and B) and MISO (B and C).
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH] PM / Domains: Fix DT example
From: Ulf Hansson @ 2017-04-25 8:01 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Kevin Hilman, linaro-kernel,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
Vincent Guittot, devicetree@vger.kernel.org
In-Reply-To: <533b52e0ea175bf6bb893370c7f8c0309aae235a.1493104411.git.viresh.kumar@linaro.org>
On 25 April 2017 at 09:18, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> The power-domain provider's #power-domain-cells field is set to 0 and
> yet the children is using an index to point the power domain. Fix it by
> removing the index field.
>
> Fixes: 70bb510e4279 ("dt/bindings / PM/Domains: Update binding for PM domain idle states")
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> Documentation/devicetree/bindings/power/power_domain.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index 940707d095cc..14bd9e945ff6 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -81,7 +81,7 @@ domain provided by the 'parent' power controller.
> child: power-controller@12341000 {
> compatible = "foo,power-controller";
> reg = <0x12341000 0x1000>;
> - power-domains = <&parent 0>;
> + power-domains = <&parent>;
> #power-domain-cells = <0>;
> domain-idle-states = <&DOMAIN_PWR_DN>;
> };
> --
> 2.12.0.432.g71c3a4f4ba37
>
^ 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