Re: [v3, 3/4] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller

[Karthik Ramasubramanian]

On 3/7/2018 2:16 PM, Doug Anderson wrote:

Hi,

On Tue, Feb 27, 2018 at 5:38 PM, Karthikeyan Ramasubramanian
 wrote:

This bus driver supports the GENI based i2c hardware controller in the
Qualcomm SOCs. The Qualcomm Generic Interface (GENI) is a programmable
module supporting a wide range of serial interfaces including I2C. The
driver supports FIFO mode and DMA mode of transfer and switches modes
dynamically depending on the size of the transfer.

Signed-off-by: Karthikeyan Ramasubramanian 
Signed-off-by: Sagar Dharia 
Signed-off-by: Girish Mahadevan 
---
  drivers/i2c/busses/Kconfig |  11 +
  drivers/i2c/busses/Makefile|   1 +
  drivers/i2c/busses/i2c-qcom-geni.c | 626 +
  3 files changed, 638 insertions(+)


I'm not an expert on geni (and, to be honest, I haven't read the main
geni patch yet).  ...but I figured I could at least add my $0.02 since
I've stared at i2c bus drivers a lot in the past.  Feel free to tell
me if I'm full or crap...



  create mode 100644 drivers/i2c/busses/i2c-qcom-geni.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index e2954fb..1ddf5cd 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -848,6 +848,17 @@ config I2C_PXA_SLAVE
   is necessary for systems where the PXA may be a target on the
   I2C bus.

+config I2C_QCOM_GENI
+   tristate "Qualcomm Technologies Inc.'s GENI based I2C controller"
+   depends on ARCH_QCOM
+   depends on QCOM_GENI_SE
+   help
+ If you say yes to this option, support will be included for the
+ built-in I2C interface on the Qualcomm Technologies Inc.'s SoCs.


Kind of a generic description and this driver is only for new SoCs,
right?  Maybe make it a little more specific?

Ok.




+
+ This driver can also be built as a module.  If so, the module
+ will be called i2c-qcom-geni.
+
  config I2C_QUP
 tristate "Qualcomm QUP based I2C controller"
 depends on ARCH_QCOM
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 2ce8576..201fce1 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_I2C_PNX) += i2c-pnx.o
  obj-$(CONFIG_I2C_PUV3) += i2c-puv3.o
  obj-$(CONFIG_I2C_PXA)  += i2c-pxa.o
  obj-$(CONFIG_I2C_PXA_PCI)  += i2c-pxa-pci.o
+obj-$(CONFIG_I2C_QCOM_GENI)+= i2c-qcom-geni.o
  obj-$(CONFIG_I2C_QUP)  += i2c-qup.o
  obj-$(CONFIG_I2C_RIIC) += i2c-riic.o
  obj-$(CONFIG_I2C_RK3X) += i2c-rk3x.o
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c 
b/drivers/i2c/busses/i2c-qcom-geni.c
new file mode 100644
index 000..e1e4268
--- /dev/null
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -0,0 +1,626 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SE_I2C_TX_TRANS_LEN0x26c
+#define SE_I2C_RX_TRANS_LEN0x270
+#define SE_I2C_SCL_COUNTERS0x278
+
+#define SE_I2C_ERR  (M_CMD_OVERRUN_EN | M_ILLEGAL_CMD_EN | M_CMD_FAILURE_EN |\
+   M_GP_IRQ_1_EN | M_GP_IRQ_3_EN | M_GP_IRQ_4_EN)
+#define SE_I2C_ABORT   BIT(1)
+
+/* M_CMD OP codes for I2C */
+#define I2C_WRITE  0x1
+#define I2C_READ   0x2
+#define I2C_WRITE_READ 0x3
+#define I2C_ADDR_ONLY  0x4
+#define I2C_BUS_CLEAR  0x6
+#define I2C_STOP_ON_BUS0x7
+/* M_CMD params for I2C */
+#define PRE_CMD_DELAY  BIT(0)
+#define TIMESTAMP_BEFORE   BIT(1)
+#define STOP_STRETCH   BIT(2)
+#define TIMESTAMP_AFTERBIT(3)
+#define POST_COMMAND_DELAY BIT(4)
+#define IGNORE_ADD_NACKBIT(6)
+#define READ_FINISHED_WITH_ACK BIT(7)
+#define BYPASS_ADDR_PHASE  BIT(8)
+#define SLV_ADDR_MSK   GENMASK(15, 9)
+#define SLV_ADDR_SHFT  9
+/* I2C SCL COUNTER fields */
+#define HIGH_COUNTER_MSK   GENMASK(29, 20)
+#define HIGH_COUNTER_SHFT  20
+#define LOW_COUNTER_MSKGENMASK(19, 10)
+#define LOW_COUNTER_SHFT   10
+#define CYCLE_COUNTER_MSK  GENMASK(9, 0)
+
+#define GP_IRQ00
+#define GP_IRQ11
+#define GP_IRQ22
+#define GP_IRQ33
+#define GP_IRQ44
+#define GP_IRQ55
+#define GENI_OVERRUN   6
+#define GENI_ILLEGAL_CMD   7
+#define GENI_ABORT_DONE8
+#define GENI_TIMEOUT   9


Above should be an enum; then use the enum type as the parameter to
geni_i2c_err() so it's obvious that "err" is not a normal linux error
code.



+#define I2C_NACK   GP_IRQ1
+#define I2C_BUS_PROTO  GP_IRQ3
+#define I2C_ARB_LOST   GP_IRQ4


Get rid of 

Re: [v3, 3/4] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller

[Doug Anderson]

Hi,

On Thu, Mar 8, 2018 at 5:06 PM, Sagar Dharia  wrote:
> Hi Doug
>
>
> On 3/8/2018 2:12 PM, Doug Anderson wrote:
>>
>> Hi,
>>
>> On Wed, Mar 7, 2018 at 9:19 PM, Doug Anderson 
>> wrote:
>
> DMA is hard and i2c transfers > 32 bytes are rare.  Do we really gain
> a lot by transferring i2c commands over DMA compared to a FIFO?
> Enough to justify the code complexity and the set of bugs that will
> show up?  I'm sure it will be a controversial assertion given that the
> code's already written, but personally I'd be supportive of ripping
> DMA mode out to simplify the driver.  I'd be curious if anyone else
> agrees.  To me it seems like premature optimization.



 Yes, we have multiple clients (e.g. touch, NFC) using I2C for data
 transfers
 bigger than 32 bytes (some transfers are 100s of bytes). The fifo size
 is
 32, so we can definitely avoid at least 1 interrupt when DMA mode is
 used
 with data size > 32.
>>>
>>>
>>> Does that 1-2 interrupts make any real difference, though?  In theory
>>> it really shouldn't affect the transfer rate.  We should be able to
>>> service the interrupt plenty fast and if we were concerned we would
>>> tweak the watermark code a little bit.  So I guess we're worried about
>>> the extra CPU cycles (and power cost) to service those extra couple
>>> interrupts?
>>>
>>> In theory when touch data is coming in or NFC data is coming in then
>>> we're probably not in a super low power state to begin with.  If it's
>>> touch data we likely want to have the CPU boosted a bunch to respond
>>> to the user quickly.  If we've got 8 cores available all of which can
>>> run at 1GHz or more a few interrupts won't kill us.  NFC data is
>>> probably not common enough that we need to optimize power/CPU
>>> utilizatoin for that.
>>>
>>>
>>> So while i can believe that you do save an interrupt or two, I still
>>> am not convinced that those interrupts are worth a bunch of complex
>>> code (and a whole second code path) to save.
>>>
>>>
>>> ...also note that above you said that coming out of runtime suspend
>>> can take several msec.  That seems like it dwarfs any slight
>>> difference in timing between a FIFO-based operation and DMA.
>>
>>
>> One last note here (sorry for not thinking of this last night) is that
>> I would also be interested in considering _only_ supporting the DMA
>> path.  Is it somehow intrinsically bad to use the DMA flow for a
>> 1-byte transfer?  Is there a bunch of extra overhead or power draw?
>>
>> Specifically my main point is that maintaining two separate flows (the
>> FIFO flow vs the DMA flow) adds complexity, code size, and bugs.  If
>> there's a really good reason to maintain both flows then fine, but we
>> should really consider if this is something that's really giving us
>> value before we agree to it.
>>
>
> FIFO mode gives us 2 advantages:
> 1. small transfers don't have to go through 'dma-map/unmap penalties.
> Some small buffers come from the stack of client caller and the
> dma-map/unmap may fail.
> 2. bring-ups are 'less eventful' (with temp. change to just not have DMA
> mode at all during bring-ups) since SMMU translation/DMA path from QUP
> (master) to memory slave may not always available when critical I2C
> peripherals need to be brought up (e.g. PMIC). CPU to QUP (slave) path
> is usually available.
>
> On the other side, DMA mode has other advantages:
> 1. Multiple android clients are still heavily using I2C in spite of
> faster peripheral buses being available in industry.
> As an example, some multi-finger Touch screens use I2C and the data to
> be transferred per transaction over the bus grows well beyond 70-100
> bytes based on number of fingers. These transactions are very frequent
> when touch is being used, and in an environment where other heavy system
> users are also running (MM/graphics).
> Another example is, NFC uses I2C (as of now) to transfer as much as 700+
> bytes. This can save us 20+ interrupts per transfer.
>
> These transfers are mostly in burst. So the RPMh penalty to resume the
> shared resources is only experienced for very first transfer. Remaining
> transfers in the burst benefit from DMA if they are too big.
>
> Goal here is to have common driver for upstream targets and android and
> android has seen proven advantages with both modes.
> If we end up keeping DMA only for downstream (or FIFO only for
> downstream), then we lose the advantage of having code in upstream since
> we have to maintain downstream patch with other mode.

OK, fair enough.  Having DMA mode alone would be a pain at bringup if
nothing else.  You're right.

I would still argue that perhaps those extra interrupts for FIFO mode
aren't quite as bit of a deal as you're making it out to be.  I've
been on systems that get massive number of interrupts almost
constantly and really it wasn't noticeable.

