* Re: [PATCH 1/2] leds: Add driver for Qualcomm LPG
From: Jacek Anaszewski @ 2017-04-09 12:32 UTC (permalink / raw)
To: Pavel Machek, Bjorn Andersson
Cc: Rob Herring, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170408133904.GA9020@amd>
Hi,
On 04/08/2017 03:39 PM, Pavel Machek wrote:
> Hi!
>
>> [..]
>>>> For the patterns I don't know how a trigger for this would look like,
>>>> how would setting the pattern of a trigger be propagated down to the
>>>> hardware?
>>>
>>> We'd need a new op and API similar to blink_set()/led_blink_set().
>>>
>>
>> I've tried to find different LED circuits with some sort of pattern
>> generator in an attempt to figure out how to design this interface, but
>> turned out to be quite hard to find examples; the three I can compare
>> are:
>>
>> * LP5xx series "implements" pattern generation by executing code.
>>
>> * Qualcomm LPG iterates over 2-64 brightness-values in a pattern, at a
>> fixed rate with knobs to configure what happens before starting and
>> after finishing iterating over the defined values. It does not support
>> smooth transitions between values.
>>
>> * AS3676 supports a pattern of 32 values controlling if the output
>> should be enabled or disabled for each 32.5ms (or 250ms) time period.
>> The delay before repeating the pattern can be configured. It support
>> smooth transitions between the states.
>>
>>
>> So, while I think I see how you would like to architect this interface I
>> am not sure how to figure out the details.
>>
>> The pattern definition would have to be expressive enough to support the
>> features of LP5xx and direct enough to support the limited AS3676. It
>> would likely have to express transitions, so that the LPG could generate
>> intermediate steps (and we will have to adapt the resolution of the
>> ramps based on the other LPGs in the system).
>>
>> How do we do with patterns that are implementable by the LP5xx but are
>> not with the LPG? Should we reject those or should we do some sort of
>> best-effort approach in the kernel?
>
> Lets say you get series of
>
> (red, green, blue, delta_t )
In order to make it possible we'd have to have a means for mapping
LED class devices to red, green and blue. In effect I see the problem
of introducing a new mechanism for creating compound LED class device
out of existing LED class devices as the first one to address.
Once we have compound LED class device, that would expose an interface
for operating on the particular color brightnesses, then we can build
upon it the pattern engine. Actually, the same compound LED mechanism
would be necessary for defining blink patterns for strings of monochrome
LEDs.
> points, meaning "in delta_t msec, change color to red, green,
> blue. Lets ignore other channels for now. delta_t of 0 would be step
> change. Would such interface work for you?
>
> Simple compiler from this to LP5XX code should not be hard to
> do. AS3676 ... I'm not sure what to do, AFAICT it is too limited.
Our new API for setting blink patterns could be defined so that it
does not guarantee setting the requested pattern, but applies only
what the hardware can support and returns the applied settings.
E.g. this way the driver could reduce the requested brightness
transition resolution.
There is also a question if we should provide software fallback
for patterns not supported by the hardware and to what extent.
In case of blink support for a single LED we do that but in case
of more complex patterns it would require more complex logic,
and at least for now I'd avoid it.
--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] Extend pca9532 device tree support
From: Jacek Anaszewski @ 2017-04-09 12:37 UTC (permalink / raw)
To: Felix Brack, Pavel Machek
Cc: rpurdie-Fm38FmjxZ/leoWH0uzbU5w, mark.rutland-5wv7dgnIgG8,
riku.voipio-X3B1VOXEql0, linux-leds-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ec012568-f540-4e87-d8c4-4065fbcd1c2a-GovowT2ENgg@public.gmane.org>
Hello Felix,
On 04/07/2017 10:22 AM, Felix Brack wrote:
> Hello Jacek,
>
> On 06.04.2017 21:00, Jacek Anaszewski wrote:
>> Hi Pavel,
>>
>> On 04/06/2017 05:50 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>>>> diff --git a/Documentation/devicetree/bindings/leds/leds-pca9532.txt b/Documentation/devicetree/bindings/leds/leds-pca9532.txt
>>>>> index 198f3ba..8374075 100644
>>>>> --- a/Documentation/devicetree/bindings/leds/leds-pca9532.txt
>>>>> +++ b/Documentation/devicetree/bindings/leds/leds-pca9532.txt
>>>>> @@ -17,6 +17,8 @@ Optional sub-node properties:
>>>>> - label: see Documentation/devicetree/bindings/leds/common.txt
>>>>> - type: Output configuration, see dt-bindings/leds/leds-pca9532.h (default NONE)
>>>>> - linux,default-trigger: see Documentation/devicetree/bindings/leds/common.txt
>>>>> + - default-state: see Documentation/devicetree/bindings/leds/common.txt
>>>>> + This property is only valid for sub-nodes of type <PCA9532_TYPE_LED>.
>>>>>
>>>>> Example:
>>>>> #include <dt-bindings/leds/leds-pca9532.h>
>>>>> @@ -33,6 +35,14 @@ Example:
>>>>> label = "pca:green:power";
>>>>> type = <PCA9532_TYPE_LED>;
>>>>> };
>>>>> + kernel-booting {
>>>>> + type = <PCA9532_TYPE_LED>;
>>>>> + default-state = "on";
>>>>> + };
>>>>> + sys-stat {
>>>>> + type = <PCA9532_TYPE_LED>;
>>>>> + default-state = "keep"; // don't touch, was set by U-Boot
>>>>> + };
>>>>
>>>> Adjusted above indentation to match the preceding lines.
>>>
>>>>> @@ -475,6 +494,16 @@ pca9532_of_populate_pdata(struct device *dev, struct device_node *np)
>>>>> of_property_read_u32(child, "type", &pdata->leds[i].type);
>>>>> of_property_read_string(child, "linux,default-trigger",
>>>>> &pdata->leds[i].default_trigger);
>>>>> + if (!of_property_read_string(child, "default-state", &state)) {
>>>>> + if (!strcmp(state, "on"))
>>>>> + pdata->leds[i].state = PCA9532_ON;
>>>>> + else if (!strcmp(state, "keep"))
>>>>> + pdata->leds[i].state = PCA9532_KEEP;
>>>>> + else if (!strcmp(state, "pwm0"))
>>>>> + pdata->leds[i].state = PCA9532_PWM0;
>>>>> + else if (!strcmp(state, "pwm1"))
>>>>> + pdata->leds[i].state = PCA9532_PWM1;
>>>>> + }
>>>>> if (++i >= maxleds) {
>>>>> of_node_put(child);
>>>>> break;
>>>
>>> This seems to look for "pwm0" and "pwm1" strings, which do not seem to
>>> be documented.
>>>
>>> Plus... is it useful to have default-state? We already have default
>>> trigger. If we keep the value by default (on PC, we do something like
>>> that) this patch should not be neccessary?
>>
>> Thanks for the heads-up. Dropping the patch for now.
>
> No, please do not drop the patch.
>
>> I guess that pwm0/1 got propagated to v2 by an omission.
>>
>
> Yes, I agree. However the two strings do not break anything and behave
> analog to the 'on' or 'keep' string. Though this code could be removed
> if absolutely necessary. An alternative would be to add a description
> for the strings. Just to be clear: these strings have nothing to with
> the exposition of device specific registers to the DT.
>
>> Regarding default-on: Felix, do you have any use case that require
>> default-on set to "keep"?
>>
>
> This patch is not about 'default-on' which is a value that could be
> assigned to the property 'linux,default-trigger' (according to DT
> bindings documentation file 'common.txt').
> My patch does not introduce anything new with the'keep' state, it rather
> completes the existing bindings according to the description in
> Documentation/devicetree/bindings/leds/common.txt which states:
>
> ....
> - default-state : The initial state of the LED. Valid values are "on",
> "off", and "keep". If the LED is already on or off and the default-state
> property is set the to same value, then no glitch should be produced
> where the LED momentarily turns off (or on). The "keep" setting will
> keep the LED at whatever its current state is, without producing a
> glitch. The default is off if this property is not present.
> ....
>
> One of my use cases is to turn a LED on by U-Boot. This LED must remain
> on until eventually, under certain conditions, some userland code
> changes it's state.
> Setting 'default-state' to 'keep' is how you can sort of tell the
> kernel, or better the driver, 'not to initialize the LED' which would
> turn it off.
Thanks for the explanation. Could you please sent v3 with removed pwm*
cases then?
--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] Extend pca9532 device tree support
From: Felix Brack @ 2017-04-09 13:11 UTC (permalink / raw)
To: Jacek Anaszewski, Pavel Machek
Cc: rpurdie-Fm38FmjxZ/leoWH0uzbU5w, mark.rutland-5wv7dgnIgG8,
riku.voipio-X3B1VOXEql0, linux-leds-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2f3815ab-71e1-b63d-aba8-167e11f719de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hello Jacek,
On 09.04.2017 14:37, Jacek Anaszewski wrote:
> Hello Felix,
>
> On 04/07/2017 10:22 AM, Felix Brack wrote:
>> Hello Jacek,
>>
>> On 06.04.2017 21:00, Jacek Anaszewski wrote:
>>> Hi Pavel,
>>>
>>> On 04/06/2017 05:50 PM, Pavel Machek wrote:
>>>> Hi!
>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/leds/leds-pca9532.txt b/Documentation/devicetree/bindings/leds/leds-pca9532.txt
>>>>>> index 198f3ba..8374075 100644
>>>>>> --- a/Documentation/devicetree/bindings/leds/leds-pca9532.txt
>>>>>> +++ b/Documentation/devicetree/bindings/leds/leds-pca9532.txt
>>>>>> @@ -17,6 +17,8 @@ Optional sub-node properties:
>>>>>> - label: see Documentation/devicetree/bindings/leds/common.txt
>>>>>> - type: Output configuration, see dt-bindings/leds/leds-pca9532.h (default NONE)
>>>>>> - linux,default-trigger: see Documentation/devicetree/bindings/leds/common.txt
>>>>>> + - default-state: see Documentation/devicetree/bindings/leds/common.txt
>>>>>> + This property is only valid for sub-nodes of type <PCA9532_TYPE_LED>.
>>>>>>
>>>>>> Example:
>>>>>> #include <dt-bindings/leds/leds-pca9532.h>
>>>>>> @@ -33,6 +35,14 @@ Example:
>>>>>> label = "pca:green:power";
>>>>>> type = <PCA9532_TYPE_LED>;
>>>>>> };
>>>>>> + kernel-booting {
>>>>>> + type = <PCA9532_TYPE_LED>;
>>>>>> + default-state = "on";
>>>>>> + };
>>>>>> + sys-stat {
>>>>>> + type = <PCA9532_TYPE_LED>;
>>>>>> + default-state = "keep"; // don't touch, was set by U-Boot
>>>>>> + };
>>>>>
>>>>> Adjusted above indentation to match the preceding lines.
>>>>
>>>>>> @@ -475,6 +494,16 @@ pca9532_of_populate_pdata(struct device *dev, struct device_node *np)
>>>>>> of_property_read_u32(child, "type", &pdata->leds[i].type);
>>>>>> of_property_read_string(child, "linux,default-trigger",
>>>>>> &pdata->leds[i].default_trigger);
>>>>>> + if (!of_property_read_string(child, "default-state", &state)) {
>>>>>> + if (!strcmp(state, "on"))
>>>>>> + pdata->leds[i].state = PCA9532_ON;
>>>>>> + else if (!strcmp(state, "keep"))
>>>>>> + pdata->leds[i].state = PCA9532_KEEP;
>>>>>> + else if (!strcmp(state, "pwm0"))
>>>>>> + pdata->leds[i].state = PCA9532_PWM0;
>>>>>> + else if (!strcmp(state, "pwm1"))
>>>>>> + pdata->leds[i].state = PCA9532_PWM1;
>>>>>> + }
>>>>>> if (++i >= maxleds) {
>>>>>> of_node_put(child);
>>>>>> break;
>>>>
>>>> This seems to look for "pwm0" and "pwm1" strings, which do not seem to
>>>> be documented.
>>>>
>>>> Plus... is it useful to have default-state? We already have default
>>>> trigger. If we keep the value by default (on PC, we do something like
>>>> that) this patch should not be neccessary?
>>>
>>> Thanks for the heads-up. Dropping the patch for now.
>>
>> No, please do not drop the patch.
>>
>>> I guess that pwm0/1 got propagated to v2 by an omission.
>>>
>>
>> Yes, I agree. However the two strings do not break anything and behave
>> analog to the 'on' or 'keep' string. Though this code could be removed
>> if absolutely necessary. An alternative would be to add a description
>> for the strings. Just to be clear: these strings have nothing to with
>> the exposition of device specific registers to the DT.
>>
>>> Regarding default-on: Felix, do you have any use case that require
>>> default-on set to "keep"?
>>>
>>
>> This patch is not about 'default-on' which is a value that could be
>> assigned to the property 'linux,default-trigger' (according to DT
>> bindings documentation file 'common.txt').
>> My patch does not introduce anything new with the'keep' state, it rather
>> completes the existing bindings according to the description in
>> Documentation/devicetree/bindings/leds/common.txt which states:
>>
>> ....
>> - default-state : The initial state of the LED. Valid values are "on",
>> "off", and "keep". If the LED is already on or off and the default-state
>> property is set the to same value, then no glitch should be produced
>> where the LED momentarily turns off (or on). The "keep" setting will
>> keep the LED at whatever its current state is, without producing a
>> glitch. The default is off if this property is not present.
>> ....
>>
>> One of my use cases is to turn a LED on by U-Boot. This LED must remain
>> on until eventually, under certain conditions, some userland code
>> changes it's state.
>> Setting 'default-state' to 'keep' is how you can sort of tell the
>> kernel, or better the driver, 'not to initialize the LED' which would
>> turn it off.
>
> Thanks for the explanation. Could you please sent v3 with removed pwm*
> cases then?
>
Yes, I will try to do so next week.
--
regards Felix
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V3 2/3] mtd: add core code reading DT specified part probes
From: Boris Brezillon @ 2017-04-09 13:28 UTC (permalink / raw)
To: Rafał Miłecki
Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
Cyrille Pitchen, Rob Herring, Mark Rutland, Frank Rowand,
Linus Walleij, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rafał Miłecki
In-Reply-To: <20170409130406.564802a4@bbrezillon>
On Sun, 9 Apr 2017 13:04:06 +0200
Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> static char **mtd_alloc_part_type_table(int nentries)
Oops, s/char **/const char **/
> {
> return kzalloc((nentries + 1) * sizeof(*res), GFP_KERNEL);
> }
>
> static void mtd_free_part_type_table(const char * const *table)
> {
> kfree(table);
> }
I realize this might not be suitable for all kind of part-probes
definitions. Some might need to dynamically allocate each string and
expect the core to free them in mtd_free_part_type_table() (the one I
have in mind is the cmdline part-probes parser). Others already have
the strings statically defined or allocated and maintained somewhere
else (this is the case with DT which provides direct access to string
definitions), which means the core shouldn't free them.
I see 3 solutions to this problem:
1/ go back to your initial solution with DT specific functions, and
wait until someone decides to implement another way to define
part-probes (cmdline or ACPI) before considering a more complex
solution
2/ always allocate strings dynamically and let
mtd_free_part_type_table() free them. This implies using kstrdup() on
strings returned by of_property_read_string_array()
3/ use something smarter to let the part-probes table creator free it,
for example, by using something like:
struct mtd_part_probes {
const char * const *types;
void (*free)(const char * const *types);
}
#3 is overkill IMO. I'm fine with #1 and #2, pick the one you prefer.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 14/37] mtd: nand: denali: support "nand-ecc-strength" DT property
From: Boris Brezillon @ 2017-04-09 16:33 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Enrico Jorns,
Artem Bityutskiy, Dinh Nguyen, Marek Vasut, Graham Moore,
David Woodhouse, Masami Hiramatsu, Chuanxiao Dong, Jassi Brar,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
Brian Norris, Richard Weinberger, Cyrille Pitchen, Rob Herring,
Mark Rutland
In-Reply-To: <CAK7LNAToTmirpkhNmPCLhcTXG_SFqS762mEGK3mjyqLKXuWa1Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, 3 Apr 2017 12:16:34 +0900
Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
> Hi Boris,
>
>
>
> 2017-03-31 18:46 GMT+09:00 Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
>
> > You can try something like that when no explicit ecc.strength and
> > ecc.size has been set in the DT and when ECC_MAXIMIZE was not passed.
> >
> > static int
> > denali_get_closest_ecc_strength(struct denali_nand_info *denali,
> > int strength)
> > {
> > /*
> > * Whatever you need to select a strength that is greater than
> > * or equal to strength.
> > */
> >
> > return X;
> > }
>
>
> Is here anything specific to Denali?
Well, only the denali driver knows what the hardware supports, though
having a generic function that takes a table of supported strengths
would work.
>
>
> > static int denali_try_to_match_ecc_req(struct denali_nand_info *denali)
> > {
> > struct nand_chip *chip = &denali->nand;
> > struct mtd_info *mtd = nand_to_mtd(chip);
> > int max_ecc_bytes = mtd->oobsize - denali->bbtskipbytes;
> > int ecc_steps, ecc_strength, ecc_bytes;
> > int ecc_size = chip->ecc_step_ds;
> > int ecc_strength = chip->ecc_strength_ds;
> >
> > /*
> > * No information provided by the NAND chip, let the core
> > * maximize the strength.
> > */
> > if (!ecc_size || !ecc_strength)
> > return -ENOTSUPP;
> >
> > if (ecc_size > 512)
> > ecc_size = 1024;
> > else
> > ecc_size = 512;
> >
> > /* Adjust ECC step size based on hardware support. */
> > if (ecc_size == 1024 &&
> > !(denali->caps & DENALI_CAP_ECC_SIZE_1024))
> > ecc_size = 512;
> > else if(ecc_size == 512 &&
> > !(denali->caps & DENALI_CAP_ECC_SIZE_512))
> > ecc_size = 1024;
> >
> > if (ecc_size < chip->ecc_size_ds) {
> > /*
> > * When the selected size if smaller than the expected
> > * one we try to use the same strength but on 512 blocks
> > * so that we can still fix the same number of errors
> > * even if they are concentrated in the first 512bytes
> > * of a 1024bytes portion.
> > */
> > ecc_strength = chip->ecc_strength_ds;
> > ecc_strength = denali_get_closest_ecc_strength(denali,
> > ecc_strength);
> > } else {
> > /* Always prefer 1024bytes ECC blocks when possible. */
> > if (ecc_size != 1024 &&
> > (denali->caps & DENALI_CAP_ECC_SIZE_1024) &&
> > mtd->writesize > 1024)
> > ecc_size = 1024;
> >
> > /*
> > * Adjust the strength based on the selected ECC step
> > * size.
> > */
> > ecc_strength = DIV_ROUND_UP(ecc_size,
> > chip->ecc_step_ds) *
> > chip->ecc_strength_ds;
> > }
> >
> > ecc_bytes = denali_calc_ecc_bytes(ecc_size,
> > ecc_strength);
> > ecc_bytes *= mtd->writesize / ecc_size;
> >
> > /*
> > * If we don't have enough space, let the core maximize
> > * the strength.
> > */
> > if (ecc_bytes > max_ecc_bytes)
> > return -ENOTSUPP;
> >
> > chip->ecc.strength = ecc_strength;
> > chip->ecc.size = ecc_size;
> >
> > return 0;
> > }
>
>
> As a whole, this does not seem to driver-specific.
It's almost controller-agnostic, except for the denali_calc_ecc_bytes()
function, but I guess we could ask drivers to implement a hook that is
passed the ECC step size and strength and returns the associated
number of ECC bytes.
>
>
> [1] A driver provides some pairs of (ecc_strength, ecc_size)
> it can support.
>
> [2] The core framework knows the chip's requirement
> (ecc_strength_ds, ecc_size_ds).
>
>
> Then, the core framework provides a function
> to return a most recommended (ecc_strength, ecc_size).
>
>
>
> struct nand_ecc_spec {
> int ecc_strength;
> int ecc_size;
> };
>
> /*
> * This function choose the most recommented (ecc_str, ecc_size)
> * "recommended" means: minimum ecc stregth that meets
> * the chip's requirment.
> *
> *
> * @chip - nand_chip
> * @controller_ecc_spec - Array of (ecc_str, ecc_size) supported by the
> controller. (terminated by NULL as sentinel)
> */
> struct nand_ecc_spec * nand_try_to_match_ecc_req(struct nand_chip *chip,
> struct nand_ecc_spec
> *controller_ecc_spec)
> {
> /*
> * Return the pointer to the most recommended
> * struct nand_ecc_spec.
> * If nothing suitable found, return NULL.
> */
> }
>
I like the idea, except I would do this slightly differently to avoid
declaring all combinations of stepsize and strengths
struct nand_ecc_stepsize_info {
int stepsize;
int nstrengths;
int *strengths;
};
struct nand_ecc_engine_caps {
int nstepsizes;
struct nand_ecc_stepsize_info *stepsizes;
int (*calc_ecc_bytes)(int stepsize, int strength);
};
int nand_try_to_match_ecc_req(struct nand_chip *chip,
const struct nand_ecc_engine_caps *caps,
struct nand_ecc_spec *spec)
{
/*
* Find the most appropriate setting based on the ECC engine
* caps and fill the spec object accordingly.
* Returns 0 in case of success and a negative error code
* otherwise.
*/
}
Note that nand_try_to_match_ecc_req() has to be more generic than
denali_try_to_match_ecc_req() WRT step sizes, which will probably
complexify the logic.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v5] dt-bindings: fpga: Add bindings document for Xilinx LogiCore PR Decoupler
From: Moritz Fischer @ 2017-04-09 17:10 UTC (permalink / raw)
To: linux-fpga-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Moritz Fischer, Michal Simek,
Sören Brinkmann
This adds the binding documentation for the Xilinx LogiCORE PR
Decoupler soft core.
Signed-off-by: Moritz Fischer <mdf-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Michal Simek <michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
Acked-by: Alan Tull <atull-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Sören Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
Changes from v4:
- Ssubject line
- Replaced 'or' by 'followed by' as suggested by Rob
Changes from v3:
- Addressed Michal's comments
- Addressed Alan's Comments
- Added Alan's Acked-by
Changes from v2:
- Added refence to generic fpga-region bindings
- Fixed up reg property in example
- Added fallback to "xlnx,pr-decoupler" without version
Changes from v1:
- Added clock names & clock to example
- Merged some of the description from Michal's version
---
.../bindings/fpga/xilinx-pr-decoupler.txt | 36 ++++++++++++++++++++++
1 file changed, 36 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
diff --git a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
new file mode 100644
index 0000000..b2c58fb
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
@@ -0,0 +1,36 @@
+Xilinx LogiCORE Partial Reconfig Decoupler Softcore
+
+The Xilinx LogiCORE Partial Reconfig Decoupler manages one or more
+decouplers / fpga bridges.
+The controller can decouple/disable the bridges which prevents signal
+changes from passing through the bridge. The controller can also
+couple / enable the bridges which allows traffic to pass through the
+bridge normally.
+
+The Driver supports only MMIO handling. A PR region can have multiple
+PR Decouplers which can be handled independently or chained via decouple/
+decouple_status signals.
+
+Required properties:
+- compatible : Should contain "xlnx,pr-decoupler-1.00" followed by
+ "xlnx,pr-decoupler"
+- regs : base address and size for decoupler module
+- clocks : input clock to IP
+- clock-names : should contain "aclk"
+
+Optional properties:
+- bridge-enable : 0 if driver should disable bridge at startup
+ 1 if driver should enable bridge at startup
+ Default is to leave bridge in current state.
+
+See Documentation/devicetree/bindings/fpga/fpga-region.txt for generic bindings.
+
+Example:
+ fpga-bridge@100000450 {
+ compatible = "xlnx,pr-decoupler-1.00",
+ "xlnx-pr-decoupler";
+ regs = <0x10000045 0x10>;
+ clocks = <&clkc 15>;
+ clock-names = "aclk";
+ bridge-enable = <0>;
+ };
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v5 0/4] gpio: mvebu: Add PWM fan support
From: Ralph Sennhauser @ 2017-04-09 18:09 UTC (permalink / raw)
To: Thierry Reding
Cc: Ralph Sennhauser, Linus Walleij, Alexandre Courbot, Rob Herring,
Mark Rutland, Jason Cooper, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, Russell King, linux-pwm, linux-gpio,
devicetree, linux-kernel, linux-arm-kernel
Hi Therry,
Resending this as v5 with some minor changes since v4. What is missing is
an ACK from you so Linus can merge the driver and Gregory the dts
changes. For this driver to make it into 4.12 it would be nice to have
it in next soon. I hope you can make some room in your schedule to have
another look at this series.
Thanks
Ralph
---
Notes:
About npwm = 1:
The only way I can think of to achieve that requires reading the
GPIO line from the device tree. This would prevent a user to
dynamically choose a line. Which is fine for the fan found on Mamba
but let's take some development board with freely accessible GPIOs
and suddenly we limit the use of this driver. Given the above, npwm
= ngpio with only one usable at a time is a more accurate
description of the situation. The only downside is some "wasted"
space.
About the new compatible string:
Orion was chosen for the SoC variant for the same reason as in
commit 5f79c651e81e ("arm: mvebu: use global interrupts for GPIOs on
Armada XP").
The "pwm" property remains optional for the new compatible string so
the compatiple string "marvell,armada-370-xp-gpio" can be used by
all and not just the first two GPIO chips. A property to select "Set
A" / "Set B" registers could be invented though.
---
Pending:
* Needs ACK from Thierry Reding to be merged via linux-gpio tree by Linus
Walleij. (fine with the general approach, requested changes which
should have been taken care of now)
---
Changes v4->v5:
All
* add Tested-by: Andrew Lunn <andrew@lunn.ch>, thanks
Patch 2/4 mvebu: xp: Add PWM properties to .dtsi files
* keep the old compatible stings, we don't have to drop them,
therefore keep them (suggested by Gregory CLEMENT)
* subject starts with ARM: dts: mvebu: (suggested by Gregory CLEMENT)
Patch 4/4 mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan
* subject starts with ARM: dts: armada-xp: (suggested by Gregory CLEMENT)
Changes v3->v4:
Patch 1/4 gpio: mvebu: Add limited PWM support:
* braces for both branches in if statement if one needs it. (suggested
by Andrew Lunn)
* introduce compatible string marvell,armada-370-xp-gpio (suggest by
Rob Herring)
* fix mvebu_pwmreg_blink_on_duration -> mvebu_pwmreg_blink_off_duration
for period callculation in mvebu_pwm_get_state()
Patch 4/4 mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan
* Drop flags from pwms for Mamba, as no longer used (suggested by
Andrew Lunn)
* Use again #pwm-cell = 2, the second cell is actually the period.
Changes v2->v3:
Patch 1/4 gpio: mvebu: Add limited PWM support:
* drop pin from mvebu_pwn, can be infered (suggested by Thierry Reding)
* rename pwm to mvpwm so pwm can be used for pwm_device as in the API,
avoids some mental gymnastic.
* drop id from struct mvebu_gpio_chip, select blink counter in
mvebu_pwm_probe for all lines instead. We do not care about the
unused ones. I think a clear improvement in readability.
Makes coming up with a good comment simple as well.
* Switch to new atomic PWM API (suggested by Thierry Reding)
* rename use mvebu_gpioreg_blink_select to
mvebu_gpioreg_blink_counter_select.
* mark *_suspend() / *_resume() as __maybe_unused (suggested by Linus
Walleij)
* document #pwm-cells = 1 (suggested by Thierry Reding)
Patch 2/4 mvebu: xp: Add PWM properties to .dtsi files
* add missing reg-names / #pwm-cell properties to
armada-xp-mv78260.dtsi gpio1 node
* set pwm-cells = 1 (suggested by Thierry Reding)
All:
* always uppercase GPIO/PWM in prose (suggested by Thierry Reding)
Changes v1 -> v2:
Patch 1/4 gpio: mvebu: Add limited PWM support:
* use BIT macro (suggested by Linus Walleij)
* move id from struct mvebu_pwm to struct mvebu_gpio_chip, implement
blink select as if else and comment on the chip id for code clarity
(to accommodate Linus Walleijs request for a code clarification /
comment. If you can word it better I'm all ears.)
* Move function comment mvebu_pwm_probe into the function itself.
---
Andrew Lunn (4):
gpio: mvebu: Add limited PWM support
ARM: dts: mvebu: Add PWM properties to .dtsi files
ARM: mvebu: Enable SENSORS_PWM_FAN in defconfig
ARM: dts: armada-xp: Use pwm-fan rather than gpio-fan
.../devicetree/bindings/gpio/gpio-mvebu.txt | 32 ++
MAINTAINERS | 2 +
arch/arm/boot/dts/armada-370.dtsi | 19 +-
arch/arm/boot/dts/armada-xp-linksys-mamba.dts | 8 +-
arch/arm/boot/dts/armada-xp-mv78230.dtsi | 16 +-
arch/arm/boot/dts/armada-xp-mv78260.dtsi | 19 +-
arch/arm/boot/dts/armada-xp-mv78460.dtsi | 19 +-
arch/arm/configs/mvebu_v7_defconfig | 2 +
drivers/gpio/gpio-mvebu.c | 324 ++++++++++++++++++++-
9 files changed, 405 insertions(+), 36 deletions(-)
--
2.10.2
^ permalink raw reply
* [PATCH v5 1/4] gpio: mvebu: Add limited PWM support
From: Ralph Sennhauser @ 2017-04-09 18:09 UTC (permalink / raw)
To: Thierry Reding
Cc: Andrew Lunn, Ralph Sennhauser, Linus Walleij, Alexandre Courbot,
Rob Herring, Mark Rutland, Jason Cooper, Gregory Clement,
Sebastian Hesselbarth, Russell King,
linux-pwm-u79uwXL29TY76Z2rM5mHXA,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170409180931.4884-1-ralph.sennhauser-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
Armada 370/XP devices can 'blink' GPIO lines with a configurable on
and off period. This can be modelled as a PWM.
However, there are only two sets of PWM configuration registers for
all the GPIO lines. This driver simply allows a single GPIO line per
GPIO chip of 32 lines to be used as a PWM. Attempts to use more return
EBUSY.
Due to the interleaving of registers it is not simple to separate the
PWM driver from the GPIO driver. Thus the GPIO driver has been
extended with a PWM driver.
Signed-off-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
URL: https://patchwork.ozlabs.org/patch/427287/
URL: https://patchwork.ozlabs.org/patch/427295/
[Ralph Sennhauser:
* Port forward
* Merge PWM portion into gpio-mvebu.c
* Switch to atomic PWM API
* Add new compatible string marvell,armada-370-xp-gpio
* Update and merge documentation patch
* Update MAINTAINERS]
Signed-off-by: Ralph Sennhauser <ralph.sennhauser-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Tested-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
---
.../devicetree/bindings/gpio/gpio-mvebu.txt | 32 ++
MAINTAINERS | 2 +
drivers/gpio/gpio-mvebu.c | 324 ++++++++++++++++++++-
3 files changed, 346 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
index a6f3bec..fe49e9d 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
@@ -38,6 +38,24 @@ Required properties:
- #gpio-cells: Should be two. The first cell is the pin number. The
second cell is reserved for flags, unused at the moment.
+Optional properties:
+
+In order to use the gpio lines in PWM mode, some additional optional
+properties are required. Only Armada 370 and XP support these properties.
+
+- compatible: Must contain "marvell,armada-370-xp-gpio"
+
+- reg: an additional register set is needed, for the GPIO Blink
+ Counter on/off registers.
+
+- reg-names: Must contain an entry "pwm" corresponding to the
+ additional register range needed for pwm operation.
+
+- #pwm-cells: Should be two. The first cell is the GPIO line number. The
+ second cell is the period in nanoseconds.
+
+- clocks: Must be a phandle to the clock for the gpio controller.
+
Example:
gpio0: gpio@d0018100 {
@@ -51,3 +69,17 @@ Example:
#interrupt-cells = <2>;
interrupts = <16>, <17>, <18>, <19>;
};
+
+ gpio1: gpio@18140 {
+ compatible = "marvell,armada-370-xp-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
+ ngpios = <17>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #pwm-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <87>, <88>, <89>;
+ clocks = <&coreclk 0>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 58b3a22..19382f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10295,6 +10295,8 @@ F: include/linux/pwm.h
F: drivers/pwm/
F: drivers/video/backlight/pwm_bl.c
F: include/linux/pwm_backlight.h
+F: drivers/gpio/gpio-mvebu.c
+F: Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
PXA2xx/PXA3xx SUPPORT
M: Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org>
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index fae4db6..e310951 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -42,22 +42,34 @@
#include <linux/io.h>
#include <linux/of_irq.h>
#include <linux/of_device.h>
+#include <linux/pwm.h>
#include <linux/clk.h>
#include <linux/pinctrl/consumer.h>
#include <linux/irqchip/chained_irq.h>
+#include <linux/platform_device.h>
#include <linux/bitops.h>
+#include "gpiolib.h"
+
/*
* GPIO unit register offsets.
*/
-#define GPIO_OUT_OFF 0x0000
-#define GPIO_IO_CONF_OFF 0x0004
-#define GPIO_BLINK_EN_OFF 0x0008
-#define GPIO_IN_POL_OFF 0x000c
-#define GPIO_DATA_IN_OFF 0x0010
-#define GPIO_EDGE_CAUSE_OFF 0x0014
-#define GPIO_EDGE_MASK_OFF 0x0018
-#define GPIO_LEVEL_MASK_OFF 0x001c
+#define GPIO_OUT_OFF 0x0000
+#define GPIO_IO_CONF_OFF 0x0004
+#define GPIO_BLINK_EN_OFF 0x0008
+#define GPIO_IN_POL_OFF 0x000c
+#define GPIO_DATA_IN_OFF 0x0010
+#define GPIO_EDGE_CAUSE_OFF 0x0014
+#define GPIO_EDGE_MASK_OFF 0x0018
+#define GPIO_LEVEL_MASK_OFF 0x001c
+#define GPIO_BLINK_CNT_SELECT_OFF 0x0020
+
+/*
+ * PWM register offsets.
+ */
+#define PWM_BLINK_ON_DURATION_OFF 0x0
+#define PWM_BLINK_OFF_DURATION_OFF 0x4
+
/* The MV78200 has per-CPU registers for edge mask and level mask */
#define GPIO_EDGE_MASK_MV78200_OFF(cpu) ((cpu) ? 0x30 : 0x18)
@@ -78,6 +90,20 @@
#define MVEBU_MAX_GPIO_PER_BANK 32
+struct mvebu_pwm {
+ void __iomem *membase;
+ unsigned long clk_rate;
+ bool used;
+ struct pwm_chip chip;
+ spinlock_t lock;
+ struct mvebu_gpio_chip *mvchip;
+
+ /* Used to preserve GPIO/PWM registers across suspend/resume */
+ u32 blink_select;
+ u32 blink_on_duration;
+ u32 blink_off_duration;
+};
+
struct mvebu_gpio_chip {
struct gpio_chip chip;
spinlock_t lock;
@@ -87,6 +113,10 @@ struct mvebu_gpio_chip {
struct irq_domain *domain;
int soc_variant;
+ /* Used for PWM support */
+ struct clk *clk;
+ struct mvebu_pwm *mvpwm;
+
/* Used to preserve GPIO registers across suspend/resume */
u32 out_reg;
u32 io_conf_reg;
@@ -110,6 +140,12 @@ static void __iomem *mvebu_gpioreg_blink(struct mvebu_gpio_chip *mvchip)
return mvchip->membase + GPIO_BLINK_EN_OFF;
}
+static void __iomem *mvebu_gpioreg_blink_counter_select(struct mvebu_gpio_chip
+ *mvchip)
+{
+ return mvchip->membase + GPIO_BLINK_CNT_SELECT_OFF;
+}
+
static void __iomem *mvebu_gpioreg_io_conf(struct mvebu_gpio_chip *mvchip)
{
return mvchip->membase + GPIO_IO_CONF_OFF;
@@ -181,6 +217,20 @@ static void __iomem *mvebu_gpioreg_level_mask(struct mvebu_gpio_chip *mvchip)
}
/*
+ * Functions returning addresses of individual registers for a given
+ * PWM controller.
+ */
+static void __iomem *mvebu_pwmreg_blink_on_duration(struct mvebu_pwm *mvpwm)
+{
+ return mvpwm->membase + PWM_BLINK_ON_DURATION_OFF;
+}
+
+static void __iomem *mvebu_pwmreg_blink_off_duration(struct mvebu_pwm *mvpwm)
+{
+ return mvpwm->membase + PWM_BLINK_OFF_DURATION_OFF;
+}
+
+/*
* Functions implementing the gpio_chip methods
*/
static void mvebu_gpio_set(struct gpio_chip *chip, unsigned int pin, int value)
@@ -484,6 +534,243 @@ static void mvebu_gpio_irq_handler(struct irq_desc *desc)
chained_irq_exit(chip, desc);
}
+/*
+ * Functions implementing the pwm_chip methods
+ */
+static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip *chip)
+{
+ return container_of(chip, struct mvebu_pwm, chip);
+}
+
+static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ struct gpio_desc *desc = gpio_to_desc(pwm->pwm);
+ unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+ if (mvpwm->used) {
+ ret = -EBUSY;
+ } else {
+ if (!desc) {
+ ret = -ENODEV;
+ goto out;
+ }
+ ret = gpiod_request(desc, "mvebu-pwm");
+ if (ret)
+ goto out;
+
+ ret = gpiod_direction_output(desc, 0);
+ if (ret) {
+ gpiod_free(desc);
+ goto out;
+ }
+
+ mvpwm->used = true;
+ }
+
+out:
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+ return ret;
+}
+
+static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ struct gpio_desc *desc = gpio_to_desc(pwm->pwm);
+ unsigned long flags;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+ gpiod_free(desc);
+ mvpwm->used = false;
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+}
+
+static void mvebu_pwm_get_state(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ struct pwm_state *state) {
+
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
+ unsigned long long val;
+ unsigned long flags;
+ u32 u;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+
+ val = (unsigned long long)
+ readl_relaxed(mvebu_pwmreg_blink_on_duration);
+ val *= NSEC_PER_SEC;
+ do_div(val, mvpwm->clk_rate);
+ if (val > UINT_MAX)
+ state->duty_cycle = UINT_MAX;
+ else if (val)
+ state->duty_cycle = val;
+ else
+ state->duty_cycle = 1;
+
+ val = (unsigned long long)
+ readl_relaxed(mvebu_pwmreg_blink_off_duration);
+ val *= NSEC_PER_SEC;
+ do_div(val, mvpwm->clk_rate);
+ if (val < state->duty_cycle) {
+ state->period = 1;
+ } else {
+ val -= state->duty_cycle;
+ if (val > UINT_MAX)
+ state->period = UINT_MAX;
+ else if (val)
+ state->period = val;
+ else
+ state->period = 1;
+ }
+
+ u = readl_relaxed(mvebu_gpioreg_blink(mvchip));
+ if (u)
+ state->enabled = true;
+ else
+ state->enabled = false;
+
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+}
+
+static int mvebu_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
+ unsigned long long val;
+ unsigned long flags;
+ unsigned int on, off;
+
+ val = (unsigned long long) mvpwm->clk_rate * state->duty_cycle;
+ do_div(val, NSEC_PER_SEC);
+ if (val > UINT_MAX)
+ return -EINVAL;
+ if (val)
+ on = val;
+ else
+ on = 1;
+
+ val = (unsigned long long) mvpwm->clk_rate *
+ (state->period - state->duty_cycle);
+ do_div(val, NSEC_PER_SEC);
+ if (val > UINT_MAX)
+ return -EINVAL;
+ if (val)
+ off = val;
+ else
+ off = 1;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+
+ writel_relaxed(on, mvebu_pwmreg_blink_on_duration(mvpwm));
+ writel_relaxed(off, mvebu_pwmreg_blink_off_duration(mvpwm));
+ if (state->enabled)
+ mvebu_gpio_blink(&mvchip->chip, pwm->hwpwm, 1);
+ else
+ mvebu_gpio_blink(&mvchip->chip, pwm->hwpwm, 0);
+
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+
+ return 0;
+}
+
+static const struct pwm_ops mvebu_pwm_ops = {
+ .request = mvebu_pwm_request,
+ .free = mvebu_pwm_free,
+ .get_state = mvebu_pwm_get_state,
+ .apply = mvebu_pwm_apply,
+ .owner = THIS_MODULE,
+};
+
+static void __maybe_unused mvebu_pwm_suspend(struct mvebu_gpio_chip *mvchip)
+{
+ struct mvebu_pwm *mvpwm = mvchip->mvpwm;
+
+ mvpwm->blink_select =
+ readl_relaxed(mvebu_gpioreg_blink_counter_select(mvchip));
+ mvpwm->blink_on_duration =
+ readl_relaxed(mvebu_pwmreg_blink_on_duration(mvpwm));
+ mvpwm->blink_off_duration =
+ readl_relaxed(mvebu_pwmreg_blink_off_duration(mvpwm));
+}
+
+static void __maybe_unused mvebu_pwm_resume(struct mvebu_gpio_chip *mvchip)
+{
+ struct mvebu_pwm *mvpwm = mvchip->mvpwm;
+
+ writel_relaxed(mvpwm->blink_select,
+ mvebu_gpioreg_blink_counter_select(mvchip));
+ writel_relaxed(mvpwm->blink_on_duration,
+ mvebu_pwmreg_blink_on_duration(mvpwm));
+ writel_relaxed(mvpwm->blink_off_duration,
+ mvebu_pwmreg_blink_off_duration(mvpwm));
+}
+
+static int mvebu_pwm_probe(struct platform_device *pdev,
+ struct mvebu_gpio_chip *mvchip,
+ int id)
+{
+ struct device *dev = &pdev->dev;
+ struct mvebu_pwm *mvpwm;
+ struct resource *res;
+
+ if (!of_device_is_compatible(mvchip->chip.of_node,
+ "marvell,armada-370-xp-gpio"))
+ return 0;
+ /*
+ * There are only two sets of PWM configuration registers for
+ * all the GPIO lines on those SoCs which this driver reserves
+ * for the first two GPIO chips. So if the resource is missing
+ * we can't treat it as an error.
+ */
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm");
+ if (!res)
+ return 0;
+
+ /*
+ * Use set A for lines of GPIO chip with id 0, B for GPIO chip
+ * with id 1. Don't allow further GPIO chips to be used for PWM.
+ */
+ if (id == 0)
+ writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
+ else if (id == 1)
+ writel_relaxed(U32_MAX,
+ mvebu_gpioreg_blink_counter_select(mvchip));
+ else
+ return -EINVAL;
+
+ mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
+ if (!mvpwm)
+ return -ENOMEM;
+ mvchip->mvpwm = mvpwm;
+ mvpwm->mvchip = mvchip;
+
+ mvpwm->membase = devm_ioremap_resource(dev, res);
+ if (IS_ERR(mvpwm->membase))
+ return PTR_ERR(mvpwm->membase);
+
+ if (IS_ERR(mvchip->clk))
+ return PTR_ERR(mvchip->clk);
+
+ mvpwm->clk_rate = clk_get_rate(mvchip->clk);
+ if (!mvpwm->clk_rate) {
+ dev_err(dev, "failed to get clock rate\n");
+ return -EINVAL;
+ }
+
+ mvpwm->chip.dev = dev;
+ mvpwm->chip.ops = &mvebu_pwm_ops;
+ mvpwm->chip.base = mvchip->chip.base;
+ mvpwm->chip.npwm = mvchip->chip.ngpio;
+
+ spin_lock_init(&mvpwm->lock);
+
+ return pwmchip_add(&mvpwm->chip);
+}
+
#ifdef CONFIG_DEBUG_FS
#include <linux/seq_file.h>
@@ -555,6 +842,10 @@ static const struct of_device_id mvebu_gpio_of_match[] = {
.data = (void *) MVEBU_GPIO_SOC_VARIANT_ARMADAXP,
},
{
+ .compatible = "marvell,armada-370-xp-gpio",
+ .data = (void *) MVEBU_GPIO_SOC_VARIANT_ORION,
+ },
+ {
/* sentinel */
},
};
@@ -600,6 +891,9 @@ static int mvebu_gpio_suspend(struct platform_device *pdev, pm_message_t state)
BUG();
}
+ if (IS_ENABLED(CONFIG_PWM))
+ mvebu_pwm_suspend(mvchip);
+
return 0;
}
@@ -643,6 +937,9 @@ static int mvebu_gpio_resume(struct platform_device *pdev)
BUG();
}
+ if (IS_ENABLED(CONFIG_PWM))
+ mvebu_pwm_resume(mvchip);
+
return 0;
}
@@ -654,7 +951,6 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
struct resource *res;
struct irq_chip_generic *gc;
struct irq_chip_type *ct;
- struct clk *clk;
unsigned int ngpios;
bool have_irqs;
int soc_variant;
@@ -688,10 +984,10 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
return id;
}
- clk = devm_clk_get(&pdev->dev, NULL);
+ mvchip->clk = devm_clk_get(&pdev->dev, NULL);
/* Not all SoCs require a clock.*/
- if (!IS_ERR(clk))
- clk_prepare_enable(clk);
+ if (!IS_ERR(mvchip->clk))
+ clk_prepare_enable(mvchip->clk);
mvchip->soc_variant = soc_variant;
mvchip->chip.label = dev_name(&pdev->dev);
@@ -822,6 +1118,10 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
mvchip);
}
+ /* Armada 370/XP has simple PWM support for GPIO lines */
+ if (IS_ENABLED(CONFIG_PWM))
+ return mvebu_pwm_probe(pdev, mvchip, id);
+
return 0;
err_domain:
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v5 2/4] ARM: dts: mvebu: Add PWM properties to .dtsi files
From: Ralph Sennhauser @ 2017-04-09 18:09 UTC (permalink / raw)
To: Thierry Reding
Cc: Mark Rutland, Andrew Lunn, Jason Cooper, Alexandre Courbot,
Linus Walleij, Russell King, linux-pwm, linux-kernel,
Gregory Clement, devicetree, Rob Herring, linux-gpio,
Ralph Sennhauser, linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20170409180931.4884-1-ralph.sennhauser@gmail.com>
From: Andrew Lunn <andrew@lunn.ch>
Add properties to the GPIO nodes to allow them to be also used as PWM
lines.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
URL: https://patchwork.ozlabs.org/patch/427294/
[Ralph Sennhauser: Add new compatible string marvell,armada-370-xp-gpio]
Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
---
arch/arm/boot/dts/armada-370.dtsi | 19 ++++++++++++++-----
arch/arm/boot/dts/armada-xp-mv78230.dtsi | 16 ++++++++++++----
arch/arm/boot/dts/armada-xp-mv78260.dtsi | 19 ++++++++++++++-----
arch/arm/boot/dts/armada-xp-mv78460.dtsi | 19 ++++++++++++++-----
4 files changed, 54 insertions(+), 19 deletions(-)
diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index cc011c8..5e815cc 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -137,29 +137,38 @@
};
gpio0: gpio@18100 {
- compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};
gpio1: gpio@18140 {
- compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>, <90>;
+ clocks = <&coreclk 0>;
};
gpio2: gpio@18180 {
- compatible = "marvell,orion-gpio";
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
reg = <0x18180 0x40>;
ngpios = <2>;
gpio-controller;
diff --git a/arch/arm/boot/dts/armada-xp-mv78230.dtsi b/arch/arm/boot/dts/armada-xp-mv78230.dtsi
index 07c5090..f77168c9 100644
--- a/arch/arm/boot/dts/armada-xp-mv78230.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78230.dtsi
@@ -202,25 +202,33 @@
internal-regs {
gpio0: gpio@18100 {
- compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};
gpio1: gpio@18140 {
- compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <17>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>;
+ clocks = <&coreclk 0>;
};
};
};
diff --git a/arch/arm/boot/dts/armada-xp-mv78260.dtsi b/arch/arm/boot/dts/armada-xp-mv78260.dtsi
index 64e936a..0ecfaf4 100644
--- a/arch/arm/boot/dts/armada-xp-mv78260.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78260.dtsi
@@ -285,29 +285,38 @@
internal-regs {
gpio0: gpio@18100 {
- compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};
gpio1: gpio@18140 {
- compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>, <90>;
+ clocks = <&coreclk 0>;
};
gpio2: gpio@18180 {
- compatible = "marvell,orion-gpio";
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
reg = <0x18180 0x40>;
ngpios = <3>;
gpio-controller;
diff --git a/arch/arm/boot/dts/armada-xp-mv78460.dtsi b/arch/arm/boot/dts/armada-xp-mv78460.dtsi
index d1383dd..670ece4c 100644
--- a/arch/arm/boot/dts/armada-xp-mv78460.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78460.dtsi
@@ -323,29 +323,38 @@
internal-regs {
gpio0: gpio@18100 {
- compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};
gpio1: gpio@18140 {
- compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>, <90>;
+ clocks = <&coreclk 0>;
};
gpio2: gpio@18180 {
- compatible = "marvell,orion-gpio";
+ compatible = "marvell,armada-370-xp-gpio",
+ "marvell,orion-gpio";
reg = <0x18180 0x40>;
ngpios = <3>;
gpio-controller;
--
2.10.2
^ permalink raw reply related
* [PATCH v5 3/4] ARM: mvebu: Enable SENSORS_PWM_FAN in defconfig
From: Ralph Sennhauser @ 2017-04-09 18:09 UTC (permalink / raw)
To: Thierry Reding
Cc: Andrew Lunn, Ralph Sennhauser, Linus Walleij, Alexandre Courbot,
Rob Herring, Mark Rutland, Jason Cooper, Gregory Clement,
Sebastian Hesselbarth, Russell King, linux-pwm, linux-gpio,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20170409180931.4884-1-ralph.sennhauser@gmail.com>
From: Andrew Lunn <andrew@lunn.ch>
Now that the GPIO driver also supports PWM operation, enable the PWM
framework and fan driver in mvebu_v7_defconfig.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
URL: https://patchwork.ozlabs.org/patch/427297/
[Ralph Sennhauser: add fan driver to defconfig]
Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
---
arch/arm/configs/mvebu_v7_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/configs/mvebu_v7_defconfig b/arch/arm/configs/mvebu_v7_defconfig
index f1a0e25..6955370 100644
--- a/arch/arm/configs/mvebu_v7_defconfig
+++ b/arch/arm/configs/mvebu_v7_defconfig
@@ -135,6 +135,8 @@ CONFIG_DMADEVICES=y
CONFIG_MV_XOR=y
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_MEMORY=y
+CONFIG_PWM=y
+CONFIG_SENSORS_PWM_FAN=y
CONFIG_EXT4_FS=y
CONFIG_ISO9660_FS=y
CONFIG_JOLIET=y
--
2.10.2
^ permalink raw reply related
* [PATCH v5 4/4] ARM: dts: armada-xp: Use pwm-fan rather than gpio-fan
From: Ralph Sennhauser @ 2017-04-09 18:09 UTC (permalink / raw)
To: Thierry Reding
Cc: Andrew Lunn, Ralph Sennhauser, Linus Walleij, Alexandre Courbot,
Rob Herring, Mark Rutland, Jason Cooper, Gregory Clement,
Sebastian Hesselbarth, Russell King, linux-pwm, linux-gpio,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20170409180931.4884-1-ralph.sennhauser@gmail.com>
From: Andrew Lunn <andrew@lunn.ch>
The mvebu GPIO driver can also perform PWM on some pins. Use the pwm-fan
driver to control the fan of the WRT1900AC, giving us finer grained control
over its speed and hence noise.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
URL: https://patchwork.ozlabs.org/patch/427291/
[Ralph Sennhauser: drop flags paramter from pwms, no longer used]
Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
---
arch/arm/boot/dts/armada-xp-linksys-mamba.dts | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-linksys-mamba.dts b/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
index 9efcf59..6d705f5 100644
--- a/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
+++ b/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
@@ -308,13 +308,11 @@
};
};
- gpio_fan {
+ pwm_fan {
/* SUNON HA4010V4-0000-C99 */
- compatible = "gpio-fan";
- gpios = <&gpio0 24 0>;
- gpio-fan,speed-map = <0 0
- 4500 1>;
+ compatible = "pwm-fan";
+ pwms = <&gpio0 24 4000>;
};
dsa {
--
2.10.2
^ permalink raw reply related
* Re: [PATCH v5 19/23] drivers/fsi: Add GPIO based FSI master
From: Christopher Bostic @ 2017-04-09 21:04 UTC (permalink / raw)
To: Randy Dunlap, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: joel-U3u1mxZcP9KHXe+LvDLADg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
andrew-zrmu5oMJ5Fs, alistair-Y4h6yKqj69EXC2x5gXVKYQ,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r, Edward A . James,
Jeremy Kerr
In-Reply-To: <5d173f9c-e01c-6093-16ab-d114857009b2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
On 4/5/17 11:35 AM, Randy Dunlap wrote:
> On 04/04/17 19:06, Christopher Bostic wrote:
>> From: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>
>> Implement a FSI master using GPIO. Will generate FSI protocol for
>> read and write commands to particular addresses. Sends master command
>> and waits for and decodes a slave response.
>>
>> Includes changes from Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> and Jeremy
>> Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>.
>>
>> Signed-off-by: Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
>> ---
>> drivers/fsi/Kconfig | 11 +
>> drivers/fsi/Makefile | 1 +
>> drivers/fsi/fsi-master-gpio.c | 610 ++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 622 insertions(+)
>> create mode 100644 drivers/fsi/fsi-master-gpio.c
>>
>> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
>> index 04c1a0e..9cf8345 100644
>> --- a/drivers/fsi/Kconfig
>> +++ b/drivers/fsi/Kconfig
>> @@ -9,4 +9,15 @@ config FSI
>> ---help---
>> FSI - the FRU Support Interface - is a simple bus for low-level
>> access to POWER-based hardware.
>> +
>> +if FSI
>> +
>> +config FSI_MASTER_GPIO
>> + tristate "GPIO-based FSI master"
>> + depends on FSI && GPIOLIB
> depends on FSI is redundant since "if FSI" does the same thing.
Hi Randy,
Thanks for the feedback, will correct.
-Chris
>> + ---help---
>> + This option enables a FSI master driver using GPIO lines.
>> +
>> +endif
>> +
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 21/23] drivers/fsi: Add SCOM FSI client device driver
From: Christopher Bostic @ 2017-04-09 21:06 UTC (permalink / raw)
To: Randy Dunlap, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: joel-U3u1mxZcP9KHXe+LvDLADg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
andrew-zrmu5oMJ5Fs, alistair-Y4h6yKqj69EXC2x5gXVKYQ,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r, Edward A . James,
Jeremy Kerr
In-Reply-To: <bddbb80e-10b6-fbb9-152a-6e192dfe0745-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
On 4/5/17 11:35 AM, Randy Dunlap wrote:
> On 04/04/17 19:06, Christopher Bostic wrote:
>> From: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>
>> Create a simple SCOM engine device driver that reads and writes
>> its control registers via an FSI bus.
>>
>> Includes changes from Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>.
>>
>> Signed-off-by: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
>> Signed-off-by: Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>> ---
>> drivers/fsi/Kconfig | 6 ++
>> drivers/fsi/Makefile | 1 +
>> drivers/fsi/fsi-scom.c | 263 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 270 insertions(+)
>> create mode 100644 drivers/fsi/fsi-scom.c
>>
>> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
>> index 9cf8345..0fa265c 100644
>> --- a/drivers/fsi/Kconfig
>> +++ b/drivers/fsi/Kconfig
>> @@ -18,6 +18,12 @@ config FSI_MASTER_GPIO
>> ---help---
>> This option enables a FSI master driver using GPIO lines.
>>
>> +config FSI_SCOM
>> + tristate "SCOM FSI client device driver"
>> + depends on FSI
> depends on FSI is redundant.
Will correct.
Thanks,
Chris
>
>> + ---help---
>> + This option enables an FSI based SCOM device driver.
>> +
>> endif
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 22/23] drivers/fsi: Add hub master support
From: Christopher Bostic @ 2017-04-09 21:07 UTC (permalink / raw)
To: Randy Dunlap, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: joel-U3u1mxZcP9KHXe+LvDLADg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
andrew-zrmu5oMJ5Fs, alistair-Y4h6yKqj69EXC2x5gXVKYQ,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r, Jeremy Kerr
In-Reply-To: <8c841062-62e9-77c6-b0c9-e0da73cad3ac-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
On 4/5/17 11:36 AM, Randy Dunlap wrote:
> On 04/04/17 19:06, Christopher Bostic wrote:
>> From: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>
>> Add an engine driver to expose a "hub" FSI master - which has a set of
>> control registers in the engine address space, and uses a chunk of the
>> slave address space for actual FSI communication.
>>
>> Additional changes from Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>.
>>
>> Signed-off-by: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
>> ---
>> drivers/fsi/Kconfig | 9 ++
>> drivers/fsi/Makefile | 1 +
>> drivers/fsi/fsi-master-hub.c | 327 +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 337 insertions(+)
>> create mode 100644 drivers/fsi/fsi-master-hub.c
>>
>> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
>> index 0fa265c..e1156b4 100644
>> --- a/drivers/fsi/Kconfig
>> +++ b/drivers/fsi/Kconfig
>> @@ -18,6 +18,15 @@ config FSI_MASTER_GPIO
>> ---help---
>> This option enables a FSI master driver using GPIO lines.
>>
>> +config FSI_MASTER_HUB
>> + tristate "FSI hub master"
>> + depends on FSI
> redundant again.
Will correct.
Thanks,
Chris
>
>> + ---help---
>> + This option enables a FSI hub master driver. Hub is a type of FSI
>> + master that is connected to the upstream master via a slave. Hubs
>> + allow chaining of FSI links to an arbitrary depth. This allows for
>> + a high target device fanout.
>> +
>> config FSI_SCOM
>> tristate "SCOM FSI client device driver"
>> depends on FSI
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 19/23] drivers/fsi: Add GPIO based FSI master
From: Christopher Bostic @ 2017-04-09 21:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Joel Stanley
Cc: Rob Herring, Mark Rutland, Russell King,
rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA,
Greg KH, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Linux Kernel Mailing List, Andrew Jeffery, Alistair Popple,
Edward A . James, Jeremy Kerr
In-Reply-To: <1491344360.4166.68.camel-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
On 4/4/17 5:19 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-04 at 12:32 -0500, Christopher Bostic wrote:
>> Agreed that there is room for improvement. I intend to look further
>> into your suggestions from here and our private conversation on the
>> matter and make changes as appropriate. I have an open issue to track
>> this. As it exists in this patch reads/writes from master to slave
>> fundamentally work.
> My understanding is they "seem to work if you get lucky with the timing
> and fall apart under load". Or did I hear wrong ?
>
>> Given the pervasiveness and time to fully evaluate
>> and test any protocol updates I intend address this in the near future
>> with a separate follow on patch.
> Please try the simple change I proposed in my email. It's a 4 or 5
> lines change max to your clock_toggle function and how it's called in
> send and receive. It should be trivial to check if things still "seem
> to work" to begin with.
Hi Benjamin,
I did try reordering the clock delays from: delay, clock 0, delay clock
1 to: clock 0, delay, clock 1, delay.
This worked fine. Making this change also removes the need for having a
third delay I had in place prior to sampling
SDA when in slave response mode.
A 3 microsecond delay is required, however, to prevent occasional issues
during heavy FSI bus load stress testing.
A 1 nanosecond delay using ndelay(1) had been specified prior to this
but after looking more closely at real time performance it turned out to
actually be roughly 1-2 microseconds. This appears to be the minimum
resolution using the delay() linux libraries on the AST2400/2500.
Given this, increasing delay to 3 microseconds doesn't impact
performance much considering I can now remove the sample input delay
based on your recommendations to re-order the two clock delays.
Thanks for your input.
Chris
>
> Do you have some kind of test mechanism that hammers the FSI
> continuously ? Such as doing a series of putmemproc/getmemproc &
> checking the values ?
>
> Then you can run that while hammering the LPC bus and generally putting
> the BMC under load and you'll quickly see if it's reliable or not.
>
> Cheers,
> Ben.
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 19/23] drivers/fsi: Add GPIO based FSI master
From: Benjamin Herrenschmidt @ 2017-04-09 21:41 UTC (permalink / raw)
To: Christopher Bostic, Joel Stanley
Cc: Mark Rutland, devicetree, Andrew Jeffery, Greg KH, Russell King,
rostedt, Linux Kernel Mailing List, Rob Herring, Jeremy Kerr,
Edward A . James, Alistair Popple, mingo, linux-arm-kernel
In-Reply-To: <5344146a-f450-7959-4688-a83ee022574b@linux.vnet.ibm.com>
On Sun, 2017-04-09 at 16:22 -0500, Christopher Bostic wrote:
> A 3 microsecond delay is required, however, to prevent occasional issues
> during heavy FSI bus load stress testing.
> A 1 nanosecond delay using ndelay(1) had been specified prior to this
> but after looking more closely at real time performance it turned out to
> actually be roughly 1-2 microseconds. This appears to be the minimum
> resolution using the delay() linux libraries on the AST2400/2500.
> Given this, increasing delay to 3 microseconds doesn't impact
> performance much considering I can now remove the sample input delay
> based on your recommendations to re-order the two clock delays.
This is huge delays. We should consider a AST2xxx specific variant of
the backend that uses nops or similar lab-calibrated constructs
instead. Otherwise we are stuck in the kHz range, this is a >200Mhz bus
:)
I don't understand why 3us delay would thus be necessary.
Where about did you observe issues ? Could it be that you don't wait
long enough in the transitions from write to read ?
Cheers,
Ben.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 19/23] drivers/fsi: Add GPIO based FSI master
From: Benjamin Herrenschmidt @ 2017-04-09 21:53 UTC (permalink / raw)
To: Christopher Bostic, Joel Stanley
Cc: Rob Herring, Mark Rutland, Russell King,
rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA,
Greg KH, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Linux Kernel Mailing List, Andrew Jeffery, Alistair Popple,
Edward A . James, Jeremy Kerr
In-Reply-To: <1491774092.4166.195.camel-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
On Mon, 2017-04-10 at 07:41 +1000, Benjamin Herrenschmidt wrote:
> On Sun, 2017-04-09 at 16:22 -0500, Christopher Bostic wrote:
> > A 3 microsecond delay is required, however, to prevent occasional
> > issues
> > during heavy FSI bus load stress testing.
> > A 1 nanosecond delay using ndelay(1) had been specified prior to
> > this
> > but after looking more closely at real time performance it turned
> > out to
> > actually be roughly 1-2 microseconds. This appears to be the
> > minimum
> > resolution using the delay() linux libraries on the
> > AST2400/2500.
> > Given this, increasing delay to 3 microseconds doesn't impact
> > performance much considering I can now remove the sample input
> > delay
> > based on your recommendations to re-order the two clock delays.
>
> This is huge delays. We should consider a AST2xxx specific variant of
> the backend that uses nops or similar lab-calibrated constructs
> instead. Otherwise we are stuck in the kHz range, this is a >200Mhz
> bus
> :)
>
> I don't understand why 3us delay would thus be necessary.
>
> Where about did you observe issues ? Could it be that you don't wait
> long enough in the transitions from write to read ?
FYI. pdbg in userspace operates without any delays in practice, the
overhead between the various load/store instructions seems sufficient.
The only delay that's needed is when going through the FSI2PIB (to do
SCOMs) where it seems like back-2-back accesses can be problematic.
Cheers,
Ben
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 19/23] drivers/fsi: Add GPIO based FSI master
From: Benjamin Herrenschmidt @ 2017-04-09 21:55 UTC (permalink / raw)
To: Christopher Bostic, Joel Stanley
Cc: Rob Herring, Mark Rutland, Russell King,
rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA,
Greg KH, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Linux Kernel Mailing List, Andrew Jeffery, Alistair Popple,
Edward A . James, Jeremy Kerr
In-Reply-To: <1491774835.4166.197.camel-8fk3Idey6ehBDgjK7y7TUQ@public.gmane.org>
On Mon, 2017-04-10 at 07:53 +1000, Benjamin Herrenschmidt wrote:
> FYI. pdbg in userspace operates without any delays in practice, the
> overhead between the various load/store instructions seems
> sufficient.
>
> The only delay that's needed is when going through the FSI2PIB (to do
> SCOMs) where it seems like back-2-back accesses can be problematic.
Note: This isn't an objection to merging the patches. We can look at
doing an improved backend later.
Cheers,
Ben.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] mfd: cros-ec: Add functions to read mapped memory
From: Guenter Roeck @ 2017-04-09 23:02 UTC (permalink / raw)
To: Moritz Fischer, linux-hwmon
Cc: linux-kernel, devicetree, lee.jones, olof, jdelvare, robh+dt,
mark.rutland, Moritz Fischer, Benson Leung
In-Reply-To: <1491602410-31518-1-git-send-email-moritz.fischer@ettus.com>
On 04/07/2017 03:00 PM, Moritz Fischer wrote:
> From: Moritz Fischer <mdf@kernel.org>
>
> The ChromeOS EC has mapped memory regions where things like temperature
> sensors and fan speed are stored. Provide access to those from the
> cros-ec mfd device.
>
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
I'll have to consult with others at Google if this is a good idea.
Benson, can you comment ?
> ---
> drivers/platform/chrome/cros_ec_proto.c | 55 +++++++++++++++++++++++++++++++++
> include/linux/mfd/cros_ec.h | 39 +++++++++++++++++++++++
> 2 files changed, 94 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index ed5dee7..28063de 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -494,3 +494,58 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
> return get_keyboard_state_event(ec_dev);
> }
> EXPORT_SYMBOL(cros_ec_get_next_event);
> +
> +static int __cros_ec_read_mapped_mem(struct cros_ec_device *ec, uint8_t offset,
> + void *buf, size_t size)
> +{
> + int ret;
> + struct ec_params_read_memmap *params;
> + struct cros_ec_command *msg;
> +
> + msg = kzalloc(sizeof(*msg) + max(sizeof(*params), size), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
I don't think using kzalloc here makes much sense. It is well known
that size is <= 4, so using a local buffer should not be a problem.
> + msg->version = 0;
> + msg->command = EC_CMD_READ_MEMMAP;
> + msg->insize = size;
> + msg->outsize = sizeof(*params);
> +
> + params = (struct ec_params_read_memmap *)msg->data;
> + params->offset = offset;
> + params->size = size;
> +
> + ret = cros_ec_cmd_xfer(ec, msg);
> + if (ret < 0 || msg->result != EC_RES_SUCCESS) {
cros_ec_cmd_xfer_status() was introduced to be able to avoid the second check.
> + dev_warn(ec->dev, "cannot read mapped reg: %d/%d\n",
> + ret, msg->result);
> + goto out_free;
> + }
> +
> + memcpy(buf, msg->data, size);
> +
> +out_free:
> + kfree(msg);
> + return ret;
> +}
> +
> +int cros_ec_read_mapped_mem32(struct cros_ec_device *ec, const uint8_t offset,
> + uint32_t *data)
> +{
> + return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> +}
> +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem32);
> +
> +int cros_ec_read_mapped_mem16(struct cros_ec_device *ec, const uint8_t offset,
> + uint16_t *data)
> +{
> + return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> +}
> +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem16);
> +
Either case, this assumes that EC endianness matches host endianness. I don't
think we can just assume that this is the case.
> +int cros_ec_read_mapped_mem8(struct cros_ec_device *ec, const uint8_t offset,
> + uint8_t *data)
> +{
> + return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> +}
> +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem8);
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index b3d04de..c2de878 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -190,6 +190,45 @@ struct cros_ec_dev {
> };
>
> /**
> + * cros_ec_read_mapped_mem8 - Read mapped memory in the ChromeOS EC
> + *
> + * This can be called by drivers to access the mapped memory in the EC
> + *
> + * @ec_dev: Device to read from
> + * @offset: Offset to read
> + * @data: Return data
> + * @return: 0 if Ok, -ve on error
> + */
> +int cros_ec_read_mapped_mem8(struct cros_ec_device *ec, const uint8_t offset,
> + uint8_t *data);
> +
> +/**
> + * cros_ec_read_mapped_mem16 - Read mapped memory in the ChromeOS EC
> + *
> + * This can be called by drivers to access the mapped memory in the EC
> + *
> + * @ec_dev: Device to read from
> + * @offset: Offset to read
> + * @data: Return data
> + * @return: 0 if Ok, -ve on error
> + */
> +int cros_ec_read_mapped_mem16(struct cros_ec_device *ec, const uint8_t offset,
> + uint16_t *data);
> +
> +/**
> + * cros_ec_read_mapped_mem32 - Read mapped memory in the ChromeOS EC
> + *
> + * This can be called by drivers to access the mapped memory in the EC
> + *
> + * @ec_dev: Device to read from
> + * @offset: Offset to read
> + * @data: Return data
> + * @return: 0 if Ok, -ve on error
> + */
> +int cros_ec_read_mapped_mem32(struct cros_ec_device *ec, const uint8_t offset,
> + uint32_t *data);
> +
> +/**
> * cros_ec_suspend - Handle a suspend operation for the ChromeOS EC device
> *
> * This can be called by drivers to handle a suspend event.
>
^ permalink raw reply
* Re: [PATCH v4 6/9] ASoC: add snd_soc_get_dai_id()
From: Kuninori Morimoto @ 2017-04-10 0:17 UTC (permalink / raw)
To: Rob Herring; +Cc: Linux-DT, Linux-ALSA, Mark Brown, Simon
In-Reply-To: <CAL_JsqJrcG+zczxdjjBXU17EDGANR9Exet5h5R08dStXiV9zzg@mail.gmail.com>
Hi Rob
Thank you for your review.
> >> > +{
> >> > + struct device_node *node;
> >> > + struct device_node *endpoint;
> >> > + int i, id;
> >> > +
> >> > + node = of_graph_get_port_parent(ep);
> >> > +
> >> > + i = 0;
> >> > + id = -1;
> >> > + for_each_endpoint_of_node(node, endpoint) {
> >> > + if (endpoint == ep)
> >> > + id = i;
> >>
> >> I don't see how this works when you have 1 DAI controller with
> >> multiple endpoints versus multiple DAI controllers with a single
> >> endpoint each. All the IDs will be 0 in the latter case.
> >
> > It support 1:1 endpoint pattern only.
>
> Then the endpoint id is always 0 and this function is pointless.
Sorry, I checked my patch-list, and I noticed that
this function will be expand to use callback function in next
patch-set (= HDMI support).
Thus, inded current function is pointless at this point.
I will merge this expansion patch in v5
^ permalink raw reply
* Re: [PATCH 1/3] mfd: cros-ec: Add functions to read mapped memory
From: Moritz Fischer @ 2017-04-10 0:40 UTC (permalink / raw)
To: Guenter Roeck
Cc: Moritz Fischer, linux-hwmon, Linux Kernel Mailing List,
Devicetree List, lee.jones, olof, jdelvare, Rob Herring,
Mark Rutland, Moritz Fischer, Benson Leung
In-Reply-To: <4917fd01-8dd9-eb58-ee29-3ff4f11de0af@roeck-us.net>
On Sun, Apr 09, 2017 at 04:02:04PM -0700, Guenter Roeck wrote:
> On 04/07/2017 03:00 PM, Moritz Fischer wrote:
> > From: Moritz Fischer <mdf@kernel.org>
> >
> > The ChromeOS EC has mapped memory regions where things like temperature
> > sensors and fan speed are stored. Provide access to those from the
> > cros-ec mfd device.
> >
> > Signed-off-by: Moritz Fischer <mdf@kernel.org>
>
> I'll have to consult with others at Google if this is a good idea.
> Benson, can you comment ?
Well to my knowledge the only other way to get to it is the 'ectool'
from userland
via ioctl calls. The other option would be IIO ...
>
> > ---
> > drivers/platform/chrome/cros_ec_proto.c | 55 +++++++++++++++++++++++++++++++++
> > include/linux/mfd/cros_ec.h | 39 +++++++++++++++++++++++
> > 2 files changed, 94 insertions(+)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> > index ed5dee7..28063de 100644
> > --- a/drivers/platform/chrome/cros_ec_proto.c
> > +++ b/drivers/platform/chrome/cros_ec_proto.c
> > @@ -494,3 +494,58 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
> > return get_keyboard_state_event(ec_dev);
> > }
> > EXPORT_SYMBOL(cros_ec_get_next_event);
> > +
> > +static int __cros_ec_read_mapped_mem(struct cros_ec_device *ec, uint8_t offset,
> > + void *buf, size_t size)
> > +{
> > + int ret;
> > + struct ec_params_read_memmap *params;
> > + struct cros_ec_command *msg;
> > +
> > + msg = kzalloc(sizeof(*msg) + max(sizeof(*params), size), GFP_KERNEL);
> > + if (!msg)
> > + return -ENOMEM;
> > +
>
> I don't think using kzalloc here makes much sense. It is well known
> that size is <= 4, so using a local buffer should not be a problem.
Good point, that was basically copy & paste from other cros-ec code ;-)
I'll fix this.
>
> > + msg->version = 0;
> > + msg->command = EC_CMD_READ_MEMMAP;
> > + msg->insize = size;
> > + msg->outsize = sizeof(*params);
> > +
> > + params = (struct ec_params_read_memmap *)msg->data;
> > + params->offset = offset;
> > + params->size = size;
> > +
> > + ret = cros_ec_cmd_xfer(ec, msg);
> > + if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>
> cros_ec_cmd_xfer_status() was introduced to be able to avoid the second check.
>
Alright, cool. Will fix this.
> > + dev_warn(ec->dev, "cannot read mapped reg: %d/%d\n",
> > + ret, msg->result);
> > + goto out_free;
> > + }
> > +
> > + memcpy(buf, msg->data, size);
> > +
> > +out_free:
> > + kfree(msg);
> > + return ret;
> > +}
> > +
> > +int cros_ec_read_mapped_mem32(struct cros_ec_device *ec, const uint8_t offset,
> > + uint32_t *data)
> > +{
> > + return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> > +}
> > +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem32);
> > +
> > +int cros_ec_read_mapped_mem16(struct cros_ec_device *ec, const uint8_t offset,
> > + uint16_t *data)
> > +{
> > + return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> > +}
> > +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem16);
> > +
>
> Either case, this assumes that EC endianness matches host endianness. I don't
> think we can just assume that this is the case.
Huh, yeah. Will need to figure out how to detect the EC endianness in
that case.
>
> > +int cros_ec_read_mapped_mem8(struct cros_ec_device *ec, const uint8_t offset,
> > + uint8_t *data)
> > +{
> > + return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> > +}
> > +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem8);
> > diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> > index b3d04de..c2de878 100644
> > --- a/include/linux/mfd/cros_ec.h
> > +++ b/include/linux/mfd/cros_ec.h
> > @@ -190,6 +190,45 @@ struct cros_ec_dev {
> > };
> >
> > /**
> > + * cros_ec_read_mapped_mem8 - Read mapped memory in the ChromeOS EC
> > + *
> > + * This can be called by drivers to access the mapped memory in the EC
> > + *
> > + * @ec_dev: Device to read from
> > + * @offset: Offset to read
> > + * @data: Return data
> > + * @return: 0 if Ok, -ve on error
> > + */
> > +int cros_ec_read_mapped_mem8(struct cros_ec_device *ec, const uint8_t offset,
> > + uint8_t *data);
> > +
> > +/**
> > + * cros_ec_read_mapped_mem16 - Read mapped memory in the ChromeOS EC
> > + *
> > + * This can be called by drivers to access the mapped memory in the EC
> > + *
> > + * @ec_dev: Device to read from
> > + * @offset: Offset to read
> > + * @data: Return data
> > + * @return: 0 if Ok, -ve on error
> > + */
> > +int cros_ec_read_mapped_mem16(struct cros_ec_device *ec, const uint8_t offset,
> > + uint16_t *data);
> > +
> > +/**
> > + * cros_ec_read_mapped_mem32 - Read mapped memory in the ChromeOS EC
> > + *
> > + * This can be called by drivers to access the mapped memory in the EC
> > + *
> > + * @ec_dev: Device to read from
> > + * @offset: Offset to read
> > + * @data: Return data
> > + * @return: 0 if Ok, -ve on error
> > + */
> > +int cros_ec_read_mapped_mem32(struct cros_ec_device *ec, const uint8_t offset,
> > + uint32_t *data);
> > +
> > +/**
> > * cros_ec_suspend - Handle a suspend operation for the ChromeOS EC device
> > *
> > * This can be called by drivers to handle a suspend event.
> >
>
Thanks for the feedback,
Moritz
^ permalink raw reply
* Re: [PATCH v4 6/9] ASoC: add snd_soc_get_dai_id()
From: Kuninori Morimoto @ 2017-04-10 0:45 UTC (permalink / raw)
To: Rob Herring; +Cc: Linux-DT, Linux-ALSA, Mark Brown, Simon
In-Reply-To: <874lxx5dcd.wl%kuninori.morimoto.gx@renesas.com>
Hi Rob, again
> > >> > +{
> > >> > + struct device_node *node;
> > >> > + struct device_node *endpoint;
> > >> > + int i, id;
> > >> > +
> > >> > + node = of_graph_get_port_parent(ep);
> > >> > +
> > >> > + i = 0;
> > >> > + id = -1;
> > >> > + for_each_endpoint_of_node(node, endpoint) {
> > >> > + if (endpoint == ep)
> > >> > + id = i;
> > >>
> > >> I don't see how this works when you have 1 DAI controller with
> > >> multiple endpoints versus multiple DAI controllers with a single
> > >> endpoint each. All the IDs will be 0 in the latter case.
> > >
> > > It support 1:1 endpoint pattern only.
> >
> > Then the endpoint id is always 0 and this function is pointless.
>
> Sorry, I checked my patch-list, and I noticed that
> this function will be expand to use callback function in next
> patch-set (= HDMI support).
> Thus, inded current function is pointless at this point.
> I will merge this expansion patch in v5
1 correction.
sound graph might have multi ports (= not multi endpoint), like below.
Each endpoints are 1:1.
ports {
port { endpoint }; /* ID = 0 */
port { endpoint }; /* ID = 1 */
port { endpoint }; /* ID = 2 */
};
1 question
It will support HDMI sound feature, thus I separated
it into OF-graph (= this patch-set) and HDMI (= next patch-set).
Should I merge it ?
Below is the expansion patch for HDMI support
----------------------
Subject: [PATCH 14/63] ASoC: add .of_xlate_dai_id() callback
ALSA SoC needs to know connected DAI ID for probing.
It is not a big problem if device/driver was only for sound,
but getting DAI ID will be difficult if device includes both
Video/Sound, like HDMI.
To solve this issue, this patch adds new .of_xlate_dai_id callback
on component driver, and use it from snd_soc_get_dai_id()
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
include/sound/soc.h | 3 +++
sound/soc/soc-core.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 95abbcb..0055fa0 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -14,6 +14,7 @@
#define __LINUX_SND_SOC_H
#include <linux/of.h>
+#include <linux/of_graph.h>
#include <linux/platform_device.h>
#include <linux/types.h>
#include <linux/notifier.h>
@@ -793,6 +794,8 @@ struct snd_soc_component_driver {
int (*of_xlate_dai_name)(struct snd_soc_component *component,
struct of_phandle_args *args,
const char **dai_name);
+ int (*of_xlate_dai_id)(struct snd_soc_component *comment,
+ struct device_node *endpoint);
void (*seq_notifier)(struct snd_soc_component *, enum snd_soc_dapm_type,
int subseq);
int (*stream_event)(struct snd_soc_component *, int event);
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 7a10522..07e4eec 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -4042,12 +4042,45 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
int snd_soc_get_dai_id(struct device_node *ep)
{
+ struct snd_soc_component *pos;
struct device_node *node;
struct device_node *endpoint;
int i, id;
+ int ret;
node = of_graph_get_port_parent(ep);
+ /*
+ * For example HDMI case, HDMI has video/sound port,
+ * but ALSA SoC needs sound port number only.
+ * Thus counting HDMI DT port/endpoint doesn't work.
+ * Then, it should have .of_xlate_dai_id
+ */
+ ret = -ENOTSUPP;
+ mutex_lock(&client_mutex);
+ list_for_each_entry(pos, &component_list, list) {
+ struct device_node *component_of_node = pos->dev->of_node;
+
+ if (!component_of_node && pos->dev->parent)
+ component_of_node = pos->dev->parent->of_node;
+
+ if (component_of_node != node)
+ continue;
+
+ if (pos->driver->of_xlate_dai_id)
+ ret = pos->driver->of_xlate_dai_id(pos, ep);
+
+ break;
+ }
+ mutex_unlock(&client_mutex);
+
+ if (ret != -ENOTSUPP)
+ return ret;
+
+ /*
+ * Non HDMI sound case, counting port/endpoint on its DT
+ * is enough. Let's count it.
+ */
i = 0;
id = -1;
for_each_endpoint_of_node(node, endpoint) {
--
1.9.1
----------------------
Best regards
---
Kuninori Morimoto
^ permalink raw reply related
* Re: [PATCH 2/5] media: Add support for CXD2880 SPI I/F
From: Mauro Carvalho Chehab @ 2017-04-10 1:38 UTC (permalink / raw)
To: Takiguchi, Yasunari
Cc: kbuild test robot, kbuild-all@01.org,
linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, tbird20d@gmail.com,
frowand.list@gmail.com, Yamamoto, Masayuki, Nozawa, Hideki (STWN),
Yonezawa, Kota, Matsumoto, Toshihiko, Watanabe, Satoshi (SSS)
In-Reply-To: <02699364973B424C83A42A84B04FDA8533BC79@JPYOKXMS113.jp.sony.com>
Em Fri, 7 Apr 2017 08:19:58 +0000
"Takiguchi, Yasunari" <Yasunari.Takiguchi@sony.com> escreveu:
> Dear All
>
> Our patches consists of the following items.
> [PATCH 1/5] dt-bindings: media: Add document file for CXD2880 SPI I/F
> [PATCH 2/5] media: Add support for CXD2880 SPI I/F
> [PATCH 3/5] media: Add suppurt for CXD2880
> [PATCH 4/5] media: Add suppurt for CXD2880 DVB-T2/T functions
> [PATCH 5/5] media: Update MAINTAINERS file for CXD2880
Didn't review your patch series yet. It should take a while to
review 14K lines ;)
> It is necessary to apply all patches before compiling kernel with our code.
>
> Could you re-compile after applying above the patches.
The kbuild test robot is... a machine. It won't read or answer to
your reply :-)
It automatically tests all patches sent to the ML and apply them at the
order the patches are found at the tree, one by one.
The rule is that no patch can cause compilation breakages, as
this breaks Kernel bisectability.
So, you have to ensure, when sending a patch series, that every single
patch is compilable, and won't cause runtime issues.
In the specific case of this patch:
> > drivers/media/spi/Kconfig | 14 +
> > drivers/media/spi/Makefile | 5 +
> > drivers/media/spi/cxd2880-spi.c | 727 +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 746 insertions(+)
> > create mode 100644 drivers/media/spi/cxd2880-spi.c
If cxd2880-spi.c need something else in order to be built, you
should not add it at Kconfig/Makefile yet. Just add the files,
using an order that makes it easier to be reviewed. In this
particular case, if it needs a header file, the patch containing
the header file should ideally be sent together with it
(or a previous patch, if adding it together wouldn't work).
Also, what I usually do when submitting drivers is that I don't
touch Kconfig/Makefile too early, putting such changes by the end
of the patch series.
E. g. you could break this patch on two separate patches, where the
first one would be just adding the cxd2880-spi.c (and cxd2880.h). Another
possibility would be to move this patch to the end, if the other patches
don't break git bisect (e. g. if they all compile without problems) and if
they're good enough to be reviewed by someone using the patch order.
Thanks,
Mauro
^ permalink raw reply
* RE: [PATCH v4 0/9] Renesas RZ/A1 pin and gpio controller
From: Chris Brandt @ 2017-04-10 2:33 UTC (permalink / raw)
To: Jacopo Mondi, linus.walleij@linaro.org, geert+renesas@glider.be,
laurent.pinchart@ideasonboard.com, robh+dt@kernel.org,
mark.rutland@arm.com, linux@armlinux.org.uk
Cc: linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1491401247-7030-1-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
On Wednesday, April 05, 2017, Jacopo Mondi wrote:
> v3 -> v4:
> - use "pinmux" property in pmx sub-nodes in place of "renesas,pins"
> - use pinconf standard properties to set pin mux additional flags
> - add "bi-directional" and "output-enable" to pinconf generic properties
> - perform pmx function parsing at dt_node_to_map() time
> - change DT bindings to use GENERIC_PINCONF
> - change DT bindings to allow sub-nodes to have "pinmux" property
> specified
> - several renames (register names, DT parse functions, set_mux() function)
I just tested this driver on the RZ/A1 RSK board.
The following worked good.
SCIF2, I2C, SDHI, Ethernet
SDHI also has bi-direction pins. For your reference, here was my DT:
/* SHDI ch1 on CN1 */
sdhi1_pins: sdhi1 {
pins {
pinmux = <RZA1_PINMUX(3, 8, 7)>, /* SD_CD_1 */
<RZA1_PINMUX(3, 9, 7)>, /* SD_WP_1 */
<RZA1_PINMUX(3, 12, 7)>, /* SD_CLK_1 */
<RZA1_PINMUX(3, 13, 7)>; /* SD_CMD_1 */
};
pins_bidir {
pinmux = <RZA1_PINMUX(3, 10, 7)>, /* SD_D1_1 */
<RZA1_PINMUX(3, 11, 7)>, /* SD_D0_1 */
<RZA1_PINMUX(3, 14, 7)>, /* SD_D3_1 */
<RZA1_PINMUX(3, 15, 7)>; /* SD_D2_1 */
bi-directional;
};
};
Thanks,
Chris
^ permalink raw reply
* Re: [PATCH v2 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Joel Stanley @ 2017-04-10 3:07 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring,
Jeremy Kerr, linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree,
Benjamin Herrenschmidt, OpenBMC Maillist
In-Reply-To: <CAHp75VckYiBccVN+_K+Tm38kcdC10=sekQZg7Uz_HwS4xyW2UA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Apr 5, 2017 at 8:24 PM, Andy Shevchenko
<andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Apr 5, 2017 at 7:03 AM, Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> wrote:
>
>> + port.port.irq = irq_of_parse_and_map(np, 0);
>
> Isn't better to get this via platform_get_irq() ?
I can't see the benefit.
>
>> + port.port.irqflags = IRQF_SHARED;
>> + port.port.iotype = UPIO_MEM;
>
>> + if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
>
> I would still go with usual pattern.
>
>> + switch (prop) {
>> + case 1:
>> + port.port.iotype = UPIO_MEM;
>> + break;
>> + case 4:
>
>> + port.port.iotype = of_device_is_big_endian(np) ?
>> + UPIO_MEM32BE : UPIO_MEM32;
>
> Hmm... And this one is not in align with IO accessors used in this
> driver. (readx()/writex() are little endian IO accessors).
We only perform readb/writeb, however you raise a good point that
we're assuming LE in the register layout. I will remove checking of this
optional property.
I will send v3 with the other cleanups you mentioned.
Cheers,
Joel
>
>> + break;
>> + default:
>> + dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n",
>> + prop);
>> + rc = -EINVAL;
>> + goto err_clk_disable;
>> + }
>> + }
>> +
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox