Re: [PATCH] ASoC: rcar: ssi: don't set SSICR.CKDV = 000 with SSIWSR.CONT
On Wed, Mar 22, 2017 at 08:56:07AM +, Kuninori Morimoto wrote: > > Hi Sergei > > > > /* > > > + * It will set SSIWSR.CONT here, but SSICR.CKDV = 000 > > > + * with it is not allowed. (SSIWSR.WS_MODE with > > > + * SSICR.CKDV = 000 is not allowed either). > > > + * Skip it. See SSICR.CKDV > > > + */ > > > + if (j == 0) > > > + continue; > > > >Why not change the *for* statement itself to start with j = 1? > > It can be one solution. Actually my local 1st patch was such style. > But I thought that it is difficult to notice such magical operation from code. > Thus, I used this style FWIIW, I think if you have a comment, like the one above, then j = 1 should be obvious enough.
Re: [GIT PULL] Renesas ARM Based SoC Drivers Updates for v4.12
On Tue, Mar 21, 2017 at 05:37:50PM -0700, Olof Johansson wrote: > On Mon, Mar 20, 2017 at 09:58:23AM +0100, Simon Horman wrote: > > Hi Olof, Hi Kevin, Hi Arnd, > > > > Please consider these Renesas ARM based SoC drivers updates for v4.12. > > > > > > The following changes since commit c1ae3cfa0e89fa1a7ecc4c99031f5e9ae99d9201: > > > > Linux 4.11-rc1 (2017-03-05 12:59:56 -0800) > > > > are available in the git repository at: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git > > tags/renesas-drivers-for-v4.12 > > > > for you to fetch changes up to cd59de80dd34dd2d1a3ca97d7a6e712c048b135a: > > > > soc: renesas: Identify RZ/G1N (2017-03-13 10:26:08 +0100) > > > > > > Renesas ARM Based SoC Drivers Updates for v4.12 > > > > * Identify RZ/G1N and RZ/G1H > > > > > > Geert Uytterhoeven (2): > > soc: renesas: Identify RZ/G1H > > soc: renesas: Identify RZ/G1N > > At first I was wondering why you'd applied the same patch twice. Renesas likes > to make names similar *and* hard to remember sometimes. :-) :-) Thanks for queueing up this and the other pull-requests I sent.
Re: [PATCH v2] media: platform: rcar_imr: add IMR-LSX3 support
On Thu, Mar 16, 2017 at 09:59:50PM +0300, Sergei Shtylyov wrote: > Add support for the image renderer light SRAM extended 3 (IMR-LSX3) found > only in the R-Car V2H (R8A7792) SoC. It differs from IMR-LX4 in that it > supports only planar video formats but can use the video capture data for > the textures. > > Signed-off-by: Sergei Shtylyov > > --- > This patch is against the 'media_tree.git' repo's 'master' branch plus the > latest version of the Renesas IMR driver... > > Changes in version 2: > - renamed *enum* 'imr_gen' to 'imr_type' and the *struct* field of this type > from 'gen' to 'type'; > - rename *struct* 'imr_type' to 'imr_info' and the fields/variables of this > type > from 'type' to 'info'; > - added comments to IMR-LX4 only CMRCR2 bits; > - added IMR type check to the WTS instruction writing to CMRCCR2. > > Documentation/devicetree/bindings/media/rcar_imr.txt | 11 + Acked-by: Rob Herring > drivers/media/platform/rcar_imr.c| 106 > +++ > 2 files changed, 97 insertions(+), 20 deletions(-)
[PATCH] ASoC: core: remove pointless auxiliary from snd_soc_component
From: Kuninori Morimoto commit 1a653aa44725 ("ASoC: core: replace aux_comp_list to ...") tried to replace aux_comp_list to component_dev_list, but it failed because of binding timing. Thus, Sylwester fixuped it by commit d2e3a1358c37 ("ASoC: Fix binding and probing of auxiliary..."). One of main purpose of commit 1a653aa44725 ("ASoC: core: replace...") was remove replaceable list (= list_aux) from snd_soc_component by using new "auxiliary" flags (but it failed). Because of this background, current code has reborned card_aux_list (= same as original list_aux), and almost pointless "auxiliary" flags. Let's remove pointless "auxiliary" flags by this patch This means, it is same as revert both commit 1a653aa44725 ("ASoC: core: replace aux_comp_list to ...") and commit d2e3a1358c37 ("ASoC: Fix binding and probing of auxiliary..."). Signed-off-by: Kuninori Morimoto Tested-by: Sylwester Nawrocki --- include/sound/soc.h | 1 - sound/soc/soc-core.c | 15 +-- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index dd78507..316fdce 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -815,7 +815,6 @@ struct snd_soc_component { unsigned int ignore_pmdown_time:1; /* pmdown_time is ignored at stop */ unsigned int registered_as_component:1; - unsigned int auxiliary:1; /* for auxiliary component of the card */ unsigned int suspended:1; /* is in suspend PM state */ struct list_head list; diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index e7d876a..5933851 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1777,7 +1777,6 @@ static int soc_bind_aux_dev(struct snd_soc_card *card, int num) } component->init = aux_dev->init; - component->auxiliary = 1; list_add(&component->card_aux_list, &card->aux_comp_list); return 0; @@ -1789,14 +1788,13 @@ static int soc_bind_aux_dev(struct snd_soc_card *card, int num) static int soc_probe_aux_devices(struct snd_soc_card *card) { - struct snd_soc_component *comp, *tmp; + struct snd_soc_component *comp; int order; int ret; for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) { - list_for_each_entry_safe(comp, tmp, &card->aux_comp_list, -card_aux_list) { + list_for_each_entry(comp, &card->aux_comp_list, card_aux_list) { if (comp->driver->probe_order == order) { ret = soc_probe_component(card, comp); if (ret < 0) { @@ -1805,7 +1803,6 @@ static int soc_probe_aux_devices(struct snd_soc_card *card) comp->name, ret); return ret; } - list_del(&comp->card_aux_list); } } } @@ -1821,14 +1818,12 @@ static void soc_remove_aux_devices(struct snd_soc_card *card) for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) { list_for_each_entry_safe(comp, _comp, - &card->component_dev_list, card_list) { - - if (!comp->auxiliary) - continue; + &card->aux_comp_list, card_aux_list) { if (comp->driver->remove_order == order) { soc_remove_component(comp); - comp->auxiliary = 0; + /* remove it from the card's aux_comp_list */ + list_del(&comp->card_aux_list); } } } -- 1.9.1 ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Re: [PATCH] backlight: pwm_bl: Fix GPIO out for unimplemented .get_direction()
On 03/22/2017 07:21 PM, Geert Uytterhoeven wrote: Commit 7613c922315e308a ("backlight: pwm_bl: Move the checks for initial power state to a separate function") not just moved some code, but made slight changes in semantics. If a gpiochip doesn't implement the optional .get_direction() callback, gpiod_get_direction always returns -EINVAL, which is never equal to GPIOF_DIR_IN, leading to the GPIO not being configured for output. The platforms I have tested the gpio drivers do implement the get_direction() callback and I was not aware of this behaviour. To avoid this, invert the test and check for not GPIOF_DIR_OUT instead, like the original code did. This restores the display on r8a7740/armadillo. Fixes: 7613c922315e308a ("backlight: pwm_bl: Move the checks for initial power state to a separate function") Signed-off-by: Geert Uytterhoeven Reviewed-by: Peter Ujfalusi --- drivers/video/backlight/pwm_bl.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index d7efcb632f7d9dde..002f1ce22bd02032 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -297,14 +297,15 @@ static int pwm_backlight_probe(struct platform_device *pdev) } /* -* If the GPIO is configured as input, change the direction to output -* and set the GPIO as active. +* If the GPIO is not known to be already configured as output, that +* is, if gpiod_get_direction returns either GPIOF_DIR_IN or -EINVAL, +* change the direction to output and set the GPIO as active. * Do not force the GPIO to active when it was already output as it * could cause backlight flickering or we would enable the backlight too * early. Leave the decision of the initial backlight state for later. */ if (pb->enable_gpio && - gpiod_get_direction(pb->enable_gpio) == GPIOF_DIR_IN) + gpiod_get_direction(pb->enable_gpio) != GPIOF_DIR_OUT) gpiod_direction_output(pb->enable_gpio, 1); pb->power_supply = devm_regulator_get(&pdev->dev, "power"); -- Péter
Re: [PATCH v2 4/7] arm: dts: r7s72100: Add pin controller node
Hi Geert, On Wed, Mar 22, 2017 at 02:12:04PM +0100, Geert Uytterhoeven wrote: > Hi Jacopo, > > On Mon, Mar 20, 2017 at 5:14 PM, Jacopo Mondi > wrote: > > Add pin controller node with 12 gpio controller sub-nodes to > > r7s72100 dtsi. > > > > Signed-off-by: Jacopo Mondi > > Reviewed-by: Geert Uytterhoeven > > > --- a/arch/arm/boot/dts/r7s72100.dtsi > > +++ b/arch/arm/boot/dts/r7s72100.dtsi > > @@ -183,6 +183,86 @@ > > }; > > }; > > > > + pinctrl: pinctrl@fcfe3000 { > > + compatible = "renesas,r7s72100-ports"; > > + > > + #pinctrl-cells = <1>; > > + > > + reg = <0xfcfe3000 0x4248>; > > How did you get to 0x4248? I had expected 0x4230. > Not that it matters much, Linux rounds it up to a multiple of PAGE_SIZE > anyway. > I guess I probably confused hex and dec values, as the correct calculation here would have been (4200 + C * 4) That 4248 value is what you get with (4200 + 12 * 4) so I probably typed 12 in the calculator instead of C :) I'll change this even if it does not make any difference.. > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@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
Re: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc
Hi Geert, thanks for review On Wed, Mar 22, 2017 at 11:33:50AM +0100, Geert Uytterhoeven wrote: > Hi Jacopo, > > On Mon, Mar 20, 2017 at 5:14 PM, Jacopo Mondi > wrote: > > Add device tree bindings documentation for Renesas RZ/A1 gpio and pin > > for the Renesas ... > > > controller. > > > > Signed-off-by: Jacopo Mondi > > --- > > .../bindings/pinctrl/renesas,rza1-pinctrl.txt | 144 > > + > > 1 file changed, 144 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt > > > > diff --git > > a/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt > > b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt > > new file mode 100644 > > index 000..0474860 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt > > @@ -0,0 +1,144 @@ > > +Renesas RZ/A1 combined Pin and GPIO controller > > + > > +Renesas SoCs of RZ/A1 family feature a combined Pin and GPIO controller > > the RZ/A1 family > > > +hardware controller, named "Ports" in the hardware reference manual. > > bogus "hardware controller" > > > +Sub-nodes > > +- > > + > > +The child nodes of the pin controller node describe a pin multiplexing > > +function or a gpio controller alternatively. > > + > > +- Pin multiplexing sub-nodes: > > + A pin multiplexing sub-node describes how to configure a set of > > + (or a single) pin in some desired alternate function mode. > > + A single sub-node may define several pin configurations groups. > > + > > + Required properties: > > +- renesas,pins > > Just "pins"? > You know, I've been thinking about this, bu the "pins" property definition in pinctrl-bidings is the following one: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt --- - pins takes a list of pin names or IDs as a required argument. The specific binding for the hardware defines: - Whether the entries are integers or strings, and their meaning. --- And all examples there assume one "pin name" or "ID" per pin. Now, we use 2 values per each pin (the pin ID and the alternate function number), so to me this is different from what the generic binding describes. Also, pinctrl-single, and pinctrl-imx which have and ABI similar to the one this driver define, use "pinctrl-single,pins" and "fsl,pins" respectively as property names. So either they have to be updated yet, or we should keep using "renesas,pins" for our own defined ABI. Maybe Linus or other pinctrl people can give some suggestion here. Thanks j > > + describes an array of pin multiplexing configurations. > > + When a pin has to be configured in alternate function mode, use this > > + property to identify the pin by its global index, and provide its > > + alternate function configuration description along with it. > > + When multiple pins are required to be configured as part of the same > > + alternate function (odds are single-pin alternate functions exist) > > they > > + shall be specified as members of the same argument list of a single > > + "renesas-pins" property. > > + Helper macros to ease calculating the pin index from its position > > + (port where it sits on and pin offset), and alternate function > > + configuration options are provided in pin controller header file at: > > the pin ... file > > > + include/dt-bindings/pinctrl/r7s72100-pinctrl.h > > Hence I'd include that file in this patch, as it's part of the bindings. > > > + Example: > > + A serial communication interface with a TX output pin and a RX input pin. > > an RX > > > + > > + &pinctrl { > > + scif2_pins: serial2 { > > + renesas,pins = , > > + ; > > Single line? > > > + }; > > + } > > + > > + Pin #0 on port #3 is configured in alternate function #6. > > + Pin #2 on port #3 is configured in alternate function #4. > > as alternate function > > > + > > + Example 2: > > + I2c master: both SDA and SCL pins need bi-directional operations > > + > > + &pinctrl { > > + i2c2_pins: i2c2 { > > + renesas,pins = , > > + ; > > + }; > > + } > > + > > + Pin #4 on port #1 is configured in alternate function #1. > > + Pin #5 on port #1 is configured in alternate function #1. > > as alternate function > > > + Both need to work in bi-directional mode. > > + > > + Example 3: > > + Multi-function timer input and output compare pins. > > + The hardware manual prescribes this pins to have input/output direction > > + specified by software. Configure TIOC0A as input and TIOC0B as output. > > + > > + &pinctrl { > > + tioc0_pins: tioc0 { > > + renesas,pins = , > > + ; > > + }; > > + } > > + > > + Pin #0 on port #4 is configured in alternate function #2 with IO > > direction > > + specifie
vsps DT property (was: Re: [PATCH 10/22] drm: rcar-du: Expose the VSP1 compositor through KMS planes)
Hi Laurent, On Mon, Sep 14, 2015 at 12:50 AM, Laurent Pinchart wrote: > Signed-off-by: Laurent Pinchart > --- /dev/null > +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > +int rcar_du_vsp_init(struct rcar_du_vsp *vsp) > +{ > + struct rcar_du_device *rcdu = vsp->dev; > + struct platform_device *pdev; > + struct device_node *np; > + unsigned int i; > + int ret; > + > + /* Find the VSP device and initialize it. */ > + np = of_parse_phandle(rcdu->dev->of_node, "vsps", vsp->index); > + if (!np) { > + dev_err(rcdu->dev, "vsps node not found\n"); > + return -ENXIO; > + } Apparently the "vsps" DT property was never documented in Documentation/devicetree/bindings/display/renesas,du.txt. Can you please fix that? Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH v2 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller
Hi Geert, thanks for detailed review On Wed, Mar 22, 2017 at 11:26:58AM +0100, Geert Uytterhoeven wrote: > Hi Jacopo, > > On Mon, Mar 20, 2017 at 5:14 PM, Jacopo Mondi > wrote: > > Add combined gpio and pin controller driver for Renesas RZ/A1 > > r7s72100 SoC. > > > > Signed-off-by: Jacopo Mondi > > Thanks for your patch! > > > --- /dev/null > > +++ b/drivers/pinctrl/pinctrl-rza1.c > > > +/* > > + * This pincontroller/gpio combined driver support Renesas devices of RZ/A1 > > + * family. > > + * This includes SoCs which are sub- or super- sets of this particular > > line, > > + * as RZ/A1H (r7s721000), RZ/A1M (r7s721001) and RZ/A1L (r7s721002) are. > > RZ/A1M = r7s721010, RZ/A1L = r7s721020 > > > +#define RZA1_ADDR(mem, reg, port) ((mem) + (reg) + ((port) * 4)) > > > > + > > +#define RZA1_NPORTS12 > > +#define RZA1_PINS_PER_PORT 16 > > +#define RZA1_NPINS (RZA1_PINS_PER_PORT * RZA1_NPORTS) > > +#define RZA1_PIN_TO_PORT(pin) ((pin) / RZA1_PINS_PER_PORT) > > +#define RZA1_PIN_TO_OFFSET(pin)((pin) % RZA1_PINS_PER_PORT) > > + > > +/* > > + * Be careful here: the pin configuration subnodes in device tree > > enumerates > > enumerate > > > + * alternate functions from 1 to 8; subtract 1 before using macros so to > > match > > + * registers configuration which expects numbers from 0 to 7 instead. > > register configuration > > > + */ > > +#define MUX_FUNC_OFFS 3 > > +#define MUX_FUNC_MASK (BIT(MUX_FUNC_OFFS) - 1) > > +#define MUX_FUNC_PFC_MASK BIT(0) > > +#define MUX_FUNC_PFCE_MASK BIT(1) > > +#define MUX_FUNC_PFCEA_MASKBIT(2) > > +#define MUX_CONF_BIDIR BIT(0) > > +#define MUX_CONF_SWIO_INPUTBIT(1) > > +#define MUX_CONF_SWIO_OUTPUT BIT(2) > > + > > +/** > > + * rza1_pin_conf - describes a pin position, id, mux config and output > > value > > + * > > + * Use uint32_t to match types used in of_device nodes argument lists. > > + * > > + * @id: the pin identifier from 0 to RZA1_NPINS > > + * @port: the port where pin sits on > > + * @offset: pin offset in the port > > + * @mux: alternate function configuration settings > > + * @value: output value to set the pin to > > + */ > > +struct rza1_pin_conf { > > + uint32_t id; > > + uint32_t port; > > + uint32_t offset; > > + uint32_t mux_conf; > > + uint32_t value; > > In the kernel, we tend to use u32 instead of uint32_t. > But many of these fields can be smaller than 32 bits, right, saving some > memory (important when running with built-in RAM only). > I'll move these to u8 (and u16 where appropriate) and fix all the above grammar comments > > +/** > > + * rza1_pinctrl - RZ pincontroller device > > + * > > + * @dev: parent device structure > > + * @mutex: protect [pinctrl|pinmux]_generic functions > > + * @base: logical address base > > + * @nports: number of pin controller ports > > + * @ports: pin controller banks > > + * @ngpiochips: number of gpio chips > > + * @gpio_ranges: gpio ranges for pinctrl core > > + * @pins: pin array for pinctrl core > > + * @desc: pincontroller desc for pinctrl core > > + * @pctl: pinctrl device > > + */ > > +struct rza1_pinctrl { > > + struct device *dev; > > + > > + struct mutex mutex; > > + > > + void __iomem *base; > > + > > + unsigned int nport; > > + struct rza1_port *ports; > > + > > + unsigned int ngpiochips; > > + > > + struct pinctrl_gpio_range *gpio_ranges; > > + struct pinctrl_pin_desc *pins; > > + struct pinctrl_desc desc; > > + struct pinctrl_dev *pctl; > > It's a good idea not to mix pointers and integers, as the pointers will > be 64-bit on 64-bit platforms, leading to gaps due to alignment rules. > Not super-important here, as (for now) this driver is meant for 32-bit SoCs. > I grouped this variables "logically", and even if I understand your concerns, there will be a single "struct rza1_pinctrl" per driver instance, so I hope we can tollerate some padding bytes in favour of more clearness. Also, as you said, there are no 64-bits RZ platforms atm. > > +/** > > + * rza1_set_bit() - un-locked set/clear a single bit in pin configuration > > + * registers > > + */ > > +static inline void rza1_set_bit(const struct rza1_port *port, > > + unsigned int reg, unsigned int offset, > > + bool set) > > I think "reg" and "set" still fit on the previous lines (many more cases > in other functions). > > I'd call "offset" "bit" (and "reg" "offset"?) I'll s/offset/bit and s/offset/pin where appropriate > > > +{ > > + void __iomem *mem = RZA1_ADDR(port->base, reg, port->id); > > I think this would be easier to read without using the RZA1_ADDR() macro. > Don't know... I can change this if you prefer... there's nothing nasty hidden behind this mac
Re: [PATCH v2 4/4] arm64: dts: salvator-x: Add current sense amplifiers
Hi Jacopo, On Thu, Mar 23, 2017 at 1:41 PM, Jacopo Mondi wrote: > Add device nodes for two Maxim max961x current sense amplifiers > sensing VDD_08 and DVFS_08 lines. > > Signed-off-by: Jacopo Mondi > --- > arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 18 ++ > 1 file changed, 18 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts > b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts > index fd05c2b..ef8b723 100644 > --- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts > +++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts > @@ -261,6 +261,24 @@ > status = "okay"; > }; > > +&i2c4 { > + status = "okay"; > + > + csa_vdd: max9611_vdd@7c { csa_vdd: adc@7c { Node names should be generic, i.e. "adc" instead of "max9611_vdd" (also in the DT binding example). > + compatible = "maxim,max9611"; > + reg = <0x7c>; > + > + maxim,shunt-resistor-uohm = <5000>; > + }; > + > + csa_dvfs: max9611_dvfs@7f { csa_dvfs: adc@7f { > + compatible = "maxim,max9611"; > + reg = <0x7f>; > + > + maxim,shunt-resistor-uohm = <5000>; > + }; > +}; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH v2 1/4] Documentation: dt-bindings: iio: Add max9611 ADC
On Thu, Mar 23, 2017 at 1:41 PM, Jacopo Mondi wrote: > Add device tree bindings documentation for Maxim max9611/max9612 current > sense amplifier. > > Signed-off-by: Jacopo Mondi > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/max9611.txt > @@ -0,0 +1,27 @@ > +* Maxim max9611/max9612 current sense amplifier with 12-bits ADC interface > + > +Maxim max9611/max9612 is an high-side current sense amplifier with integrated > +12-bits ADC communicating over I2c bus. > +The device node for this driver shall be a child of a I2c controller. > + > +Required properties > + - compatible: Should be "maxim,max9611" or "maxim,max9612" > + - reg: The 7-bits long I2c address of the device > + - maxim,shunt-resistor-uohm: Resistor value, in uOhm, of the current sense > + shunt resistor. Isn't this sufficiently generic to drop the "maxim," prefix? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
[PATCH v2 1/4] Documentation: dt-bindings: iio: Add max9611 ADC
Add device tree bindings documentation for Maxim max9611/max9612 current sense amplifier. Signed-off-by: Jacopo Mondi --- .../devicetree/bindings/iio/adc/max9611.txt| 27 ++ 1 file changed, 27 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/max9611.txt diff --git a/Documentation/devicetree/bindings/iio/adc/max9611.txt b/Documentation/devicetree/bindings/iio/adc/max9611.txt new file mode 100644 index 000..aaaeec0 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/max9611.txt @@ -0,0 +1,27 @@ +* Maxim max9611/max9612 current sense amplifier with 12-bits ADC interface + +Maxim max9611/max9612 is an high-side current sense amplifier with integrated +12-bits ADC communicating over I2c bus. +The device node for this driver shall be a child of a I2c controller. + +Required properties + - compatible: Should be "maxim,max9611" or "maxim,max9612" + - reg: The 7-bits long I2c address of the device + - maxim,shunt-resistor-uohm: Resistor value, in uOhm, of the current sense + shunt resistor. + +Example: + +&i2c4 { + csa: max9611@7c { + compatible = "maxim,max9611"; + reg = <0x7c>; + + maxim,shunt-resistor-uohm = <5000>; + }; +}; + +This device node describes a current sense amplifier sitting on I2c4 bus +with address 0x7c (read address is 0xf9, write address is 0xf8). +A sense resistor of 0,005 Ohm is installed between RS+ and RS- +current-sensing inputs. -- 2.7.4
[PATCH v2 0/4] iio: adc: Maxim max9611 driver
Hello! Second round for Maxim max9611/max9612 high-side current sense amplifier driver. For reference, a simplified integration schematic drawing is here reported: o/\/\/-o---|LOAD|--- |shunt | |__|___ | RS+RS- | | |-gain-| | | | | | | | |max9611 |->| ADC |= I2c |__| public datasheet available at https://datasheets.maximintegrated.com/en/ds/MAX9611-MAX9612.pdf Compared to v1, all channels whose values are calculated using parameters depending on the applied gain have been transformed in "processed" channels. Gain selection procedure is performed in kernel space, and gain-dependent values (current sense amplifier gain and offset) are not exposed to userspace anymore. output reported from iio_info tool: iio:device0: max9611_vdd 6 channels found: voltage0: (input) 1 channel-specific attributes found: attr 0: input value: 4.08500 voltage1: (input) 3 channel-specific attributes found: attr 0: scale value: 14 attr 1: offset value: 1 attr 2: raw value: 59 shunt: (input) 2 channel-specific attributes found: attr 0: resistor_power value: 5000 attr 1: resistor_current value: 5000 power: (input) 1 channel-specific attributes found: attr 0: input value: 663.40400 temp: (input) 2 channel-specific attributes found: attr 0: scale value: 0.480076812 attr 1: raw value: 59 current: (input) 1 channel-specific attributes found: attr 0: input value: 817.0 Tested on Salvator-X M3-W board. Thanks j v1 -> v2: - Drop wildcard (max961x) in driver, documentation and dt-bindings. Use max9611 instead. - Make 3 processed channels for csa voltage, csa current and power load - Remove wrapper functions around i2c buffer access - Add locking in read_raw() - Make 2 separate attributes for shunt resistor: current and power - Renamed shunt resistor attribute - Fixed several review comments Jacopo Mondi (4): Documentation: dt-bindings: iio: Add max9611 ADC iio: Documentation: Add max9611 sysfs documentation iio: adc: Add Maxim max9611 ADC driver arm64: dts: salvator-x: Add current sense amplifiers .../ABI/testing/sysfs-bus-iio-adc-max9611 | 16 + .../devicetree/bindings/iio/adc/max9611.txt| 27 + arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 18 + drivers/iio/adc/Kconfig| 10 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/max9611.c | 590 + 6 files changed, 662 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 create mode 100644 Documentation/devicetree/bindings/iio/adc/max9611.txt create mode 100644 drivers/iio/adc/max9611.c -- 2.7.4
[PATCH v2 2/4] iio: Documentation: Add max9611 sysfs documentation
Add documentation for max9611 driver. Document attributes describing value of shunt resistor installed between RS+ and RS- voltage sense inputs. Signed-off-by: Jacopo Mondi --- Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 | 16 1 file changed, 16 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 b/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 new file mode 100644 index 000..9c60824 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 @@ -0,0 +1,16 @@ +What: /sys/bus/iio/devices/iio:deviceX/in_shunt_resistor_power +Date: March 2017 +KernelVersion: 4.12 +Contact: linux-...@vger.kernel.org +Description: The value of the shunt resistor used to compute power drain on +common input voltage pin (RS+). In micro Ohms. + +What: /sys/bus/iio/devices/iio:deviceX/in_shunt_resistor_current +Date: March 2017 +KernelVersion: 4.12 +Contact: linux-...@vger.kernel.org +Description: The value of the shunt resistor used to compute current flowing +between RS+ and RS- voltage sense inputs. In micro Ohms. + This attributes describe a single physical component, exposed + as two distinct attributes as it is used to calculate two + different values. -- 2.7.4
[PATCH v2 3/4] iio: adc: Add Maxim max9611 ADC driver
Add iio driver for Maxim max9611 and max9612 current-sense amplifiers with 12-bits ADC interface. Datasheet publicly available at: https://datasheets.maximintegrated.com/en/ds/MAX9611-MAX9612.pdf Signed-off-by: Jacopo Mondi --- drivers/iio/adc/Kconfig | 10 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/max9611.c | 590 ++ 3 files changed, 601 insertions(+) create mode 100644 drivers/iio/adc/max9611.c diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index dedae7a..cdd785f 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -354,6 +354,16 @@ config MAX1363 To compile this driver as a module, choose M here: the module will be called max1363. +config MAX9611 + tristate "Maxim max9611/max9612 ADC driver" + depends on I2C + help + Say yes here to build support for Maxim max9611/max9612 current sense + amplifier with 12-bits ADC interface. + + To compile this driver as a module, choose M here: the module will be + called max9611. + config MCP320X tristate "Microchip Technology MCP3x01/02/04/08" depends on SPI diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index d001262..149f979 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -34,6 +34,7 @@ obj-$(CONFIG_LTC2485) += ltc2485.o obj-$(CONFIG_MAX1027) += max1027.o obj-$(CONFIG_MAX11100) += max11100.o obj-$(CONFIG_MAX1363) += max1363.o +obj-$(CONFIG_MAX9611) += max9611.o obj-$(CONFIG_MCP320X) += mcp320x.o obj-$(CONFIG_MCP3422) += mcp3422.o obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o diff --git a/drivers/iio/adc/max9611.c b/drivers/iio/adc/max9611.c new file mode 100644 index 000..f1e5827 --- /dev/null +++ b/drivers/iio/adc/max9611.c @@ -0,0 +1,590 @@ +/* + * iio/adc/max9611.c + * + * Maxim max9611/max9612 high side current sense amplifier with + * 12-bit ADC interface. + * + * Copyright (C) 2017 Jacopo Mondi + * + * 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 driver supports input common-mode voltage, current-sense + * amplifier with programmable gains and die temperature reading from + * Maxim max9611/max9612. + * + * Op-amp, analog comparator, and watchdog functionalities are not + * supported by this driver. + */ + +#include +#include +#include +#include +#include + +#define DRIVER_NAME "max9611" + +/* max9611 register addresses */ +#define MAX9611_REG_CSA_DATA 0x00 +#define MAX9611_REG_RS_DATA0x02 +#define MAX9611_REG_TEMP_DATA 0x08 +#define MAX9611_REG_CTRL1 0x0a +#define MAX9611_REG_CTRL2 0x0b + +/* max9611 REG1 mux configuration options */ +#define MAX9611_MUX_MASK 0x07 +#define MAX9611_MUX_SENSE_1x 0x00 +#define MAX9611_MUX_SENSE_4x 0x01 +#define MAX9611_MUX_SENSE_8x 0x02 +#define MAX9611_INPUT_VOLT 0x03 +#define MAX9611_MUX_TEMP 0x06 + +/* max9611 voltage (both csa and input) helper macros */ +#define MAX9611_VOLTAGE_SHIFT 0x04 +#define MAX9611_VOLTAGE_RAW(_r)((_r) >> MAX9611_VOLTAGE_SHIFT) + +/* + * max9611 current sense amplifier voltage output: + * LSB and offset values depends on selected gain (1x, 4x, 8x) + * + * GAINLSB (nV)OFFSET (LSB steps) + * 1x 107500 1 + * 4x 26880 1 + * 8x 13440 3 + * + * The complete formula to calculate current sense voltage is: + * (((adc_read >> 4) - offset) / ((1 / LSB) * 10^-3) + */ +#define CSA_VOLT_1x_LSB_nV 107500 +#define CSA_VOLT_4x_LSB_nV 26880 +#define CSA_VOLT_8x_LSB_nV 13440 + +#define CSA_VOLT_1x_OFFS_RAW 1 +#define CSA_VOLT_4x_OFFS_RAW 1 +#define CSA_VOLT_8x_OFFS_RAW 3 + +/* + * max9611 common input mode (CIM): LSB is 14mV, with 14mV offset at 25 C + * + * The complete formula to calculate input common voltage is: + * (((adc_read >> 4) * 1000) - offset) / (1 / 14 * 1000) + */ +#define CIM_VOLTAGE_LSB_mV 14 +#define CIM_VOLTAGE_OFFSET_RAW 1 + +/* + * max9611 temperature reading: LSB is 0.48 degrees Celsius + * + * The complete formula to calculate temperature is: + * ((adc_read >> 7) * 1000) / (1 / 0.48 * 1000) + */ +#define TEMP_SHIFT 0x07 +#define TEMP_MAX_RAW_POS 0x7f80 +#define TEMP_MAX_RAW_NEG 0xff80 +#define TEMP_MIN_RAW_NEG 0xd980 +#define TEMP_MASK ((1 << TEMP_SHIFT) - 1) +#define TEMP_RAW(_r) ((_r) >> TEMP_SHIFT) +#define TEMP_LSB_mC480 +#define TEMP_SCALE_NUM 1000 +#define TEMP_SCALE_DIV 2083 + +struct max9611_dev { + stru
[PATCH v2 4/4] arm64: dts: salvator-x: Add current sense amplifiers
Add device nodes for two Maxim max961x current sense amplifiers sensing VDD_08 and DVFS_08 lines. Signed-off-by: Jacopo Mondi --- arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 18 ++ 1 file changed, 18 insertions(+) diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts index fd05c2b..ef8b723 100644 --- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts +++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts @@ -261,6 +261,24 @@ status = "okay"; }; +&i2c4 { + status = "okay"; + + csa_vdd: max9611_vdd@7c { + compatible = "maxim,max9611"; + reg = <0x7c>; + + maxim,shunt-resistor-uohm = <5000>; + }; + + csa_dvfs: max9611_dvfs@7f { + compatible = "maxim,max9611"; + reg = <0x7f>; + + maxim,shunt-resistor-uohm = <5000>; + }; +}; + &wdt0 { timeout-sec = <60>; status = "okay"; -- 2.7.4
Re: [PATCH] arm64: dts: r8a7795: Correct SATA device size to 2MiB
Hi Magnus, On Thu, Mar 23, 2017 at 9:30 AM, Magnus Damm wrote: > On Tue, Mar 21, 2017 at 6:30 PM, Geert Uytterhoeven > wrote: >> On Tue, Mar 21, 2017 at 8:17 AM, Magnus Damm wrote: >>> On Mon, Mar 20, 2017 at 7:57 PM, Geert Uytterhoeven >>> wrote: On Mon, Mar 20, 2017 at 9:49 AM, Magnus Damm wrote: > --- 0001/arch/arm64/boot/dts/renesas/r8a7795.dtsi > +++ work/arch/arm64/boot/dts/renesas/r8a7795.dtsi 2017-03-20 > 17:41:36.390607110 +0900 > @@ -1209,7 +1209,7 @@ > > sata: sata@ee30 { > compatible = "renesas,sata-r8a7795"; > - reg = <0 0xee30 0 0x1fff>; > + reg = <0 0xee30 0 0x20>; While the datasheet does mention the 2 MiB area, it also says no (write) access should be made to registers not listed in the table, while these are all covered by the existing area? >>> >>> That bit about not writing to non-listed registers seems like just >>> common sense to me, but perhaps there is more to the story than just >> >> Sure. >> >>> that? Like you say it is probably possible to use the driver with the >>> existing 8K-1 size, but in my mind we should use window sizes defined >>> in the data sheet for DT? >> >> The 2 MiB window size is a lot larger than needed to cover all documented >> rregisters. Either there are more undocumented registers, or the hardware >> engineers were lazy and just decoded the full 2 MiB block. > > Yeah, and on top of that it looks to me like the size is not aligned > to the offset either. I would expect a 2MiB block to be aligned to a > 2MiB boundary... The base address was aligned to 2 miB on R-Car H1. As of R-Car Gen2, it's no longer aligned. Too much copy-'n-paste... BTW, what about the Reference Clock Source Select Register, which lies in a further undocumented area? >>> >>> Yeah, no idea. This would be good task for the I/O or Core group to >> >> SATA is I/O. > > For sure, however I wonder how the external clock register REFSEL is > supposed to be handled. It looks like an external hardware block > somehow. It's a single bit to select between internal and external clock. No documention about which external clock. >>> figure out how to handle. I'm surprised that the SATA DT device nodes >>> with the strange and incorrect 0x1fff size got merged upstream without >>> anyone thinking about that register that you are mentioning. Seems >>> like supporting that should be part of SATA development for R-Car >>> Gen3? >> >> Probably it was just an oversight. I almost missed it myself when >> reviewing your patch. > > Probably yes. It was up-ported from a patch in the BSP, which may predate the datasheet revision that documented REFSEL. >> The register is not present (not documented) on R-Car H1 and Gen2. >> The IP core is derived from SH-Navi2G (sh7775), but no datasheet. > > On R-Car Gen3 there seem to be a chance that random glue hardware for > other devices may also be present in the 0xe65e range however > documentations seem to be lacking... And without documentation... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH] arm64: dts: r8a7795: Correct SATA device size to 2MiB
Hi Geert, On Tue, Mar 21, 2017 at 6:30 PM, Geert Uytterhoeven wrote: > Hi Magnus, > > On Tue, Mar 21, 2017 at 8:17 AM, Magnus Damm wrote: >> On Mon, Mar 20, 2017 at 7:57 PM, Geert Uytterhoeven >> wrote: >>> On Mon, Mar 20, 2017 at 9:49 AM, Magnus Damm wrote: --- 0001/arch/arm64/boot/dts/renesas/r8a7795.dtsi +++ work/arch/arm64/boot/dts/renesas/r8a7795.dtsi 2017-03-20 17:41:36.390607110 +0900 @@ -1209,7 +1209,7 @@ sata: sata@ee30 { compatible = "renesas,sata-r8a7795"; - reg = <0 0xee30 0 0x1fff>; + reg = <0 0xee30 0 0x20>; >>> >>> While the datasheet does mention the 2 MiB area, it also says no (write) >>> access should be made to registers not listed in the table, while these are >>> all covered by the existing area? >> >> That bit about not writing to non-listed registers seems like just >> common sense to me, but perhaps there is more to the story than just > > Sure. > >> that? Like you say it is probably possible to use the driver with the >> existing 8K-1 size, but in my mind we should use window sizes defined >> in the data sheet for DT? > > The 2 MiB window size is a lot larger than needed to cover all documented > rregisters. Either there are more undocumented registers, or the hardware > engineers were lazy and just decoded the full 2 MiB block. Yeah, and on top of that it looks to me like the size is not aligned to the offset either. I would expect a 2MiB block to be aligned to a 2MiB boundary... >>> BTW, what about the Reference Clock Source Select Register, which lies >>> in a further undocumented area? >> >> Yeah, no idea. This would be good task for the I/O or Core group to > > SATA is I/O. For sure, however I wonder how the external clock register REFSEL is supposed to be handled. It looks like an external hardware block somehow. >> figure out how to handle. I'm surprised that the SATA DT device nodes >> with the strange and incorrect 0x1fff size got merged upstream without >> anyone thinking about that register that you are mentioning. Seems >> like supporting that should be part of SATA development for R-Car >> Gen3? > > Probably it was just an oversight. I almost missed it myself when > reviewing your patch. Probably yes. > The register is not present (not documented) on R-Car H1 and Gen2. > The IP core is derived from SH-Navi2G (sh7775), but no datasheet. On R-Car Gen3 there seem to be a chance that random glue hardware for other devices may also be present in the 0xe65e range however documentations seem to be lacking... / magnus
Re: [PATCH v4] mmc: sh_mmcif: Document r7s72100 DT bindings
On 22 March 2017 at 15:42, Chris Brandt wrote: > Signed-off-by: Chris Brandt > Reviewed-by: Geert Uytterhoeven > Reviewed-by: Simon Horman > Acked-by: Rob Herring Thanks, applied for next! Kind regards Uffe > --- > v4: > * added Reviewed-by and Acked-by > v3: > * added list of how many interrupts each SOC has > v2: > * added interrupt description > --- > Documentation/devicetree/bindings/mmc/renesas,mmcif.txt | 8 > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt > b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt > index e4ba92aa..c32dc5a 100644 > --- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt > +++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt > @@ -8,6 +8,7 @@ Required properties: > > - compatible: should be "renesas,mmcif-", "renesas,sh-mmcif" as a >fallback. Examples with are: > + - "renesas,mmcif-r7s72100" for the MMCIF found in r7s72100 SoCs > - "renesas,mmcif-r8a73a4" for the MMCIF found in r8a73a4 SoCs > - "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs > - "renesas,mmcif-r8a7778" for the MMCIF found in r8a7778 SoCs > @@ -17,6 +18,13 @@ Required properties: > - "renesas,mmcif-r8a7794" for the MMCIF found in r8a7794 SoCs > - "renesas,mmcif-sh73a0" for the MMCIF found in sh73a0 SoCs > > +- interrupts: Some SoCs have only 1 shared interrupt, while others have > either > + 2 or 3 individual interrupts (error, int, card detect). Below is the number > + of interrupts for each SoC: > +1: r8a73a4, r8a7778, r8a7790, r8a7791, r8a7793, r8a7794 > +2: r8a7740, sh73a0 > +3: r7s72100 > + > - clocks: reference to the functional clock > > - dmas: reference to the DMA channels, one per channel name listed in the > -- > 2.10.1 > >