In any case, I didn't think I'd really convince anyone with this one,
so unless someone

Re: [PATCH v3 4/4] tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP

[Karthik Ramasubramanian]

On 3/8/2018 3:32 PM, Stephen Boyd wrote:

Quoting Karthik Ramasubramanian (2018-03-07 22:06:29)



On 3/6/2018 2:45 PM, Stephen Boyd wrote:

Quoting Karthik Ramasubramanian (2018-03-05 16:51:23)

On 3/2/2018 3:11 PM, Stephen Boyd wrote:


Ok. I've seen similar designs in some mmc drivers. For example, you can
look at drivers/mmc/host/meson-gx-mmc.c and see that they register a
clk_ops and then just start using that clk directly from the driver.
There are also some helper functions for dividers that would hopefully
make the job of calculating the best divider easier.

Thanks for the pointers. I will take a look at it. In the meanwhile I
had discussions with our clock team. They pointed out that the register
to write the divider value here is outside the scope of clock controller
which makes it trickier to implement your suggestion. They are already
in the mailing list and we will discuss further and get back to you in
this regard.


Ok. Let me know if I can help answer any questions.

Ok.




Why are these noirq variants? Please add a comment.

The intention is to enable the console UART port usage as late as
possible in the suspend cycle. Hence noirq variants. I will add this as
a comment. Please let me know if the usage does not match the intention.


Hm.. Does no_console_suspend not work? Or not work well enough?

It works. When console suspend is disabled, the suspend operation does
not get triggered and the resume operation checks if the console suspend
is disabled and performs the needed thing.


Ok so then do we need the noirq variants? Or console suspend is special
enough for this to not matter?
I am a little confused as to whether you prefer the console to not 
suspend at all or you prefer the console suspend at an earlier stage 
than no_irq stage.


If it is former, then with the console_suspend_enabled flag set by 
default this seems the right thing to do. Atleast my understanding is 
that console is expecting the serial port to suspend as well.


If it is latter, then I will check the stage at which suspend_console() 
is initiated and can suspend the serial port after that.



Regards,
Karthik.
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v3, 3/4] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller

[Sagar Dharia]

Hi Doug,

On 3/7/2018 10:19 PM, Doug Anderson wrote:

Hi,

On Wed, Mar 7, 2018 at 6:42 PM, Sagar Dharia  wrote:

Hi Doug,
Thank you for reviewing the patch. I will take a stab at a few comments
below. We will address most of the other comments in next version of I2C
patch.



+
+static const struct geni_i2c_clk_fld geni_i2c_clk_map[] = {
+   {KHz(100), 7, 10, 11, 26},
+   {KHz(400), 2,  5, 12, 24},
+   {KHz(1000), 1, 3,  9, 18},



So I guess this is all relying on an input serial clock of 19.2MHz?
Maybe document that?

Assuming I'm understanding the math here, is it really OK for your
100kHz and 1MHz mode to be running slightly fast?

19200. / 2 / 24


400.0



19200. / 7 / 26


105.49450549450549



19200. / 1 / 18


1066.7



It seems like you'd want the fastest clock that you can make that's
_less than_ the spec.


It would also be interesting to know if it's expected that boards
might need to tweak the t_high / t_low depending on their electrical
characteristics.  In the past I've had lots of requests from board
makers to tweak things because they've got a long trace, or a stronger
or weaker pull, or ...  If so we might later need to add some dts
properties like "i2c-scl-rising-time-ns" and make the math more
dynamic here, unless your hardware somehow automatically adjusts for
this type of thing...
These values are derived by our HW team to comply with the t-high and


t-low specs of I2C. We have confirmed on scope that the frequency of SCL is
indeed less than/equal to the spec. We have not come across slaves who have
needed to tweak these things. We are open to adding these properties in dts
if you have any such slaves not conforming due to board-layout of other
reasons.


OK, I'm fine with leaving something like this till later if/when it
comes up.  Documenting a little bit more about how these timings work
seems like it would be nice, though, at least mentioning what the
source clock is...



Yes, we will document how t-high and t-low is derived from source.


Wow, that's a cluster of arcane calls to handle a call that probably
will never fail (it just enables clocks and sets pinctrl).  Sigh.
...but as far as I can tell the whole sequence is right.  You
definitely need a "put" after a failed get and it looks like
pm_runtime_set_suspended() has a special exception where it can be
called if you got a runtime error...


We didn't have this in before either. But this condition is somewhat
frequent if I2C transactions are called on cusp of exiting system suspend.
(e.g. PMIC slave getting a wakeup-IRQ and trying to read from PMIC through
I2C to read its status as to what caused that wake-up. At that time,
get_sync doesn't really enable resources (kernel 4.9) since the runtime-pm
ref-count isn't decremented. We run the risk of unclocked access if these
arcane calls aren't present. You can go through runtime-pm documentation
chapter 6 for more details.


Yeah, I certainly agree that the calls are needed if
pm_runtime_get_sync() and I'm not suggesting removing them.  Hence the
"as far as I can tell the whole sequence is right".

...but I'm actually kinda worried if you're saying that you actually
ran into this case.  Hopefully that got fixed and code no longer tries
to read from the PMIC at a bad time anymore?  That code should be
fixed not to be running so late in suspend.



I have added Harry Y and Abhijeet D (developers for PMIC I2C client
team). They can comment if there is still a usecase of very late
transaction during suspend and/or very early transaction during resume.





+   /* Make sure no transactions are pending */
+   ret = i2c_trylock_bus(&gi2c->adap, I2C_LOCK_SEGMENT);
+   if (!ret) {
+   dev_err(gi2c->se.dev, "late I2C transaction request\n");
+   return -EBUSY;
+   }



Does this happen?  How?

Nothing about this code looks special for your hardware.  If this is
really needed, why is it not part of the i2c core since there's
nothing specific about your driver here?


There have been some clients that don't implement sys-suspend/resume
callbacks (so i2c adapter has no clue they are done with their transactions)
and this allows us to be flexible when they call I2C transactions extremely
late.


Still feels like this belongs in the i2c core, not your driver.  Maybe
you should send a patch for the core and remove it from here?

...and also, it seems like any i2c clients that don't implement the
suspend/resume callbacks and try to do i2c transactions late in the
game need to be fixed.  It should be documented that this isn't a
valid thing for a driver to do and if we end up in this error case
then it's not an i2c issue but it's a bad driver somewhere.


You are right: this check is special for our HW due to usecases
mentioned above.
This check can go in i2c-core, but then it will not be necessary if
all adapters and clients that we work with are upstreamed (and all
implement system suspend/resume).
We will remove this in next ve

Re: [v3, 3/4] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller

[Sagar Dharia]

Hi Doug

On 3/8/2018 2:12 PM, Doug Anderson wrote:

Hi,

On Wed, Mar 7, 2018 at 9:19 PM, Doug Anderson  wrote:

DMA is hard and i2c transfers > 32 bytes are rare.  Do we really gain
a lot by transferring i2c commands over DMA compared to a FIFO?
Enough to justify the code complexity and the set of bugs that will
show up?  I'm sure it will be a controversial assertion given that the
code's already written, but personally I'd be supportive of ripping
DMA mode out to simplify the driver.  I'd be curious if anyone else
agrees.  To me it seems like premature optimization.



Yes, we have multiple clients (e.g. touch, NFC) using I2C for data transfers
bigger than 32 bytes (some transfers are 100s of bytes). The fifo size is
32, so we can definitely avoid at least 1 interrupt when DMA mode is used
with data size > 32.


Does that 1-2 interrupts make any real difference, though?  In theory
it really shouldn't affect the transfer rate.  We should be able to
service the interrupt plenty fast and if we were concerned we would
tweak the watermark code a little bit.  So I guess we're worried about
the extra CPU cycles (and power cost) to service those extra couple
interrupts?

In theory when touch data is coming in or NFC data is coming in then
we're probably not in a super low power state to begin with.  If it's
touch data we likely want to have the CPU boosted a bunch to respond
to the user quickly.  If we've got 8 cores available all of which can
run at 1GHz or more a few interrupts won't kill us.  NFC data is
probably not common enough that we need to optimize power/CPU
utilizatoin for that.


So while i can believe that you do save an interrupt or two, I still
am not convinced that those interrupts are worth a bunch of complex
code (and a whole second code path) to save.


...also note that above you said that coming out of runtime suspend
can take several msec.  That seems like it dwarfs any slight
difference in timing between a FIFO-based operation and DMA.


One last note here (sorry for not thinking of this last night) is that
I would also be interested in considering _only_ supporting the DMA
path.  Is it somehow intrinsically bad to use the DMA flow for a
1-byte transfer?  Is there a bunch of extra overhead or power draw?

Specifically my main point is that maintaining two separate flows (the
FIFO flow vs the DMA flow) adds complexity, code size, and bugs.  If
there's a really good reason to maintain both flows then fine, but we
should really consider if this is something that's really giving us
value before we agree to it.



FIFO mode gives us 2 advantages:
1. small transfers don't have to go through 'dma-map/unmap penalties.
Some small buffers come from the stack of client caller and the
dma-map/unmap may fail.
2. bring-ups are 'less eventful' (with temp. change to just not have DMA
mode at all during bring-ups) since SMMU translation/DMA path from QUP
(master) to memory slave may not always available when critical I2C
peripherals need to be brought up (e.g. PMIC). CPU to QUP (slave) path
is usually available.

On the other side, DMA mode has other advantages:
1. Multiple android clients are still heavily using I2C in spite of
faster peripheral buses being available in industry.
As an example, some multi-finger Touch screens use I2C and the data to
be transferred per transaction over the bus grows well beyond 70-100
bytes based on number of fingers. These transactions are very frequent
when touch is being used, and in an environment where other heavy system
users are also running (MM/graphics).
Another example is, NFC uses I2C (as of now) to transfer as much as 700+
bytes. This can save us 20+ interrupts per transfer.

These transfers are mostly in burst. So the RPMh penalty to resume the
shared resources is only experienced for very first transfer. Remaining
transfers in the burst benefit from DMA if they are too big.

Goal here is to have common driver for upstream targets and android and
android has seen proven advantages with both modes.
If we end up keeping DMA only for downstream (or FIFO only for
downstream), then we lose the advantage of having code in upstream since
we have to maintain downstream patch with other mode.

Thanks
Sagar



-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/8] Documentation: gpio: Move driver documentation to driver-api

[Jonathan Neuschäfer]

Move gpio/driver.txt to driver-api/gpio/driver.rst and make sure it
builds cleanly as ReST.

Signed-off-by: Jonathan Neuschäfer 
---
 .../driver.txt => driver-api/gpio/driver.rst}  | 80 +++---
 Documentation/driver-api/gpio/index.rst|  1 +
 Documentation/gpio/00-INDEX|  2 -
 3 files changed, 42 insertions(+), 41 deletions(-)
 rename Documentation/{gpio/driver.txt => driver-api/gpio/driver.rst} (93%)

diff --git a/Documentation/gpio/driver.txt 
b/Documentation/driver-api/gpio/driver.rst
similarity index 93%
rename from Documentation/gpio/driver.txt
rename to Documentation/driver-api/gpio/driver.rst
index 3392a0fd4c23..505ee906d7d9 100644
--- a/Documentation/gpio/driver.txt
+++ b/Documentation/driver-api/gpio/driver.rst
@@ -1,3 +1,4 @@
+
 GPIO Descriptor Driver Interface
 
 
@@ -53,9 +54,9 @@ common to each controller of that type:
 
 The code implementing a gpio_chip should support multiple instances of the
 controller, possibly using the driver model. That code will configure each
-gpio_chip and issue gpiochip_add[_data]() or devm_gpiochip_add_data().
-Removing a GPIO controller should be rare; use [devm_]gpiochip_remove() when
-it is unavoidable.
+gpio_chip and issue ``gpiochip_add[_data]()`` or ``devm_gpiochip_add_data()``.
+Removing a GPIO controller should be rare; use ``[devm_]gpiochip_remove()``
+when it is unavoidable.
 
 Often a gpio_chip is part of an instance-specific structure with states not
 exposed by the GPIO interfaces, such as addressing, power management, and more.
@@ -115,7 +116,7 @@ GPIOs with open drain/source support
 
 Open drain (CMOS) or open collector (TTL) means the line is not actively driven
 high: instead you provide the drain/collector as output, so when the transistor
-is not open, it will present a high-impedance (tristate) to the external rail.
+is not open, it will present a high-impedance (tristate) to the external rail::
 
 
CMOS CONFIGURATION  TTL CONFIGURATION
@@ -148,19 +149,19 @@ level-shift to the higher VDD.
 Integrated electronics often have an output driver stage in the form of a CMOS
 "totem-pole" with one N-MOS and one P-MOS transistor where one of them drives
 the line high and one of them drives the line low. This is called a push-pull
-output. The "totem-pole" looks like so:
-
- VDD
-  |
-OD||--+
- +--/ ---o|| P-MOS-FET
- |||--+
-IN --++- out
- |||--+
- +--/ || N-MOS-FET
-OS||--+
-  |
- GND
+output. The "totem-pole" looks like so::
+
+ VDD
+  |
+OD||--+
+ +--/ ---o|| P-MOS-FET
+ |||--+
+IN --++- out
+ |||--+
+ +--/ || N-MOS-FET
+OS||--+
+  |
+ GND
 
 The desired output signal (e.g. coming directly from some GPIO output register)
 arrives at IN. The switches named "OD" and "OS" are normally closed, creating
@@ -219,8 +220,9 @@ systems simultaneously: gpio and irq.
 
 RT_FULL: a realtime compliant GPIO driver should not use spinlock_t or any
 sleepable APIs (like PM runtime) as part of its irq_chip implementation.
-- spinlock_t should be replaced with raw_spinlock_t [1].
-- If sleepable APIs have to be used, these can be done from the .irq_bus_lock()
+
+* spinlock_t should be replaced with raw_spinlock_t [1].
+* If sleepable APIs have to be used, these can be done from the .irq_bus_lock()
   and .irq_bus_unlock() callbacks, as these are the only slowpath callbacks
   on an irqchip. Create the callbacks if needed [2].
 
@@ -232,12 +234,12 @@ GPIO irqchips usually fall in one of two categories:
   system interrupt controller. This means that the GPIO irqchip handler will
   be called immediately from the parent irqchip, while holding the IRQs
   disabled. The GPIO irqchip will then end up calling something like this
-  sequence in its interrupt handler:
+  sequence in its interrupt handler::
 
-  static irqreturn_t foo_gpio_irq(int irq, void *data)
-  chained_irq_enter(...);
-  generic_handle_irq(...);
-  chained_irq_exit(...);
+static irqreturn_t foo_gpio_irq(int irq, void *data)
+chained_irq_enter(...);
+generic_handle_irq(...);
+chained_irq_exit(...);
 
   Chained GPIO irqchips typically can NOT set the .can_sleep flag on
   struct gpio_chip, as everything happens directly in the callbacks: no
@@ -252,7 +254,7 @@ GPIO irqchips usually fall in one of two categories:
   (for example, see [3]).
   Know W/A: The generic_handle_irq() is expected to be called with IRQ 
disabled,
   so the IRQ core will complain if it is called from an IRQ handler which is
-  forced to a thread. The "fake?" raw lock can be used to W/A this problem:
+  forced to a thread. The "fake?" 

[PATCH 7/8] Documentation: gpio: Move GPIO mapping documentation to driver-api

[Jonathan Neuschäfer]

Move gpio/board.txt to driver-api/gpio/board.rst and make sure it builds
cleanly as ReST.

Signed-off-by: Jonathan Neuschäfer 
---
 .../{gpio/board.txt => driver-api/gpio/board.rst}  | 39 --
 Documentation/driver-api/gpio/index.rst|  1 +
 Documentation/gpio/00-INDEX|  2 --
 3 files changed, 22 insertions(+), 20 deletions(-)
 rename Documentation/{gpio/board.txt => driver-api/gpio/board.rst} (88%)

diff --git a/Documentation/gpio/board.txt 
b/Documentation/driver-api/gpio/board.rst
similarity index 88%
rename from Documentation/gpio/board.txt
rename to Documentation/driver-api/gpio/board.rst
index 659bb19f5b3c..25d62b2e9fd0 100644
--- a/Documentation/gpio/board.txt
+++ b/Documentation/driver-api/gpio/board.rst
@@ -1,3 +1,4 @@
+=
 GPIO Mappings
 =
 
@@ -23,7 +24,7 @@ device tree bindings for your controller.
 
 GPIOs mappings are defined in the consumer device's node, in a property named
 -gpios, where  is the function the driver will request
-through gpiod_get(). For example:
+through gpiod_get(). For example::
 
foo_device {
compatible = "acme,foo";
@@ -40,7 +41,7 @@ it but are only supported for compatibility reasons and 
should not be used for
 newer bindings since it has been deprecated.
 
 This property will make GPIOs 15, 16 and 17 available to the driver under the
-"led" function, and GPIO 1 as the "power" GPIO:
+"led" function, and GPIO 1 as the "power" GPIO::
 
struct gpio_desc *red, *green, *blue, *power;
 
@@ -60,13 +61,13 @@ looked up by the gpiod functions internally) used in the 
device tree. With above
 
 Internally, the GPIO subsystem prefixes the GPIO suffix ("gpios" or "gpio")
 with the string passed in con_id to get the resulting string
-(snprintf(... "%s-%s", con_id, gpio_suffixes[]).
+(``snprintf(... "%s-%s", con_id, gpio_suffixes[]``).
 
 ACPI
 
 ACPI also supports function names for GPIOs in a similar fashion to DT.
 The above DT example can be converted to an equivalent ACPI description
-with the help of _DSD (Device Specific Data), introduced in ACPI 5.1:
+with the help of _DSD (Device Specific Data), introduced in ACPI 5.1::
 
Device (FOO) {
Name (_CRS, ResourceTemplate () {
@@ -105,12 +106,12 @@ Documentation/acpi/gpio-properties.txt.
 Platform Data
 -
 Finally, GPIOs can be bound to devices and functions using platform data. Board
-files that desire to do so need to include the following header:
+files that desire to do so need to include the following header::
 
#include 
 
 GPIOs are mapped by the means of tables of lookups, containing instances of the
-gpiod_lookup structure. Two macros are defined to help declaring such mappings:
+gpiod_lookup structure. Two macros are defined to help declaring such 
mappings::
 
GPIO_LOOKUP(chip_label, chip_hwnum, con_id, flags)
GPIO_LOOKUP_IDX(chip_label, chip_hwnum, con_id, idx, flags)
@@ -141,22 +142,24 @@ end. The 'dev_id' field of the table is the identifier of 
the device that will
 make use of these GPIOs. It can be NULL, in which case it will be matched for
 calls to gpiod_get() with a NULL device.
 
-struct gpiod_lookup_table gpios_table = {
-   .dev_id = "foo.0",
-   .table = {
-   GPIO_LOOKUP_IDX("gpio.0", 15, "led", 0, GPIO_ACTIVE_HIGH),
-   GPIO_LOOKUP_IDX("gpio.0", 16, "led", 1, GPIO_ACTIVE_HIGH),
-   GPIO_LOOKUP_IDX("gpio.0", 17, "led", 2, GPIO_ACTIVE_HIGH),
-   GPIO_LOOKUP("gpio.0", 1, "power", GPIO_ACTIVE_LOW),
-   { },
-   },
-};
+.. code-block:: c
+
+struct gpiod_lookup_table gpios_table = {
+.dev_id = "foo.0",
+.table = {
+GPIO_LOOKUP_IDX("gpio.0", 15, "led", 0, 
GPIO_ACTIVE_HIGH),
+GPIO_LOOKUP_IDX("gpio.0", 16, "led", 1, 
GPIO_ACTIVE_HIGH),
+GPIO_LOOKUP_IDX("gpio.0", 17, "led", 2, 
GPIO_ACTIVE_HIGH),
+GPIO_LOOKUP("gpio.0", 1, "power", GPIO_ACTIVE_LOW),
+{ },
+},
+};
 
-And the table can be added by the board code as follows:
+And the table can be added by the board code as follows::
 
gpiod_add_lookup_table(&gpios_table);
 
-The driver controlling "foo.0" will then be able to obtain its GPIOs as 
follows:
+The driver controlling "foo.0" will then be able to obtain its GPIOs as 
follows::
 
struct gpio_desc *red, *green, *blue, *power;
 
diff --git a/Documentation/driver-api/gpio/index.rst 
b/Documentation/driver-api/gpio/index.rst
index 6ba9e0043310..2b73ea5a1fbb 100644
--- a/Documentation/driver-api/gpio/index.rst
+++ b/Documentation/driver-api/gpio/index.rst
@@ -10,6 +10,7 @@ Contents:
intro
driver
consumer
+   board
legacy
 
 Core
diff --git a/Documentation/gpio/00-INDEX b/Documentation/gpio/00-INDEX
index f960fc00a3ef..650cb0696211 100644
--- a/Documentation/gpi

[PATCH 6/8] Documentation: gpio: Move gpiod_* consumer documentation to driver-api

[Jonathan Neuschäfer]

Move gpio/consumer.txt to driver-api/gpio/consumer.rst and make sure it
builds cleanly as ReST.

Signed-off-by: Jonathan Neuschäfer 
---
 .../consumer.txt => driver-api/gpio/consumer.rst}  | 85 +++---
 Documentation/driver-api/gpio/index.rst|  1 +
 Documentation/gpio/00-INDEX|  2 -
 3 files changed, 44 insertions(+), 44 deletions(-)
 rename Documentation/{gpio/consumer.txt => driver-api/gpio/consumer.rst} (89%)

diff --git a/Documentation/gpio/consumer.txt 
b/Documentation/driver-api/gpio/consumer.rst
similarity index 89%
rename from Documentation/gpio/consumer.txt
rename to Documentation/driver-api/gpio/consumer.rst
index d53e5b5cfc9c..c71a50d85b50 100644
--- a/Documentation/gpio/consumer.txt
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -1,3 +1,4 @@
+==
 GPIO Descriptor Consumer Interface
 ==
 
@@ -30,10 +31,10 @@ warnings. These stubs are used for two use cases:
   be met with console warnings that may be perceived as intimidating.
 
 All the functions that work with the descriptor-based GPIO interface are
-prefixed with gpiod_. The gpio_ prefix is used for the legacy interface. No
-other function in the kernel should use these prefixes. The use of the legacy
-functions is strongly discouraged, new code should use 
-and descriptors exclusively.
+prefixed with ``gpiod_``. The ``gpio_`` prefix is used for the legacy
+interface. No other function in the kernel should use these prefixes. The use
+of the legacy functions is strongly discouraged, new code should use
+ and descriptors exclusively.
 
 
 Obtaining and Disposing GPIOs
@@ -43,13 +44,13 @@ With the descriptor-based interface, GPIOs are identified 
with an opaque,
 non-forgeable handler that must be obtained through a call to one of the
 gpiod_get() functions. Like many other kernel subsystems, gpiod_get() takes the
 device that will use the GPIO and the function the requested GPIO is supposed 
to
-fulfill:
+fulfill::
 
struct gpio_desc *gpiod_get(struct device *dev, const char *con_id,
enum gpiod_flags flags)
 
 If a function is implemented by using several GPIOs together (e.g. a simple LED
-device that displays digits), an additional index argument can be specified:
+device that displays digits), an additional index argument can be specified::
 
struct gpio_desc *gpiod_get_index(struct device *dev,
  const char *con_id, unsigned int idx,
@@ -84,7 +85,7 @@ occurred while trying to acquire it. This is useful to 
discriminate between mere
 errors and an absence of GPIO for optional GPIO parameters. For the common
 pattern where a GPIO is optional, the gpiod_get_optional() and
 gpiod_get_index_optional() functions can be used. These functions return NULL
-instead of -ENOENT if no GPIO has been assigned to the requested function:
+instead of -ENOENT if no GPIO has been assigned to the requested function::
 
struct gpio_desc *gpiod_get_optional(struct device *dev,
 const char *con_id,
@@ -101,14 +102,14 @@ This is helpful to driver authors, since they do not need 
to special case
 -ENOSYS return codes.  System integrators should however be careful to enable
 gpiolib on systems that need it.
 
-For a function using multiple GPIOs all of those can be obtained with one call:
+For a function using multiple GPIOs all of those can be obtained with one 
call::
 
struct gpio_descs *gpiod_get_array(struct device *dev,
   const char *con_id,
   enum gpiod_flags flags)
 
 This function returns a struct gpio_descs which contains an array of
-descriptors:
+descriptors::
 
struct gpio_descs {
unsigned int ndescs;
@@ -116,13 +117,13 @@ descriptors:
}
 
 The following function returns NULL instead of -ENOENT if no GPIOs have been
-assigned to the requested function:
+assigned to the requested function::
 
struct gpio_descs *gpiod_get_array_optional(struct device *dev,
const char *con_id,
enum gpiod_flags flags)
 
-Device-managed variants of these functions are also defined:
+Device-managed variants of these functions are also defined::
 
struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id,
 enum gpiod_flags flags)
@@ -149,11 +150,11 @@ Device-managed variants of these functions are also 
defined:
 const char *con_id,
 enum gpiod_flags flags)
 
-A GPIO descriptor can be disposed of using the gpiod_put() function:
+A GPIO descriptor can be disposed of using the gpiod_put() function::
 
void gpiod_p

[PATCH 2/8] Documentation: driver-api: Move gpio.rst to gpio/index.rst

[Jonathan Neuschäfer]

To make space for more files in the GPIO section, create a
Documentation/driver-api/gpio/ directory.

Signed-off-by: Jonathan Neuschäfer 
---
 Documentation/driver-api/{gpio.rst => gpio/index.rst} | 0
 Documentation/driver-api/index.rst| 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename Documentation/driver-api/{gpio.rst => gpio/index.rst} (100%)

diff --git a/Documentation/driver-api/gpio.rst 
b/Documentation/driver-api/gpio/index.rst
similarity index 100%
rename from Documentation/driver-api/gpio.rst
rename to Documentation/driver-api/gpio/index.rst
diff --git a/Documentation/driver-api/index.rst 
b/Documentation/driver-api/index.rst
index 62d056471c0d..9b54fb99d9ca 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -45,7 +45,7 @@ available subsections can be seen below.
uio-howto
firmware/index
pinctl
-   gpio
+   gpio/index
misc_devices
dmaengine/index
slimbus
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/8] Documentation: gpio: Move drivers-on-gpio.txt to driver-api

[Jonathan Neuschäfer]

Move gpio/drivers-on-gpio.txt to driver-api/gpio/drivers-on-gpio.rst and
make sure it builds cleanly as ReST.

Signed-off-by: Jonathan Neuschäfer 
---

This patch applies cleanly on top of 93db446a424c ("mtd: nand: move raw
NAND related code to the raw/ subdir")
---
 .../drivers-on-gpio.txt => driver-api/gpio/drivers-on-gpio.rst}  | 1 +
 Documentation/driver-api/gpio/index.rst  | 1 +
 Documentation/gpio/00-INDEX  | 3 ---
 Documentation/gpio/sysfs.txt | 5 ++---
 4 files changed, 4 insertions(+), 6 deletions(-)
 rename Documentation/{gpio/drivers-on-gpio.txt => 
driver-api/gpio/drivers-on-gpio.rst} (99%)

diff --git a/Documentation/gpio/drivers-on-gpio.txt 
b/Documentation/driver-api/gpio/drivers-on-gpio.rst
similarity index 99%
rename from Documentation/gpio/drivers-on-gpio.txt
rename to Documentation/driver-api/gpio/drivers-on-gpio.rst
index a3e612f55bc7..7da0c1dd1f7a 100644
--- a/Documentation/gpio/drivers-on-gpio.txt
+++ b/Documentation/driver-api/gpio/drivers-on-gpio.rst
@@ -1,3 +1,4 @@
+
 Subsystem drivers using GPIO
 
 
diff --git a/Documentation/driver-api/gpio/index.rst 
b/Documentation/driver-api/gpio/index.rst
index 2b73ea5a1fbb..6a374ded1287 100644
--- a/Documentation/driver-api/gpio/index.rst
+++ b/Documentation/driver-api/gpio/index.rst
@@ -11,6 +11,7 @@ Contents:
driver
consumer
board
+   drivers-on-gpio
legacy
 
 Core
diff --git a/Documentation/gpio/00-INDEX b/Documentation/gpio/00-INDEX
index 650cb0696211..17e19a68058f 100644
--- a/Documentation/gpio/00-INDEX
+++ b/Documentation/gpio/00-INDEX
@@ -1,7 +1,4 @@
 00-INDEX
- This file
-drivers-on-gpio.txt:
-   - Drivers in other subsystems that can use GPIO to provide more
- complex functionality.
 sysfs.txt
- Information about the GPIO sysfs interface
diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
index 6cdeab8650cd..58eeab81f349 100644
--- a/Documentation/gpio/sysfs.txt
+++ b/Documentation/gpio/sysfs.txt
@@ -32,9 +32,8 @@ standard kernels won't know about. And for some tasks, simple 
userspace
 GPIO drivers could be all that the system really needs.
 
 DO NOT ABUSE SYSFS TO CONTROL HARDWARE THAT HAS PROPER KERNEL DRIVERS.
-PLEASE READ THE DOCUMENT NAMED "drivers-on-gpio.txt" IN THIS DOCUMENTATION
-DIRECTORY TO AVOID REINVENTING KERNEL WHEELS IN USERSPACE. I MEAN IT.
-REALLY.
+PLEASE READ THE DOCUMENT AT Documentation/driver-api/gpio/drivers-on-gpio.rst
+TO AVOID REINVENTING KERNEL WHEELS IN USERSPACE. I MEAN IT. REALLY.
 
 Paths in Sysfs
 --
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/8] Documentation: gpio: Move legacy documentation to driver-api

[Jonathan Neuschäfer]

Move gpio/gpio-legacy.txt to driver-api/gpio/legacy.rst and make sure it
builds cleanly as ReST.

Also move the legacy API reference from index.rst to legacy.rst.

Signed-off-by: Jonathan Neuschäfer 
---
 Documentation/driver-api/gpio/index.rst| 10 +---
 .../gpio-legacy.txt => driver-api/gpio/legacy.rst} | 68 +-
 Documentation/gpio/00-INDEX|  2 -
 3 files changed, 41 insertions(+), 39 deletions(-)
 rename Documentation/{gpio/gpio-legacy.txt => driver-api/gpio/legacy.rst} (96%)

diff --git a/Documentation/driver-api/gpio/index.rst 
b/Documentation/driver-api/gpio/index.rst
index e1fbc5408cf6..fd22c0d1419e 100644
--- a/Documentation/driver-api/gpio/index.rst
+++ b/Documentation/driver-api/gpio/index.rst
@@ -9,6 +9,7 @@ Contents:
 
intro
driver
+   legacy
 
 Core
 
@@ -19,15 +20,6 @@ Core
 .. kernel-doc:: drivers/gpio/gpiolib.c
:export:
 
-Legacy API
-==
-
-The functions listed in this section are deprecated. The GPIO descriptor based
-API described above should be used in new code.
-
-.. kernel-doc:: drivers/gpio/gpiolib-legacy.c
-   :export:
-
 ACPI support
 
 
diff --git a/Documentation/gpio/gpio-legacy.txt 
b/Documentation/driver-api/gpio/legacy.rst
similarity index 96%
rename from Documentation/gpio/gpio-legacy.txt
rename to Documentation/driver-api/gpio/legacy.rst
index 8356d0e78f67..5e9421e05f1d 100644
--- a/Documentation/gpio/gpio-legacy.txt
+++ b/Documentation/driver-api/gpio/legacy.rst
@@ -1,4 +1,6 @@
-GPIO Interfaces
+==
+Legacy GPIO Interfaces
+==
 
 This provides an overview of GPIO access conventions on Linux.
 
@@ -129,7 +131,7 @@ The first thing a system should do with a GPIO is allocate 
it, using
 the gpio_request() call; see later.
 
 One of the next things to do with a GPIO, often in board setup code when
-setting up a platform_device using the GPIO, is mark its direction:
+setting up a platform_device using the GPIO, is mark its direction::
 
/* set as input or output, returning 0 or negative errno */
int gpio_direction_input(unsigned gpio);
@@ -164,7 +166,7 @@ Those don't need to sleep, and can safely be done from 
inside hard
 (nonthreaded) IRQ handlers and similar contexts.
 
 Use the following calls to access such GPIOs,
-for which gpio_cansleep() will always return false (see below):
+for which gpio_cansleep() will always return false (see below)::
 
/* GPIO INPUT:  return zero or nonzero */
int gpio_get_value(unsigned gpio);
@@ -201,11 +203,11 @@ This requires sleeping, which can't be done from inside 
IRQ handlers.
 
 Platforms that support this type of GPIO distinguish them from other GPIOs
 by returning nonzero from this call (which requires a valid GPIO number,
-which should have been previously allocated with gpio_request):
+which should have been previously allocated with gpio_request)::
 
int gpio_cansleep(unsigned gpio);
 
-To access such GPIOs, a different set of accessors is defined:
+To access such GPIOs, a different set of accessors is defined::
 
/* GPIO INPUT:  return zero or nonzero, might sleep */
int gpio_get_value_cansleep(unsigned gpio);
@@ -222,27 +224,27 @@ Other than the fact that these accessors might sleep, and 
will work
 on GPIOs that can't be accessed from hardIRQ handlers, these calls act
 the same as the spinlock-safe calls.
 
-  ** IN ADDITION ** calls to setup and configure such GPIOs must be made
+**IN ADDITION** calls to setup and configure such GPIOs must be made
 from contexts which may sleep, since they may need to access the GPIO
-controller chip too:  (These setup calls are usually made from board
-setup or driver probe/teardown code, so this is an easy constraint.)
+controller chip too  (These setup calls are usually made from board
+setup or driver probe/teardown code, so this is an easy constraint.)::
 
-   gpio_direction_input()
-   gpio_direction_output()
-   gpio_request()
+gpio_direction_input()
+gpio_direction_output()
+gpio_request()
 
-## gpio_request_one()
-## gpio_request_array()
-## gpio_free_array()
+## gpio_request_one()
+## gpio_request_array()
+## gpio_free_array()
 
-   gpio_free()
-   gpio_set_debounce()
+gpio_free()
+gpio_set_debounce()
 
 
 
 Claiming and Releasing GPIOs
 
-To help catch system configuration errors, two calls are defined.
+To help catch system configuration errors, two calls are defined::
 
/* request GPIO, returning 0 or negative errno.
 * non-null labels may be useful for diagnostics.
@@ -296,7 +298,7 @@ Also note that it's your responsibility to have stopped 
using a GPIO
 before you free it.
 
 Considering in most cases GPIOs are actually configured right after they
-are claimed, three additional calls are defined:
+are claimed, th

[PATCH 1/8] MAINTAINERS: GPIO: Add Documentation/driver-api/gpio/

[Jonathan Neuschäfer]

Steer patches to Documentation/driver-api/gpio/ into the right
direction.

Signed-off-by: Jonathan Neuschäfer 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d6a89d31e4a4..313c0907020c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6029,6 +6029,7 @@ L:linux-g...@vger.kernel.org
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
 S: Maintained
 F: Documentation/devicetree/bindings/gpio/
+F: Documentation/driver-api/gpio/
 F: Documentation/gpio/
 F: Documentation/ABI/testing/gpio-cdev
 F: Documentation/ABI/obsolete/sysfs-gpio
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/8] Move most GPIO documentation to driver-api/gpio/ and ReST

[Jonathan Neuschäfer]

The aim of this patchset is to move the GPIO subsystem's documentation
under Documentation/driver-api/gpio/ such that it is picked up by Sphinx
and compiled into HTML. I moved everything except for sysfs.txt, because
this file describes the legacy sysfs ABI, and doesn't seem to serve much
(non-historical) purpose anymore.

There are still some rough edges:

* I think the API documentation (kernel-doc) should be moved out of
  index.rst into more appropriate files.
* The headings are arguably wrong, because driver.rst, consumer.rst,
  etc. use the "document title" style, even though they are part of the
  GPIO chapter. But the resulting TOC tree is consistent, and I did not
  want to change almost all headings.
* Some of the files could use more :c:func:`...` references and general
  ReST polish.

But I don't want to make this patchset too large.

Jonathan Neuschäfer (8):
  MAINTAINERS: GPIO: Add Documentation/driver-api/gpio/
  Documentation: driver-api: Move gpio.rst to gpio/index.rst
  Documentation: gpio: Move introduction to driver-api
  Documentation: gpio: Move driver documentation to driver-api
  Documentation: gpio: Move legacy documentation to driver-api
  Documentation: gpio: Move gpiod_* consumer documentation to driver-api
  Documentation: gpio: Move GPIO mapping documentation to driver-api
  Documentation: gpio: Move drivers-on-gpio.txt to driver-api

 .../{gpio/board.txt => driver-api/gpio/board.rst}  | 39 +-
 .../consumer.txt => driver-api/gpio/consumer.rst}  | 85 +++---
 .../driver.txt => driver-api/gpio/driver.rst}  | 80 ++--
 .../gpio/drivers-on-gpio.rst}  |  1 +
 .../driver-api/{gpio.rst => gpio/index.rst}| 21 +++---
 .../{gpio/gpio.txt => driver-api/gpio/intro.rst}   |  9 ++-
 .../gpio-legacy.txt => driver-api/gpio/legacy.rst} | 68 ++---
 Documentation/driver-api/index.rst |  2 +-
 Documentation/gpio/00-INDEX| 13 
 Documentation/gpio/sysfs.txt   |  5 +-
 MAINTAINERS|  1 +
 11 files changed, 169 insertions(+), 155 deletions(-)
 rename Documentation/{gpio/board.txt => driver-api/gpio/board.rst} (88%)
 rename Documentation/{gpio/consumer.txt => driver-api/gpio/consumer.rst} (89%)
 rename Documentation/{gpio/driver.txt => driver-api/gpio/driver.rst} (93%)
 rename Documentation/{gpio/drivers-on-gpio.txt => 
driver-api/gpio/drivers-on-gpio.rst} (99%)
 rename Documentation/driver-api/{gpio.rst => gpio/index.rst} (74%)
 rename Documentation/{gpio/gpio.txt => driver-api/gpio/intro.rst} (96%)
 rename Documentation/{gpio/gpio-legacy.txt => driver-api/gpio/legacy.rst} (96%)

-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/8] Documentation: gpio: Move introduction to driver-api

[Jonathan Neuschäfer]

Move gpio/intro.txt to driver-api/gpio/intro.rst and make sure it builds
cleanly as ReST.

Signed-off-by: Jonathan Neuschäfer 
---
 Documentation/driver-api/gpio/index.rst| 7 +++
 Documentation/{gpio/gpio.txt => driver-api/gpio/intro.rst} | 9 +++--
 Documentation/gpio/00-INDEX| 2 --
 3 files changed, 14 insertions(+), 4 deletions(-)
 rename Documentation/{gpio/gpio.txt => driver-api/gpio/intro.rst} (96%)

diff --git a/Documentation/driver-api/gpio/index.rst 
b/Documentation/driver-api/gpio/index.rst
index 6dd4aa647f27..db47d845f473 100644
--- a/Documentation/driver-api/gpio/index.rst
+++ b/Documentation/driver-api/gpio/index.rst
@@ -2,6 +2,13 @@
 General Purpose Input/Output (GPIO)
 ===
 
+Contents:
+
+.. toctree::
+   :maxdepth: 2
+
+   intro
+
 Core
 
 
diff --git a/Documentation/gpio/gpio.txt 
b/Documentation/driver-api/gpio/intro.rst
similarity index 96%
rename from Documentation/gpio/gpio.txt
rename to Documentation/driver-api/gpio/intro.rst
index cd9b356e88cd..74591489d0b5 100644
--- a/Documentation/gpio/gpio.txt
+++ b/Documentation/driver-api/gpio/intro.rst
@@ -1,3 +1,8 @@
+
+Introduction
+
+
+
 GPIO Interfaces
 ===
 
@@ -9,9 +14,9 @@ Due to the history of GPIO interfaces in the kernel, there are 
two different
 ways to obtain and use GPIOs:
 
   - The descriptor-based interface is the preferred way to manipulate GPIOs,
-and is described by all the files in this directory excepted gpio-legacy.txt.
+and is described by all the files in this directory excepted 
gpio-legacy.txt.
   - The legacy integer-based interface which is considered deprecated (but 
still
-usable for compatibility reasons) is documented in gpio-legacy.txt.
+usable for compatibility reasons) is documented in gpio-legacy.txt.
 
 The remainder of this document applies to the new descriptor-based interface.
 gpio-legacy.txt contains the same information applied to the legacy
diff --git a/Documentation/gpio/00-INDEX b/Documentation/gpio/00-INDEX
index 179beb234f98..52fe0fa6c964 100644
--- a/Documentation/gpio/00-INDEX
+++ b/Documentation/gpio/00-INDEX
@@ -1,7 +1,5 @@
 00-INDEX
- This file
-gpio.txt
-   - Introduction to GPIOs and their kernel interfaces
 consumer.txt
- How to obtain and use GPIOs in a driver
 driver.txt
-- 
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/4] tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP

[Stephen Boyd]

Quoting Karthik Ramasubramanian (2018-03-07 22:06:29)
> 
> 
> On 3/6/2018 2:45 PM, Stephen Boyd wrote:
> > Quoting Karthik Ramasubramanian (2018-03-05 16:51:23)
> >> On 3/2/2018 3:11 PM, Stephen Boyd wrote:
> > 
> > Ok. I've seen similar designs in some mmc drivers. For example, you can
> > look at drivers/mmc/host/meson-gx-mmc.c and see that they register a
> > clk_ops and then just start using that clk directly from the driver.
> > There are also some helper functions for dividers that would hopefully
> > make the job of calculating the best divider easier.
> Thanks for the pointers. I will take a look at it. In the meanwhile I 
> had discussions with our clock team. They pointed out that the register 
> to write the divider value here is outside the scope of clock controller 
> which makes it trickier to implement your suggestion. They are already 
> in the mailing list and we will discuss further and get back to you in 
> this regard.

Ok. Let me know if I can help answer any questions.

> >>>
> >>> Why are these noirq variants? Please add a comment.
> >> The intention is to enable the console UART port usage as late as
> >> possible in the suspend cycle. Hence noirq variants. I will add this as
> >> a comment. Please let me know if the usage does not match the intention.
> > 
> > Hm.. Does no_console_suspend not work? Or not work well enough?
> It works. When console suspend is disabled, the suspend operation does 
> not get triggered and the resume operation checks if the console suspend 
> is disabled and performs the needed thing.

Ok so then do we need the noirq variants? Or console suspend is special
enough for this to not matter?
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v3, 3/4] i2c: i2c-qcom-geni: Add bus driver for the Qualcomm GENI I2C controller

[Doug Anderson]

Hi,

On Wed, Mar 7, 2018 at 9:19 PM, Doug Anderson  wrote:
>>> DMA is hard and i2c transfers > 32 bytes are rare.  Do we really gain
>>> a lot by transferring i2c commands over DMA compared to a FIFO?
>>> Enough to justify the code complexity and the set of bugs that will
>>> show up?  I'm sure it will be a controversial assertion given that the
>>> code's already written, but personally I'd be supportive of ripping
>>> DMA mode out to simplify the driver.  I'd be curious if anyone else
>>> agrees.  To me it seems like premature optimization.
>>
>>
>> Yes, we have multiple clients (e.g. touch, NFC) using I2C for data transfers
>> bigger than 32 bytes (some transfers are 100s of bytes). The fifo size is
>> 32, so we can definitely avoid at least 1 interrupt when DMA mode is used
>> with data size > 32.
>
> Does that 1-2 interrupts make any real difference, though?  In theory
> it really shouldn't affect the transfer rate.  We should be able to
> service the interrupt plenty fast and if we were concerned we would
> tweak the watermark code a little bit.  So I guess we're worried about
> the extra CPU cycles (and power cost) to service those extra couple
> interrupts?
>
> In theory when touch data is coming in or NFC data is coming in then
> we're probably not in a super low power state to begin with.  If it's
> touch data we likely want to have the CPU boosted a bunch to respond
> to the user quickly.  If we've got 8 cores available all of which can
> run at 1GHz or more a few interrupts won't kill us.  NFC data is
> probably not common enough that we need to optimize power/CPU
> utilizatoin for that.
>
>
> So while i can believe that you do save an interrupt or two, I still
> am not convinced that those interrupts are worth a bunch of complex
> code (and a whole second code path) to save.
>
>
> ...also note that above you said that coming out of runtime suspend
> can take several msec.  That seems like it dwarfs any slight
> difference in timing between a FIFO-based operation and DMA.

One last note here (sorry for not thinking of this last night) is that
I would also be interested in considering _only_ supporting the DMA
path.  Is it somehow intrinsically bad to use the DMA flow for a
1-byte transfer?  Is there a bunch of extra overhead or power draw?

Specifically my main point is that maintaining two separate flows (the
FIFO flow vs the DMA flow) adds complexity, code size, and bugs.  If
there's a really good reason to maintain both flows then fine, but we
should really consider if this is something that's really giving us
value before we agree to it.


-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/4] soc: qcom: Add GENI based QUP Wrapper driver

[Karthik Ramasubramanian]

On 3/8/2018 6:24 AM, Robin Murphy wrote:

On 08/03/18 06:46, Karthik Ramasubramanian wrote:



On 3/6/2018 2:56 PM, Stephen Boyd wrote:

Quoting Karthik Ramasubramanian (2018-03-02 16:58:23)


+   return iova;
+}
+EXPORT_SYMBOL(geni_se_tx_dma_prep);
+
+/**
+ * geni_se_rx_dma_prep() - Prepare the Serial Engine for RX DMA 
transfer
+ * @se:    Pointer to the concerned Serial 
Engine.

+ * @buf:   Pointer to the RX buffer.
+ * @len:   Length of the RX buffer.
+ *
+ * This function is used to prepare the buffers for DMA RX.
+ *
+ * Return: Mapped DMA Address of the buffer on success, NULL on 
failure.

+ */
+dma_addr_t geni_se_rx_dma_prep(struct geni_se *se, void *buf, 
size_t len)

+{
+   dma_addr_t iova;
+   struct geni_wrapper *wrapper = se->wrapper;
+   u32 val;
+
+   iova = dma_map_single(wrapper->dev, buf, len, 
DMA_FROM_DEVICE);

+   if (dma_mapping_error(wrapper->dev, iova))
+   return (dma_addr_t)NULL;


Can't return a dma_mapping_error address to the caller and have them
figure it out?

Earlier we used to return the DMA_ERROR_CODE which has been removed
recently in arm64 architecture. If we return the dma_mapping_error, 
then

the caller also needs the device which encountered the mapping error.
The serial interface drivers can use their parent currently to resolve
the mapping error. Once the wrapper starts mapping using IOMMU context
bank, then the serial interface drivers do not know which device to use
to know if there is an error.

Having said that, the dma_ops suggestion might help with handling this
situation. I will look into it further.


Ok, thanks.


+{
+   struct geni_wrapper *wrapper = se->wrapper;
+
+   if (iova)
+   dma_unmap_single(wrapper->dev, iova, len, 
DMA_FROM_DEVICE);

+}
+EXPORT_SYMBOL(geni_se_rx_dma_unprep);


Instead of having the functions exported, could we set the dma_ops on
all child devices of the wrapper that this driver populates and then
implement the DMA ops for those devices here? I assume that there's
never another DMA master between the wrapper and the serial engine, 
so I

think it would work.

This suggestion looks like it will work.


It would be a good idea to check with some other people on the dma_ops
suggestion. Maybe add the DMA mapping subsystem folks to help out here

I have added the DMA mapping subsystem folks to help out here.

To present an overview, we have a wrapper controller which is composed 
of several serial engines. The serial engines are programmed with 
UART, I2C or SPI protocol and support DMA transfer. When the serial 
engines perform DMA transfer, the wrapper controller device is used to 
perform the mapping. The reason wrapper device is used is because of 
IOMMU and there is only one IOMMU context bank to perform the 
translation for the entire wrapper controller. So the wrapper 
controller exports map and unmap functions to the individual protocol 
drivers.


There is a suggestion to make the parent wrapper controller implement 
the dma_map_ops, instead of exported map/unmap functions and populate 
those dma_map_ops on all the children serial engines. Can you please 
provide your inputs regarding this suggestion?


Implementing dma_map_ops inside a driver for real hardware is almost 
always the wrong thing to do.


Based on what I could infer about the hardware from looking through the 
whole series in the linux-arm-msm archive, this is probably more like a 
multi-channel DMA controller where each "channel" has a configurable 
serial interface on the other end, as opposed to an actual bus where the 
serial engines are individually distinct AHB masters routed through the 
wrapper. If that's true, then using the QUP platform device for DMA API 
calls is the appropriate thing to do. Personally I'd be inclined not to 
abstract the dma_{map,unmap} calls at all, and just have the protocol 
drivers make them directly using dev->parent/wrapper->dev/whatever, but 
if you do want to abstract those then just give the abstraction a saner 
interface, i.e. pass the DMA handle by reference and return a regular 
int for error/success status.


Robin.
Thank you Robin & Christoph for your inputs. The wrapper driver used to 
provide the recommended abstraction until v2 of this patch series. In v3 
it was tweaked to address a comment. If there are no objections, I will 
revive it back.


Regards,
Karthik.
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [v2] docs: clarify security-bugs disclosure policy

[Jonathan Corbet]

On Wed, 7 Mar 2018 13:53:06 -0800
Linus Torvalds  wrote:

> I'm guessing this will go through Jon?

Sounds like a fine guess to me; will apply shortly.

Thanks,

jon
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [v2] docs: clarify security-bugs disclosure policy

[Greg KH]

On Wed, Mar 07, 2018 at 01:46:24PM -0800, Dave Hansen wrote:
> 
> From: Dave Hansen 
> 
> I think we need to soften the language a bit.  It might scare folks
> off, especially the:
> 
>We prefer to fully disclose the bug as soon as possible.
> 
> which is not really the case.  Linus says:
> 
>   It's not full disclosure, it's not coordinated disclosure,
>   and it's not "no disclosure".  It's more like just "timely
>   open fixes".
> 
> I changed a bit of the wording in here, but mostly to remove the word
> "disclosure" since it seems to mean very specific things to people
> that we do not mean here.
> 
> Signed-off-by: Dave Hansen 
> Reviewed-by: Dan Williams 
> Cc: Thomas Gleixner 
> Cc: Greg Kroah-Hartman 
> Cc: Linus Torvalds 
> Cc: Alan Cox 
> Cc: Andrea Arcangeli 
> Cc: Andy Lutomirski 
> Cc: Kees Cook 
> Cc: Tim Chen 
> Cc: Alexander Viro 
> Cc: Andrew Morton 
> Cc: linux-doc@vger.kernel.org
> Cc: Jonathan Corbet 
> Cc: Mark Rutland 
> ---
> 

Reviewed-by: Greg Kroah-Hartman 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

[Laxman Dewangan]

On Thursday 08 March 2018 08:01 PM, Guenter Roeck wrote:

On 03/07/2018 10:06 PM, Laxman Dewangan wrote:



The RPM is measured speed via PWM signal capture  which is output 
from fan.

So should we have the fan[1..n]_output_rpm?



No. I hear you clearly that you for some reason dislike fan[1..n]_input.
While ABIs are not always to our liking, that doesn't mean that we get
to change them at our whim. If that is not acceptable for you, I can't
help you. And you can't change inX_input to inX_voltage either, sorry.


My opinion is only to not use "input" as this is not really the input to 
fan.

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/4] soc: qcom: Add GENI based QUP Wrapper driver

[Christoph Hellwig]

On Thu, Mar 08, 2018 at 01:24:45PM +, Robin Murphy wrote:
> Implementing dma_map_ops inside a driver for real hardware is almost always 
> the wrong thing to do.

Agreed.  dma_map_ops should be a platform decision based on the bus.

Even our dma_virt_ops basically just works around bad driver layering.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

[Guenter Roeck]

On 03/07/2018 11:57 PM, Mikko Perttunen wrote:



On 07.03.2018 16:20, Guenter Roeck wrote:

On 03/07/2018 01:47 AM, Rajkumar Rampelli wrote:



On Wednesday 28 February 2018 07:59 PM, Guenter Roeck wrote:

On 02/27/2018 11:03 PM, Mikko Perttunen wrote:

On 02/28/2018 08:12 AM, Rajkumar Rampelli wrote:


On Wednesday 28 February 2018 11:28 AM, Guenter Roeck wrote:

On 02/27/2018 09:38 PM, Rajkumar Rampelli wrote:


On Wednesday 21 February 2018 08:20 PM, Guenter Roeck wrote:

On 02/20/2018 10:58 PM, Rajkumar Rampelli wrote:

Add generic PWM based tachometer driver via HWMON interface
to report the RPM of motor. This drivers get the period/duty
cycle from PWM IP which captures the motor PWM output.

This driver implements a simple interface for monitoring the
speed of
a fan and exposes it in roatations per minute (RPM) to the user
space
by using the hwmon's sysfs interface

Signed-off-by: Rajkumar Rampelli 
---
  Documentation/hwmon/generic-pwm-tachometer |  17 +
  drivers/hwmon/Kconfig  |  10 +++
  drivers/hwmon/Makefile |   1 +
  drivers/hwmon/generic-pwm-tachometer.c | 112
+
  4 files changed, 140 insertions(+)
  create mode 100644 Documentation/hwmon/generic-pwm-tachometer
  create mode 100644 drivers/hwmon/generic-pwm-tachometer.c

diff --git a/Documentation/hwmon/generic-pwm-tachometer
b/Documentation/hwmon/generic-pwm-tachometer
new file mode 100644
index 000..e0713ee
--- /dev/null
+++ b/Documentation/hwmon/generic-pwm-tachometer
@@ -0,0 +1,17 @@
+Kernel driver generic-pwm-tachometer
+
+
+This driver enables the use of a PWM module to monitor a fan.
It uses the
+generic PWM interface and can be used on SoCs as along as the
SoC supports
+Tachometer controller that moniors the Fan speed in periods.
+
+Author: Rajkumar Rampelli 
+
+Description
+---
+
+The driver implements a simple interface for monitoring the
Fan speed using
+PWM module and Tachometer controller. It requests period value
through PWM
+capture interface to Tachometer and measures the Rotations per
minute using
+received period value. It exposes the Fan speed in RPM to the
user space by
+using the hwmon's sysfs interface.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index ef23553..8912dcb 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1878,6 +1878,16 @@ config SENSORS_XGENE
    If you say yes here you get support for the temperature
    and power sensors for APM X-Gene SoC.
  +config GENERIC_PWM_TACHOMETER
+    tristate "Generic PWM based tachometer driver"
+    depends on PWM
+    help
+  Enables a driver to use PWM signal from motor to use
+  for measuring the motor speed. The RPM is captured by
+  PWM modules which has PWM capture capability and this
+  drivers reads the captured data from PWM IP to convert
+  it to speed in RPM.
+
  if ACPI
    comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index f814b4a..9dcc374 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_SENSORS_WM8350)    +=
wm8350-hwmon.o
  obj-$(CONFIG_SENSORS_XGENE)    += xgene-hwmon.o
    obj-$(CONFIG_PMBUS)    += pmbus/
+obj-$(CONFIG_GENERIC_PWM_TACHOMETER) += generic-pwm-tachometer.o
    ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
  diff --git a/drivers/hwmon/generic-pwm-tachometer.c
b/drivers/hwmon/generic-pwm-tachometer.c
new file mode 100644
index 000..9354d43
--- /dev/null
+++ b/drivers/hwmon/generic-pwm-tachometer.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights
reserved.
+ *
+ * This program is free software; you can redistribute it
and/or modify it
+ * under the terms and conditions of the GNU General Public
License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful,
but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General
Public License for
+ * more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct pwm_hwmon_tach {
+    struct device    *dev;
+    struct pwm_device    *pwm;
+    struct device    *hwmon;
+};
+
+static ssize_t show_rpm(struct device *dev, struct
device_attribute *attr,
+    char *buf)
+{
+    struct pwm_hwmon_tach *ptt = dev_get_drvdata(dev);
+    struct pwm_device *pwm = ptt->pwm;
+    struct pwm_capture result;
+    int err;
+    unsigned int rpm = 0;
+
+    err = pwm_capture(pwm, &result, 0);
+    if (err < 0) {
+    dev_err(ptt->dev, "Failed to capture PWM: %d\n", err);
+    return err;
+    }
+
+    if (result.period)
+    rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+    result.period);
+
+    return sprintf(buf, "%u\n", rpm);
+}
+
+static SENSOR_DEVICE_ATTR(rpm, 0444, s

Re: [PATCH 05/10] hwmon: generic-pwm-tachometer: Add generic PWM based tachometer

[Guenter Roeck]

On 03/07/2018 10:06 PM, Laxman Dewangan wrote:



On Wednesday 07 March 2018 07:50 PM, Guenter Roeck wrote:

On 03/07/2018 01:47 AM, Rajkumar Rampelli wrote:




While I am not opposed to ABI changes, the merits of those would need to be
discussed on the mailing list. But replacing "fan1_input" with "rpm" is
not an acceptable ABI change, even if it may measure something that turns
but isn't a fan.

If this _is_ in fact supposed to be used for something else but fans, we
would have to discuss what that might be, and if hwmon is the appropriate
subsystem to measure and report it. This does to some degree lead back to
my concern of having the "fan" part of this patch series in the pwm core.
I am still not sure if that makes sense.

Thanks,
Guenter

I am planning to add tachometer support in pwm-fan.c driver (drivers/hwmon/) 
instead of adding new generic-pwm-tachometer.c driver. Measuring RPM value will 
be done in pwm-fan driver itself using pwm capture feature and will add new 
sysfs attributes under this driver to report rpm value of fan.


There is an existing attribute to report the RPM of fans. It is called 
fan[1..n]_input.

"replacing "fan1_input" with "rpm" is not an acceptable ABI change"

Preemptive NACK.


The RPM is measured speed via PWM signal capture  which is output from fan.
So should we have the fan[1..n]_output_rpm?



No. I hear you clearly that you for some reason dislike fan[1..n]_input.
While ABIs are not always to our liking, that doesn't mean that we get
to change them at our whim. If that is not acceptable for you, I can't
help you. And you can't change inX_input to inX_voltage either, sorry.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/4] soc: qcom: Add GENI based QUP Wrapper driver

[Robin Murphy]

On 08/03/18 06:46, Karthik Ramasubramanian wrote:



On 3/6/2018 2:56 PM, Stephen Boyd wrote:

Quoting Karthik Ramasubramanian (2018-03-02 16:58:23)


+   return iova;
+}
+EXPORT_SYMBOL(geni_se_tx_dma_prep);
+
+/**
+ * geni_se_rx_dma_prep() - Prepare the Serial Engine for RX DMA 
transfer
+ * @se:    Pointer to the concerned Serial 
Engine.

+ * @buf:   Pointer to the RX buffer.
+ * @len:   Length of the RX buffer.
+ *
+ * This function is used to prepare the buffers for DMA RX.
+ *
+ * Return: Mapped DMA Address of the buffer on success, NULL on 
failure.

+ */
+dma_addr_t geni_se_rx_dma_prep(struct geni_se *se, void *buf, 
size_t len)

+{
+   dma_addr_t iova;
+   struct geni_wrapper *wrapper = se->wrapper;
+   u32 val;
+
+   iova = dma_map_single(wrapper->dev, buf, len, 
DMA_FROM_DEVICE);

+   if (dma_mapping_error(wrapper->dev, iova))
+   return (dma_addr_t)NULL;


Can't return a dma_mapping_error address to the caller and have them
figure it out?

Earlier we used to return the DMA_ERROR_CODE which has been removed
recently in arm64 architecture. If we return the dma_mapping_error, then
the caller also needs the device which encountered the mapping error.
The serial interface drivers can use their parent currently to resolve
the mapping error. Once the wrapper starts mapping using IOMMU context
bank, then the serial interface drivers do not know which device to use
to know if there is an error.

Having said that, the dma_ops suggestion might help with handling this
situation. I will look into it further.


Ok, thanks.


+{
+   struct geni_wrapper *wrapper = se->wrapper;
+
+   if (iova)
+   dma_unmap_single(wrapper->dev, iova, len, 
DMA_FROM_DEVICE);

+}
+EXPORT_SYMBOL(geni_se_rx_dma_unprep);


Instead of having the functions exported, could we set the dma_ops on
all child devices of the wrapper that this driver populates and then
implement the DMA ops for those devices here? I assume that there's
never another DMA master between the wrapper and the serial engine, 
so I

think it would work.

This suggestion looks like it will work.


It would be a good idea to check with some other people on the dma_ops
suggestion. Maybe add the DMA mapping subsystem folks to help out here

I have added the DMA mapping subsystem folks to help out here.

To present an overview, we have a wrapper controller which is composed 
of several serial engines. The serial engines are programmed with UART, 
I2C or SPI protocol and support DMA transfer. When the serial engines 
perform DMA transfer, the wrapper controller device is used to perform 
the mapping. The reason wrapper device is used is because of IOMMU and 
there is only one IOMMU context bank to perform the translation for the 
entire wrapper controller. So the wrapper controller exports map and 
unmap functions to the individual protocol drivers.


There is a suggestion to make the parent wrapper controller implement 
the dma_map_ops, instead of exported map/unmap functions and populate 
those dma_map_ops on all the children serial engines. Can you please 
provide your inputs regarding this suggestion?


Implementing dma_map_ops inside a driver for real hardware is almost 
always the wrong thing to do.


Based on what I could infer about the hardware from looking through the 
whole series in the linux-arm-msm archive, this is probably more like a 
multi-channel DMA controller where each "channel" has a configurable 
serial interface on the other end, as opposed to an actual bus where the 
serial engines are individually distinct AHB masters routed through the 
wrapper. If that's true, then using the QUP platform device for DMA API 
calls is the appropriate thing to do. Personally I'd be inclined not to 
abstract the dma_{map,unmap} calls at all, and just have the protocol 
drivers make them directly using dev->parent/wrapper->dev/whatever, but 
if you do want to abstract those then just give the abstraction a saner 
interface, i.e. pass the DMA handle by reference and return a regular 
int for error/success status.


Robin.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 5/7] thermal/drivers/cpu_cooling: Add idle cooling device documentation

[Daniel Thompson]

On Thu, Mar 08, 2018 at 09:59:49AM +0100, Pavel Machek wrote:
> Hi!
> 
> > >> +Under certain circumstances, the SoC reaches a temperature exceeding
> > >> +the allocated power budget or the maximum temperature limit. The
> > > 
> > > I don't understand. Power budget is in W, temperature is in
> > > kelvin. Temperature can't exceed power budget AFAICT.
> > 
> > Yes, it is badly worded. Is the following better ?
> > 
> > "
> > Under certain circumstances a SoC can reach the maximum temperature
> > limit or is unable to stabilize the temperature around a temperature
> > control.
> > 
> > When the SoC has to stabilize the temperature, the kernel can act on a
> > cooling device to mitigate the dissipated power.
> > 
> > When the maximum temperature is reached and to prevent a catastrophic
> > situation a radical decision must be taken to reduce the temperature
> > under the critical threshold, that impacts the performance.
> > 
> > "
> 
> Actually... if hardware is expected to protect itself, I'd tone it
> down. No need to be all catastrophic and critical... But yes, better.

Makes sense. For a thermally overcommitted but passively cooled device 
work close to max operating temperature it is not a critical situation 
requiring a radical reaction, it is normal operation.

Put another way, it would severely bogus to attach KERN_CRITICAL 
messages to reaching the cooling threshold.


Daniel.


> > > Critical here, critical there. I have trouble following
> > > it. Theoretically hardware should protect itself, because you don't
> > > want kernel bug to damage your CPU?
> > 
> > There are several levels of protection. The first level is mitigating
> > the temperature from the kernel, then in the temperature sensor a reset
> > line will trigger the reboot of the CPUs. Usually it is a register where
> > you write the maximum temperature, from the driver itself. I never tried
> > to write 1000°C in this register and see if I can burn the board.
> > 
> > I know some boards have another level of thermal protection in the
> > hardware itself and some other don't.
> > 
> > In any case, from a kernel point of view, it is a critical situation as
> > we are about to hard reboot the system and in this case it is preferable
> > to drop drastically the performance but give the opportunity to the
> > system to run in a degraded mode.
> 
> Agreed you want to keep going. In ACPI world, we shutdown when
> critical trip point is reached, so this is somehow confusing.
> 
> > >> +Solutions:
> > >> +--
> > >> +
> > >> +If we can remove the static and the dynamic leakage for a specific
> > >> +duration in a controlled period, the SoC temperature will
> > >> +decrease. Acting at the idle state duration or the idle cycle
> > > 
> > > "should" decrease? If you are in bad environment..
> > 
> > No, it will decrease in any case because of the static leakage drop. The
> > bad environment will impact the speed of this decrease.
> 
> I meant... if ambient temperature is 105C, there's not much you can do
> to cool system down :-).
> 
> > >> +Idle Injection:
> > >> +---
> > >> +
> > >> +The base concept of the idle injection is to force the CPU to go to an
> > >> +idle state for a specified time each control cycle, it provides
> > >> +another way to control CPU power and heat in addition to
> > >> +cpufreq. Ideally, if all CPUs of a cluster inject idle synchronously,
> > >> +this cluster can get into the deepest idle state and achieve minimum
> > >> +power consumption, but that will also increase system response latency
> > >> +if we inject less than cpuidle latency.
> > > 
> > > I don't understand last sentence.
> > 
> > Is it better ?
> > 
> > "Ideally, if all CPUs, belonging to the same cluster, inject their idle
> > cycle synchronously, the cluster can reach its power down state with a
> > minimum power consumption and static leakage drop. However, these idle
> > cycles injection will add extra latencies as the CPUs will have to
> > wakeup from a deep sleep state."
> 
> Extra comma "CPUs , belonging". But yes, better.
> 
> > Thanks!
> 
> You are welcome. Best regards,
>   Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 5/7] thermal/drivers/cpu_cooling: Add idle cooling device documentation

[Pavel Machek]

Hi!

> >> +Under certain circumstances, the SoC reaches a temperature exceeding
> >> +the allocated power budget or the maximum temperature limit. The
> > 
> > I don't understand. Power budget is in W, temperature is in
> > kelvin. Temperature can't exceed power budget AFAICT.
> 
> Yes, it is badly worded. Is the following better ?
> 
> "
> Under certain circumstances a SoC can reach the maximum temperature
> limit or is unable to stabilize the temperature around a temperature
> control.
> 
> When the SoC has to stabilize the temperature, the kernel can act on a
> cooling device to mitigate the dissipated power.
> 
> When the maximum temperature is reached and to prevent a catastrophic
> situation a radical decision must be taken to reduce the temperature
> under the critical threshold, that impacts the performance.
> 
> "

Actually... if hardware is expected to protect itself, I'd tone it
down. No need to be all catastrophic and critical... But yes, better.

> > Critical here, critical there. I have trouble following
> > it. Theoretically hardware should protect itself, because you don't
> > want kernel bug to damage your CPU?
> 
> There are several levels of protection. The first level is mitigating
> the temperature from the kernel, then in the temperature sensor a reset
> line will trigger the reboot of the CPUs. Usually it is a register where
> you write the maximum temperature, from the driver itself. I never tried
> to write 1000°C in this register and see if I can burn the board.
> 
> I know some boards have another level of thermal protection in the
> hardware itself and some other don't.
> 
> In any case, from a kernel point of view, it is a critical situation as
> we are about to hard reboot the system and in this case it is preferable
> to drop drastically the performance but give the opportunity to the
> system to run in a degraded mode.

Agreed you want to keep going. In ACPI world, we shutdown when
critical trip point is reached, so this is somehow confusing.

> >> +Solutions:
> >> +--
> >> +
> >> +If we can remove the static and the dynamic leakage for a specific
> >> +duration in a controlled period, the SoC temperature will
> >> +decrease. Acting at the idle state duration or the idle cycle
> > 
> > "should" decrease? If you are in bad environment..
> 
> No, it will decrease in any case because of the static leakage drop. The
> bad environment will impact the speed of this decrease.

I meant... if ambient temperature is 105C, there's not much you can do
to cool system down :-).

> >> +Idle Injection:
> >> +---
> >> +
> >> +The base concept of the idle injection is to force the CPU to go to an
> >> +idle state for a specified time each control cycle, it provides
> >> +another way to control CPU power and heat in addition to
> >> +cpufreq. Ideally, if all CPUs of a cluster inject idle synchronously,
> >> +this cluster can get into the deepest idle state and achieve minimum
> >> +power consumption, but that will also increase system response latency
> >> +if we inject less than cpuidle latency.
> > 
> > I don't understand last sentence.
> 
> Is it better ?
> 
> "Ideally, if all CPUs, belonging to the same cluster, inject their idle
> cycle synchronously, the cluster can reach its power down state with a
> minimum power consumption and static leakage drop. However, these idle
> cycles injection will add extra latencies as the CPUs will have to
> wakeup from a deep sleep state."

Extra comma "CPUs , belonging". But yes, better.

> Thanks!

You are welcome. Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature