* Re: Add Allwinner Q8 tablets hardware manager
From: Pierre-Hugues Husson @ 2016-10-27 17:31 UTC (permalink / raw)
To: Hans de Goede
Cc: Mark Rutland, devicetree, Arnd Bergmann, Greg Kroah-Hartman,
Pantelis Antoniou, Chen-Yu Tsai, Rob Herring, Maxime Ripard,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <4cfdd415-1965-7be9-b204-86a9931683a6-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-10-27 16:53 GMT+02:00 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> Hi,
>
> No, the firmware-name (and matching resolution as different firmwares
> report different axis-ranges for the same digitizer) is selected
> primarily by the touchscreen_variant which sets: touchscreen_fw_name,
> touchscreen_width and touchscreen_height.
>
> The touchscreen_variant module option defaults to -1 which means "auto",
> when it is auto it gets set based on the touchscreen / accelerometer
> combination (which more or less uniquely identifies boards sofar),
> likewise all the other touchscreen module options default to -1,
> but can be overridden from the commandline.
>
> The intention is for things to just work, the commandline options are
> there as a fallback.
Ok, so I was really off. Sorry.
Now I think I understand the full complexity of the problem.
> We could just have:
>
> i2c-probe-stop-at-first-match-0 = <&touchscreen1>, <&touchscreen2>,
> <&touchscreen3>;
> i2c-probe-stop-at-first-match-1 = <&accelerometer1>,
> <&accelerometer2>;
>
> And have the i2c bus code look for an i2c-probe-stop-at-first-match-[i++]
> property
> until it is not found. Having a child-node with its own compatible for this
> feels wrong, as it uses a hierarchy where there really is none.
Ok, looks much better indeed.
I had one case where accelerometers could be on either i2c1 or i2c5.
Do you think this could be handled as well, or this makes things much
more complicated to fit in the i2c driver?
> So this would require us to be able to filter (to use your example)
> on if another i2c device is found and on which address it is found,
> that does not even take the rda559x check into account and is
> going to cause interesting ordering issues, how do we know when
> we can actually do the filtering if some of the variables we are
> filtering on are set by other auto-detected paths. Which auto-detect /
> i2c-probe-stop-at-first-match list do we execute first ? Worse
> actually for accelerometer orientation I will likely need to
> set the mount-matrix based on the detected touchscreen ...
>
> The rda559x here is a sdio wifi chip, which is also connected to the
> i2c, and currently is detected through i2c to be able to separately
> identify 2 q8 boards which share the same touchscreen + accelerometer
> combination and who knows what other checks I or other people can
> come up with to differentiate board variants which do not have
> a simple eeprom to uniquely id them.
>
> So as said before, no this cannot be all done in dt without
> adding a turing complete language to dt, and that is just to
> select which touchscreen_variant to use.
Ok, now that I understand the scope of your needs.
I agree with you, this needs a (close to) turing complete language.
I'm still not really happy about doing it in a driver, but I agree the
full scope you need is scarce enough.
Assuming this is done in a driver, there would need to be some
plumbing between i2c-probe-stop-at-first-match, device's probe
function and your driver, so that your driver only does the various
if/cases and DT changes, but there is no actual device communication
done in that driver.
> Then there also the probem of the combinatorial explosion having
> not only 2 firmware files but also invert-x and invert-y flags causes:
> We have 2 revisions with each 2 different firmware-files (more actually
> but I've reduced the set since some firmwares are compatible) with each
> both the x- and / or y axis as normal or inverted, for a total of:
> 2 (revision) * 2 (firmware-files) * 2 (x-inverted or not) * 2 (y...) = 16
> touchscreen variants, which means dt nodes for touchscreen1 to touchscreen16
> and that is just the silead gsl1680, some of these tablets also have
> elan or zeitec touchscreen controllers.
>
> Now imagine what happens if a new board comes out which needs a 3th firmware
> file... I hope you can understand this is not a route I want to go.
Right, I agree with you.
> Another problem is that if a user encounters the need for a new firmware
> variant he can now not easily try this (where as before we had
> module options to separately override firmware-name, the size, etc.
>
> As written in my previous mail, this is all rather gsl1680 specific,
> and esp. being able to override the firmware-name, the size, etc.
> through module options is going to be useful (to ask endusers to test
> stuff without recompiling) on x86 too. So we will likely want to add
> most of the necessary stuff to the silead driver anyways.
That's not currently the case, but can't we assume it will become easy
for users to install a DT overlay?
This would drop all your needs of module parameters, and would
actually be more modulable than your current scheme.
It is a more longer term thought, and could instead apply to further
boards having the same sort of troubles as you, rather than for this
first driver.
--
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: [RFC PATCH 0/5] Add an overlay manager to handle board capes
From: Rob Herring @ 2016-10-27 17:30 UTC (permalink / raw)
To: Hans de Goede
Cc: Antoine Tenart, Maxime Ripard, Pantelis Antoniou, Mark Rutland,
Stephen Boyd, Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Frank Rowand,
Dmitry Shmidt
In-Reply-To: <4ca9db09-e52c-11ec-133b-8f193b9b7174-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Thu, Oct 27, 2016 at 10:13 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Hi,
>
> On 27-10-16 15:41, Rob Herring wrote:
>>
>> Please Cc the maintainers of drivers/of/.
>>
>> + Frank R, Hans, Dmitry S
>>
>> On Wed, Oct 26, 2016 at 9:57 AM, Antoine Tenart
>> <antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>
>>> Hi all,
>>>
>>> Many boards now come with dips and compatible capes; among others the
>>> C.H.I.P, or Beaglebones. All these boards have a kernel implementing an
>>> out-of-tree "cape manager" which is used to detected capes, retrieve
>>> their description and apply a corresponding overlay. This series is an
>>> attempt to start a discussion, with an implementation of such a manager
>>> which is somehow generic (i.e. formats or cape detectors can be added).
>>> Other use cases could make use of this manager to dynamically load dt
>>> overlays based on some input / hw presence.
>>
>>
>> I'd like to see an input source be the kernel command line and/or a DT
>> chosen property. Another overlay manager was proposed not to long
>> ago[1] as well. There's also the Allwinner tablet use case from Hans
>> where i2c devices are probed and detected. That's not using overlays
>> currently, but maybe could.
>
>
> Actually I'm currently thinking in a different direction, which I
> think will be good for the boards where some ICs are frequently
> replaced by 2nd (and 3th and 4th) sources, rather then that we're
> dealing with an extension connector with capes / daughter boards.
>
> Although there is some overlap I'm starting to think that we need to
> treat these 2 cases differently. Let me quickly copy and paste
> the basic idea I've for the 2nd source touchscreen / accelerometer
> chip case:
>
> """
> The kernel actually already has a detect() method in struct i2c_driver,
> we could use that (we would need to implement it in drivers which do not
> have it yet). Note on second thought it seems it may be better to use
> probe() for this, see below.
>
> Then we could have something like this in dt:
>
> &i2c0 {
> touchscreen1: gsl1680@40 {
> reg = <0x40>;
> compatible = "silead,gsl1680";
> enable-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
> status = "disabled";
> };
>
> touchscreen2: ektf2127@15 {
> reg = <0x15>;
Do you ever have different devices with the same address? That would
be somewhat problematic as really these should be
"touchscreen@<addr>".
> compatible = "elan,ektf2127";
> enable-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
> status = "disabled";
> };
>
> i2c-probe-stop-at-first-match-0 = <&touchscreen1>, <&touchscreen2>;
> i2c-probe-stop-at-first-match-1 = <&accelerometer1>, <&accelerometer2>;
> }
>
> Which would make the i2c subsys call detect (*) on each device, until
> a device is found. Likewise we could have a "i2c-probe-all" property
> which also walks a list of phandles but does not stop on the first
> match.
>
> ...
>
> *) Yes this sounds Linux specific, but it really is just "execute
> to-be-probed
> device compatible specific detection method"
> """
Yeah, not a fan of these properties at first glance. Why can't you
just fail probe on the non-existent devices?
> This does not 100% solve all q8 issues (see the "Add Allwinner Q8 tablets
> hardware manager" thread), but does solve quite a bit of the use-case
> and this matches what many vendor os-images (typically android) are
> actually doing for these kind of boards.
BTW, I've been meaning to ask you if you are looking at the Android
side of things as well?
> As for the bits this does not solve, those are mostly board specific details
> which cannot be probed at all, and on x86 are typically solved in the device
> driver by doing a dmi check to identify the board and then apply a board
> specific workaround in the driver.
>
> I've come to believe that we should similarly delegate dealing this to
> device
> drivers in the devicetree case. Note that dt should still of course fully
> describe the hardware for normal hardware, the driver would just need to
> care
> about weird board quirks in certain exceptions.
Which is fine IMO, though I do think we should look at those cases
carefully to ensure they stay the exception.
> A more interesting problem here is that dt does not have something like
> DMI, there is the machine compatible, but that typically does not contain
> board revision info (where as DMI often does). I believe that this is
> actually something which should be fixed at the bootloader level
> have it prepend a new machine compatible which contains revision info.
>
> Hmm, if we make the bootloader prepend a new machine compatible which
> contains
> revision info, we could then trigger quirks on this and in some cases avoid
> the need for dealing with board quirks in the driver ...
That would work. Board and chip versions both need better handling in
kernel IMO.
QCom has a whole scheme around version numbering in compatible
strings. (Unfortunately, bootloaders only support their previous way
of doing things.)
> Note this is all very specific to dealing with board (revision) variants,
> for add-ons having the bootloader add info to the machine compatible does
> not seem the right solution.
Agreed.
Rob
--
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 2/7] ASoC: sunxi: Add a simple HDMI CODEC
From: Jean-Francois Moine @ 2016-10-27 17:16 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Dave Airlie, Liam Girdwood, Mark Brown, Maxime Ripard,
Rob Herring, Linux-ALSA, devicetree, dri-devel, linux-arm-kernel,
linux-sunxi
In-Reply-To: <CAGb2v67cE8v3=XtTzDBst2D2U6wM8pffQYK9nEok+j4xnPT39A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, 28 Oct 2016 00:54:34 +0800
Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> wrote:
> There's already a driver for basically the same thing:
>
> sound/soc/codec/hdmi-codec.c
>
> You use it by registering a sub-device from your hdmi driver, with the
> proper platform_data and callbacks. Grep for HDMI_CODEC_DRV_NAME for
> platforms already using it.
I know that for a long time, and I will not use it on any account: it is
a gasworks!
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [PATCH 3/3] ARM: dts: exynos: Document eMMC/SD/SDIO devices in Exynos5800 Peach Pi board
From: Javier Martinez Canillas @ 2016-10-27 17:11 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Javier Martinez Canillas, devicetree-u79uwXL29TY76Z2rM5mHXA,
Kukjin Kim, Russell King,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Mark Rutland, Krzysztof Kozlowski,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1477588303-13681-1-git-send-email-javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
There's a cognitive load to figure out which mmc device nodes corresponds
to the eMMC flash, uSD card and WiFI SDIO module on the Peach Pi board.
So it's better to have comments in the DTS to make this more clear.
Signed-off-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
---
arch/arm/boot/dts/exynos5800-peach-pi.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index 01f466816fea..f9ff7f07ae0c 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -665,6 +665,7 @@
status = "okay";
};
+/* eMMC flash */
&mmc_0 {
status = "okay";
num-slots = <1>;
@@ -683,6 +684,7 @@
bus-width = <8>;
};
+/* WiFi SDIO module */
&mmc_1 {
status = "okay";
num-slots = <1>;
@@ -702,6 +704,7 @@
vqmmc-supply = <&buck10_reg>;
};
+/* uSD card */
&mmc_2 {
status = "okay";
num-slots = <1>;
--
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 2/3] ARM: dts: exynos: Document eMMC/SD/SDIO devices in Exynos5420 Pit board
From: Javier Martinez Canillas @ 2016-10-27 17:11 UTC (permalink / raw)
To: linux-kernel
Cc: Javier Martinez Canillas, devicetree, Kukjin Kim, Russell King,
linux-samsung-soc, Rob Herring, Mark Rutland, Krzysztof Kozlowski,
linux-arm-kernel
In-Reply-To: <1477588303-13681-1-git-send-email-javier@osg.samsung.com>
There's a cognitive load to figure out which mmc device nodes corresponds
to the eMMC flash, uSD card and WiFI SDIO module on the Peach Pit board.
So it's better to have comments in the DTS to make this more clear.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
arch/arm/boot/dts/exynos5420-peach-pit.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index ec4a00f1ce01..1f964ec35c5e 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -697,6 +697,7 @@
status = "okay";
};
+/* eMMC flash */
&mmc_0 {
status = "okay";
num-slots = <1>;
@@ -714,6 +715,7 @@
bus-width = <8>;
};
+/* WiFi SDIO module */
&mmc_1 {
status = "okay";
num-slots = <1>;
@@ -733,6 +735,7 @@
vqmmc-supply = <&buck10_reg>;
};
+/* uSD card */
&mmc_2 {
status = "okay";
num-slots = <1>;
--
2.7.4
^ permalink raw reply related
* [PATCH 1/3] ARM: dts: exynos: Document eMMC/SD/SDIO devices in Exynos5250 Snow board
From: Javier Martinez Canillas @ 2016-10-27 17:11 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Javier Martinez Canillas, devicetree-u79uwXL29TY76Z2rM5mHXA,
Kukjin Kim, Russell King,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Mark Rutland, Krzysztof Kozlowski,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
There's a cognitive load to figure out which mmc device node corresponds
to the eMMC flash, uSD card and WiFI SDIO module on the Snow boards.
So it's better to have comments in the DTS to make this more clear.
Signed-off-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
---
arch/arm/boot/dts/exynos5250-snow-common.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5250-snow-common.dtsi b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
index d5d51916bb74..8f3a80430748 100644
--- a/arch/arm/boot/dts/exynos5250-snow-common.dtsi
+++ b/arch/arm/boot/dts/exynos5250-snow-common.dtsi
@@ -523,6 +523,7 @@
status = "okay";
};
+/* eMMC flash */
&mmc_0 {
status = "okay";
num-slots = <1>;
@@ -536,6 +537,7 @@
cap-mmc-highspeed;
};
+/* uSD card */
&mmc_2 {
status = "okay";
num-slots = <1>;
@@ -553,6 +555,8 @@
/*
* On Snow we've got SIP WiFi and so can keep drive strengths low to
* reduce EMI.
+ *
+ * WiFi SDIO module
*/
&mmc_3 {
status = "okay";
--
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
* Re: Add Allwinner Q8 tablets hardware manager
From: Pantelis Antoniou @ 2016-10-27 17:11 UTC (permalink / raw)
To: Pierre-Hugues Husson
Cc: Hans de Goede, Mark Rutland, devicetree, Arnd Bergmann,
Greg Kroah-Hartman, Chen-Yu Tsai, Rob Herring, Maxime Ripard,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <CAJ-oXjT=eVwEk7t5WoZAhjMhiZZkq0NN5tFLidSnUNO97D62zQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Pierre,
> On Oct 27, 2016, at 19:59 , Pierre-Hugues Husson <phh-8tEavu1zA38@public.gmane.org> wrote:
>
> 2016-10-27 17:52 GMT+02:00 Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>:
>> Yes there is no EEPROM but you might be able to map probing results to
>> a fake ‘model’ number.
>>
>> Let me expand a bit:
>>
>> Assume that you have a number of probing steps, for example A, B, C each
>> returning true or false, and C being executed only when B is ‘true’ you
>> could do this to generate a bit field that identifies it.
>>
>> For example let’s assume that model FOO’s probing steps are
>>
>> A false, B true, C false -> 010
>>
>> Model’s BAR
>>
>> A true, B false, C don’t care -> 10x
>>
>> Mapping these to models could be
>>
>> Model FOO, (010 & 111) == 010 (all three probing steps must match)
>>
>> Model BAR, (10x & 110) = 100 (the first two probing steps must match)
>
> This method looks too complex on multiple grounds.
> Assuming your method, I'm not too sure how this would actually be
> described in a DTS.
> Such probing steps should include reading/matching IDs in an EEPROM/on
> an ADC, but it should also include the result of a driver's probe.
> Also, drivers should have a way to report an ID/OTP instead of just a
> boolean.
>
Err, I don’t think you got the point.
The probing steps are done by a board specific probe driver.
This driver performs the probing steps (which is exactly what Hans’s
method now does) but instead of applying changes to the device tree
programmatically generates a model string.
This model string can be used by a general purpose overlay manager to apply
the overlay(s) for the specific board. The plural part is important - read
below.
> As you mentioned, it is a way to distinguish models, not just a set of
> parameters.
> Does this mean that this DT would lead to loading various DT based on
> the matching model, which would look like a FIT?
> Also there is a modularity problem there. If I have phones with either
> screen A or screen B, and with either accelerometer A or accelerometer
> B, I would have to implement all four combinations.
>
The model lookup need not result in a simple overlay to apply.
So for your case it would be:
model corp,0 -> overlay screen A + overlay accel A
model corp,1 -> overlay screen A + overlay accel B
model corp,2 -> overlay screen B + overlay accel A
model corp,3 -> overlay screen B + overlay accel B
You don’t need the combinatorial number of overlays.
> I'm starting to agree with Hans, and to be able to implement
> everything he needs, would require a turing complete device-tree,
> which can include and apply device-tree overlays.
> This doesn't mean it can't be done, nor that it shouldn't be done, but
> that's a lot of work.
>
> Hans' i2c-probe-stop-at-first-match does make sense for most usecases,
> but I have two problems with it:
> 1. It is I2C specific (as I've mentioned earlier, I have the same
> needs with DSI panels)
> 2. This looks like a temporary solution if a turing-complete solution
> is to be implemented.
A custom I2C method would not be optimal IMO.
Regards
— Pantelis
--
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 5/6] ARM: tegra: Add Tegra20 GMI support
From: kbuild test robot @ 2016-10-27 17:09 UTC (permalink / raw)
Cc: kbuild-all, swarren, thierry.reding, jonathanh, gnurou, linux,
pdeschrijver, pgaikwad, mturquette, sboyd, robh+dt, mark.rutland,
devicetree, linux-tegra, linux-kernel, linux-arm-kernel,
linux-clk, Mirza Krak
In-Reply-To: <1477576872-2665-6-git-send-email-mirza.krak@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]
Hi Mirza,
[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.9-rc2 next-20161027]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Mirza-Krak/clk-tegra-add-TEGRA20_CLK_NOR-to-init-table/20161027-225528
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-davinci_all_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
>> Error: arch/arm/boot/dts/tegra20.dtsi:1.1-8 syntax error
FATAL ERROR: Unable to parse input tree
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 20355 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] Documentation: dt: reset: Add TI SCI reset binding
From: Suman Anna @ 2016-10-27 17:07 UTC (permalink / raw)
To: Andrew F. Davis, Nishanth Menon, Tero Kristo, Santosh Shilimkar,
Philipp Zabel, Rob Herring, Mark Rutland
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161026193938.25093-2-afd-l0cyMroinI0@public.gmane.org>
Hi Andrew,
On 10/26/2016 02:39 PM, Andrew F. Davis wrote:
> Add TI SCI reset controller binding. This describes the DT binding
> details for a reset controller node providing reset management services
> to hardware blocks (reset consumers) using the Texas Instrument's System
> Control Interface (TI SCI) protocol to communicate to a system controller
> block present on the SoC.
>
> Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
> [s-anna-l0cyMroinI0@public.gmane.org: revise the binding format]
> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
> ---
> .../devicetree/bindings/reset/ti,sci-reset.txt | 68 ++++++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 69 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reset/ti,sci-reset.txt
>
> diff --git a/Documentation/devicetree/bindings/reset/ti,sci-reset.txt b/Documentation/devicetree/bindings/reset/ti,sci-reset.txt
> new file mode 100644
> index 0000000..189e284
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/ti,sci-reset.txt
> @@ -0,0 +1,68 @@
> +Texas Instruments System Control Interface (TI-SCI) Reset Controller
> +=====================================================================
> +
> +Some TI SoCs contain a system controller (like the Power Management Micro
> +Controller (PMMC) on Keystone K2G SoC) that are responsible for controlling
> +the state of the various hardware modules present on the SoC. Communication
> +between the host processor running an OS and the system controller happens
> +through a protocol called TI System Control Interface (TI-SCI protocol).
> +For TI SCI details, please refer to the document,
> +Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> +
> +TI-SCI Reset Controller Node
> +============================
> +This reset controller node uses the TI SCI protocol to perform the reset
> +management of various hardware modules present on the SoC.
> +
> +Required properties:
> +--------------------
> + - compatible : Should be "ti,sci-reset"
> + - ti,sci : phandle to the TI SCI device to use for managing resets
This does not look correct. Looks like you may have posted an older
series. We should not be needing the ti,sci property anymore and this
node should be a child node of the pmmc. Look at Tero's ti-sci clock series.
> + - #reset-cells : Should be 2. Please see the reset consumer node below for
> + usage details.
> +
> +TI-SCI Reset Consumer Nodes
> +===========================
> +Each of the reset consumer nodes should have the following properties,
> +in addition to their own properties.
> +
> +Required properties:
> +--------------------
> + - resets : A phandle and reset specifier pair, one pair for each reset
> + signal that affects the device, or that the device manages.
> + The phandle should point to the TI-SCI reset controller node,
> + and the reset specifier should have 2 cell-values. The first
> + cell should contain the device ID, the values of which are
> + specified in the <dt-bindings/genpd/<soc>.h> include file.
> + The second cell should contain the reset mask value used by
> + system controller, the values of which are specified in the
> + include file <dt-bindings/reset/<soc>.h>, where <soc> is the
> + name of the SoC involved, for example 'k2g'.
> +
> +Please also refer to Documentation/devicetree/bindings/reset/reset.txt for
> +common reset controller usage by consumers.
> +
> +
> +Example:
> +--------
> +The following example demonstrates both a TI-SCI reset controller node and a
> +consumer (a DSP device) on the K2G SoC.
> +
> +#include <dt-bindings/genpd/k2g.h>
> +#include <dt-bindings/reset/k2g.h>
> +
> +/ {
> + soc {
> + k2g_reset: k2g_reset {
> + compatible = "ti,sci-reset";
> + ti,sci = <&pmmc>;
> + #reset-cells = <2>;
> + };
This example is also wrong. Please update it to represent this node as a
child node of the pmmc.
regards
Suman
> +
> + dsp0: dsp0 {
> + ...
> + resets = <&k2g_reset K2G_DEV_CGEM0 K2G_DEV_CGEM0_DSP0_RESET>;
> + ...
> + };
> + };
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6e93976..d8068df 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11899,6 +11899,7 @@ F: drivers/soc/ti/ti_sci_pm_domains.c
> F: Documentation/devicetree/bindings/clock/ti,sci-clk.txt
> F: include/dt-bindings/clock/k2g.h
> F: drivers/clk/keystone/sci-clk.c
> +F: Documentation/devicetree/bindings/reset/ti,sci-reset.txt
>
> THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
> M: Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/4] Input: goodix - Tweak configuration to use passed in touchscreen resolution
From: Franklin S Cooper Jr @ 2016-10-27 17:03 UTC (permalink / raw)
To: Bastien Nocera, dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
octavian.purdila-ral2JQCrhuEAvxtiuMwx3w,
irina.tirdea-ral2JQCrhuEAvxtiuMwx3w,
merker-8fiUuRrzOP0dnm+yROfE0A, linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0,
nm-l0cyMroinI0
In-Reply-To: <1477564458.2458.10.camel-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org>
On 10/27/2016 05:34 AM, Bastien Nocera wrote:
> On Thu, 2016-10-20 at 14:59 -0500, Franklin S Cooper Jr wrote:
>> Some goodix touchscreen controllers aren't programed properly to
>> match the
>> display panel it is used on. Although the defaults may largely work
>> it is
>> also likely that the screen resolution will be incorrect. Therefore,
>> add support to allow via DT for the touchscreen resolution to be set
>> and
>> reprogram the controller to use this updated resolution.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>> ---
>> .../bindings/input/touchscreen/goodix.txt | 2 ++
>> drivers/input/touchscreen/goodix.c | 26
>> +++++++++++++++++++++-
>> 2 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
>> b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
>> index c98757a..ebc7cb7 100644
>> --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
>> @@ -19,6 +19,8 @@ Optional properties:
>> interrupt gpio pin as output to reset the
>> device.
>> - reset-gpios : GPIO pin used for reset
>>
>> + - touchscreen-size-x : horizontal resolution of touchscreen
>> (in pixels)
>> + - touchscreen-size-y : vertical resolution of touchscreen (in
>> pixels)
>> - touchscreen-inverted-x : X axis is inverted (boolean)
>> - touchscreen-inverted-y : Y axis is inverted (boolean)
>> - touchscreen-swapped-x-y : X and Y axis are swapped (boolean)
>> diff --git a/drivers/input/touchscreen/goodix.c
>> b/drivers/input/touchscreen/goodix.c
>> index 01e12f8..c2428e1 100644
>> --- a/drivers/input/touchscreen/goodix.c
>> +++ b/drivers/input/touchscreen/goodix.c
>> @@ -496,6 +496,10 @@ static void goodix_tweak_config(struct
>> goodix_ts_data *ts)
>> return;
>> }
>>
>> + /* Setting X and Y Resolution */
>> + put_unaligned_le16(ts->abs_x_max, &config[RESOLUTION_LOC]);
>> + put_unaligned_le16(ts->abs_y_max, &config[RESOLUTION_LOC +
>> 2]);
>> +
>> check_sum = goodix_calculate_checksum(ts->cfg_len, config);
>>
>> config[raw_cfg_len] = check_sum;
>> @@ -669,6 +673,8 @@ static int goodix_request_input_dev(struct
>> goodix_ts_data *ts)
>> static int goodix_configure_dev(struct goodix_ts_data *ts)
>> {
>> int error;
>> + bool alter_config = false;
>> + u32 max_x, max_y;
>>
>> ts->swapped_x_y = device_property_read_bool(&ts->client-
>>> dev,
>> "touchscreen-
>> swapped-x-y");
>> @@ -676,9 +682,27 @@ static int goodix_configure_dev(struct
>> goodix_ts_data *ts)
>> "touchscreen-
>> inverted-x");
>> ts->inverted_y = device_property_read_bool(&ts->client->dev,
>> "touchscreen-
>> inverted-y");
>> -
>
> No need for that linefeed removal.
>
>> goodix_read_config(ts);
>>
>> + if (device_property_present(&ts->client->dev, "touchscreen-
>> size-x") &&
>
> Is it expected that incomplete or invalid arguments do not generate
> errors? I'd expect that the presence of just one of those properties,
> or...
Looking back at this there is technically nothing wrong with only
wanting to set 1 property. So I will update this to decouple these two
properties.
>
>> + device_property_present(&ts->client->dev, "touchscreen-
>> size-y")) {
>> +
>> + device_property_read_u32(&ts->client->dev,
>> "touchscreen-size-x",
>> + &max_x);
>> +
>> + device_property_read_u32(&ts->client->dev,
>> "touchscreen-size-y",
>> + &max_y);
>> +
>> + if (max_x > 0 && max_y > 0) {
>
> ... invalid values for those properties would throw errors (either
> warnings, or lower severity messages).
I'll add a warning here indicating that the passed in value with be ignored.
>
>> + ts->abs_x_max = max_x;
>> + ts->abs_y_max = max_y;
>> + alter_config = true;
>> + }
>> + }
>> +
>> + if (alter_config)
>> + goodix_tweak_config(ts);
>> +
>> error = goodix_request_input_dev(ts);
>> if (error)
>> return error;
--
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: Add Allwinner Q8 tablets hardware manager
From: Pierre-Hugues Husson @ 2016-10-27 16:59 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Hans de Goede, Mark Rutland, devicetree, Arnd Bergmann,
Greg Kroah-Hartman, Chen-Yu Tsai, Rob Herring, Maxime Ripard,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <9B288597-7812-459D-A5C7-B61107751DA6-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
2016-10-27 17:52 GMT+02:00 Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/HQFizaE/u3fw@public.gmane.orgm>:
> Yes there is no EEPROM but you might be able to map probing results to
> a fake ‘model’ number.
>
> Let me expand a bit:
>
> Assume that you have a number of probing steps, for example A, B, C each
> returning true or false, and C being executed only when B is ‘true’ you
> could do this to generate a bit field that identifies it.
>
> For example let’s assume that model FOO’s probing steps are
>
> A false, B true, C false -> 010
>
> Model’s BAR
>
> A true, B false, C don’t care -> 10x
>
> Mapping these to models could be
>
> Model FOO, (010 & 111) == 010 (all three probing steps must match)
>
> Model BAR, (10x & 110) = 100 (the first two probing steps must match)
This method looks too complex on multiple grounds.
Assuming your method, I'm not too sure how this would actually be
described in a DTS.
Such probing steps should include reading/matching IDs in an EEPROM/on
an ADC, but it should also include the result of a driver's probe.
Also, drivers should have a way to report an ID/OTP instead of just a
boolean.
As you mentioned, it is a way to distinguish models, not just a set of
parameters.
Does this mean that this DT would lead to loading various DT based on
the matching model, which would look like a FIT?
Also there is a modularity problem there. If I have phones with either
screen A or screen B, and with either accelerometer A or accelerometer
B, I would have to implement all four combinations.
I'm starting to agree with Hans, and to be able to implement
everything he needs, would require a turing complete device-tree,
which can include and apply device-tree overlays.
This doesn't mean it can't be done, nor that it shouldn't be done, but
that's a lot of work.
Hans' i2c-probe-stop-at-first-match does make sense for most usecases,
but I have two problems with it:
1. It is I2C specific (as I've mentioned earlier, I have the same
needs with DSI panels)
2. This looks like a temporary solution if a turing-complete solution
is to be implemented.
--
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 2/4] Input: goodix - Allow tweaking of configuration file dynamically
From: Franklin S Cooper Jr @ 2016-10-27 16:58 UTC (permalink / raw)
To: Bastien Nocera, dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
octavian.purdila-ral2JQCrhuEAvxtiuMwx3w,
irina.tirdea-ral2JQCrhuEAvxtiuMwx3w,
merker-8fiUuRrzOP0dnm+yROfE0A, linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0,
nm-l0cyMroinI0
In-Reply-To: <1477564424.2458.9.camel-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org>
On 10/27/2016 05:33 AM, Bastien Nocera wrote:
> On Thu, 2016-10-20 at 14:59 -0500, Franklin S Cooper Jr wrote:
>> Some goodix touchscreen controllers don't have the correct
>> configuration
>> firmware for the display panel it is attached to. Therefore, settings
>> such
>> as touchscreen x and y size may need to be passed in via DT and have
>> the
>> panel reprogrammed for the updated touchscreen resolution.
>>
>> This patchset adds support for reading the current configuration
>> firmware
>> on the panel and allowing it to be modified and rewritten back to the
>> device.
>>
>> Currently this function is unused but later patches will make use of
>> it.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>> ---
>> drivers/input/touchscreen/goodix.c | 28 ++++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/input/touchscreen/goodix.c
>> b/drivers/input/touchscreen/goodix.c
>> index a43c8ca..01e12f8 100644
>> --- a/drivers/input/touchscreen/goodix.c
>> +++ b/drivers/input/touchscreen/goodix.c
>> @@ -478,6 +478,34 @@ static int goodix_get_gpio_config(struct
>> goodix_ts_data *ts)
>> return 0;
>> }
>>
>> +static void goodix_tweak_config(struct goodix_ts_data *ts)
>> +{
>> + u8 config[GOODIX_CONFIG_MAX_LENGTH];
>> + int error;
>> + int raw_cfg_len;
>> + u8 check_sum = 0;
>> +
>> + raw_cfg_len = ts->cfg_len - 2;
>> +
>> + error = goodix_i2c_read(ts->client, GOODIX_REG_CONFIG_DATA,
>> + config, ts->cfg_len);
>> + if (error) {
>> + dev_warn(&ts->client->dev,
>> + "Error reading config (%d), avoid tweaking
>> config\n",
>> + error);
>> + return;
>> + }
>
> Please add a placeholder comment here, to go along with the commit
> message explanation.
You just want a comment here stating that changes to the firmware should
be made here?
>
>> + check_sum = goodix_calculate_checksum(ts->cfg_len, config);
>> +
>> + config[raw_cfg_len] = check_sum;
>> + config[raw_cfg_len + 1] = 1;
>> +
>> + error = goodix_send_cfg(ts, ts->cfg_len, config);
>> + if (error)
>> + dev_warn(&ts->client->dev,
>> + "Error writing config (%d)\n", error);
>> +}
>> /**
>> * goodix_read_config - Read the embedded configuration of the panel
>> *
--
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: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from local function names
From: Rob Herring @ 2016-10-27 16:58 UTC (permalink / raw)
To: Frank Rowand
Cc: Pantelis Antoniou, Pantelis Antoniou,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <58122CC0.3090700-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Thu, Oct 27, 2016 at 11:35 AM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 10/27/16 05:47, Rob Herring wrote:
>> On Tue, Oct 25, 2016 at 3:58 PM, <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> From: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
>>
>> I prefer to leave the prefixes and this is getting into pointless churn.
>>
>>>
>>> Signed-off-by: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
>>> ---
>>> drivers/of/resolver.c | 10 +++++-----
>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>
> If I was just submitting this as a single patch, I would agree.
>
> But since I am making so many other changes, I think it makes
> sense to do this as part of this series. It is broken apart
> as a separate patch to be easy to review and not pollute any
> of the other patches in the series.
>
> The prefixes add no value for a local function, but they do
> add noise when reading code.
The value is when reading the calling function, you know the function
is a DT related function. You don't know it's a static function
without looking up the function name. That said, I wouldn't object to
code originally written either way, I just don't see the value in
changing it.
Rob
--
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: [RFC PATCH 13/13] of: Remove unused variable overlay_symbols
From: Frank Rowand @ 2016-10-27 16:57 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Rob Herring, devicetree, linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <58123122.8020908-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 10/27/16 09:53, Frank Rowand wrote:
> On 10/27/16 09:27, Frank Rowand wrote:
>> On 10/27/16 07:41, Pantelis Antoniou wrote:
>>> Hi Frank,
>>>
>>>
>>>> On Oct 25, 2016, at 23:59 , frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>>>
>>>> From: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
>>>>
>>>> This unused variable is a reminder that symbols in overlays are
>>>> not available to subsequent overlays. If such a feature is
>>>> desired then there are several ways it could be implemented.
>>>>
>>>
>>> Please don’t apply that. There’s a patch that actually imports
>>> the symbol table from overlays that subsequent operations
>>> work.
>>>
>>> Please see:
>>>
>>> https://patchwork.kernel.org/patch/9104701/
>>
>> Thanks for the pointer! When the import symbols patch is applied
>> then the comment in my patch header becomes incorrect. I will
>> change the patch comment to act is if the import symbols patch
>> is in place.
>>
>> But the node pointer that my patch removes is still not used
>> for anything, even if the import symbols patch is applied.
>>
>> Am I missing something?
>
> I was missing a later patch in the symbol import patch set that
> updated resolver.c to use the imported symbols. I'll go look
> at that.
Crap. I misread the file name in the patch that I thought was
updating resolver.c. It was actually overlay.c. So I am back
to my question of:
Am I missing something?
--
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/2] ARM: memory: da8xx-ddrctl: new driver
From: Kevin Hilman @ 2016-10-27 16:55 UTC (permalink / raw)
Cc: Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand,
Mark Rutland, Peter Ujfalusi, Russell King, LKML, arm-soc,
linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen,
David Airlie, Laurent Pinchart
In-Reply-To: <1477503355-2600-2-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:
> Create a new driver for the da8xx DDR2/mDDR controller and implement
> support for writing to the Peripheral Bus Burst Priority Register.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
[...]
> + for (; setting->name; setting++) {
> + knob = da8xx_ddrctl_match_knob(setting);
> + if (!knob) {
> + dev_warn(dev,
> + "no such config option: %s\n", setting->name);
> + continue;
> + }
> +
> + if (knob->reg > (res->end - res->start - sizeof(u32))) {
Why the "- sizeof(u32)"? Shouldn't this just be resource_size(res)?
(c.f. linux/ioport.h)
> + dev_warn(dev,
> + "register offset of '%s' exceeds mapped memory size\n",
> + knob->name);
> + continue;
> + }
> +
> + reg = __raw_readl(ddrctl + knob->reg);
> + reg &= knob->mask;
> + reg |= setting->val << knob->shift;
> +
> + dev_dbg(dev, "writing 0x%08x to %s\n", reg, setting->name);
> +
> + __raw_writel(reg, ddrctl + knob->reg);
Can you use the normal readl/writel here? Or explain why the raw ones
are needed?
> + }
> +
> + return 0;
> +}
> +
Otherwise, looks good to me. With the changes above, feel free to add
Reviewed-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Kevin
--
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 2/7] ASoC: sunxi: Add a simple HDMI CODEC
From: Chen-Yu Tsai @ 2016-10-27 16:54 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: Dave Airlie, Liam Girdwood, Mark Brown, Maxime Ripard,
Rob Herring, Linux-ALSA, devicetree, dri-devel, linux-arm-kernel,
linux-sunxi
In-Reply-To: <5cb540f20f64d28bd7dee82a0e14ee5209631979.1477142934.git.moinejf-GANU6spQydw@public.gmane.org>
On Fri, Oct 21, 2016 at 3:44 PM, Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org> wrote:
> Allwinner's SoCs include support for both audio and video on HDMI.
> This patch defines a simple audio CODEC which may be used in sunxi
> HDMI video drivers.
>
> Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
There's already a driver for basically the same thing:
sound/soc/codec/hdmi-codec.c
You use it by registering a sub-device from your hdmi driver, with the
proper platform_data and callbacks. Grep for HDMI_CODEC_DRV_NAME for
platforms already using it.
ChenYu
> ---
> include/sound/sunxi_hdmi.h | 23 +++++++++
> sound/soc/codecs/Kconfig | 9 ++++
> sound/soc/codecs/Makefile | 2 +
> sound/soc/codecs/sunxi-hdmi.c | 106 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 140 insertions(+)
> create mode 100644 include/sound/sunxi_hdmi.h
> create mode 100644 sound/soc/codecs/sunxi-hdmi.c
>
> diff --git a/include/sound/sunxi_hdmi.h b/include/sound/sunxi_hdmi.h
> new file mode 100644
> index 0000000..0986bb9
> --- /dev/null
> +++ b/include/sound/sunxi_hdmi.h
> @@ -0,0 +1,23 @@
> +#ifndef __SUNXI_HDMI_H__
> +#define __SUNXI_HDMI_H__
> +/*
> + * Copyright (C) 2016 Jean-François Moine
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +struct sunxi_hdmi_codec {
> + u8 *eld;
> + int (*set_audio_input)(struct device *dev,
> + int enable,
> + unsigned sample_rate,
> + unsigned sample_bit);
> +};
> +
> +int sunxi_hdmi_codec_register(struct device *dev);
> +void sunxi_hdmi_codec_unregister(struct device *dev);
> +
> +#endif /* __SUNXI_HDMI_H__ */
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index c67667b..53385b1 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -131,6 +131,7 @@ config SND_SOC_ALL_CODECS
> select SND_SOC_STA529 if I2C
> select SND_SOC_STAC9766 if SND_SOC_AC97_BUS
> select SND_SOC_STI_SAS
> + select SND_SOC_SUNXI_HDMI
> select SND_SOC_TAS2552 if I2C
> select SND_SOC_TAS5086 if I2C
> select SND_SOC_TAS571X if I2C
> @@ -793,6 +794,14 @@ config SND_SOC_STAC9766
> config SND_SOC_STI_SAS
> tristate "codec Audio support for STI SAS codec"
>
> +config SND_SOC_SUNXI_HDMI
> + tristate "Allwinner sunxi HDMI Support"
> + default m if DRM_SUNXI_DE2_HDMI=m
> + default y if DRM_SUNXI_DE2_HDMI=y
> + select SND_PCM_ELD
> + help
> + Enable HDMI audio output
> +
> config SND_SOC_TAS2552
> tristate "Texas Instruments TAS2552 Mono Audio amplifier"
> depends on I2C
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index 958cd49..35804eb 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -139,6 +139,7 @@ snd-soc-sta350-objs := sta350.o
> snd-soc-sta529-objs := sta529.o
> snd-soc-stac9766-objs := stac9766.o
> snd-soc-sti-sas-objs := sti-sas.o
> +snd-soc-sunxi-hdmi-objs := sunxi-hdmi.o
> snd-soc-tas5086-objs := tas5086.o
> snd-soc-tas571x-objs := tas571x.o
> snd-soc-tas5720-objs := tas5720.o
> @@ -359,6 +360,7 @@ obj-$(CONFIG_SND_SOC_STA350) += snd-soc-sta350.o
> obj-$(CONFIG_SND_SOC_STA529) += snd-soc-sta529.o
> obj-$(CONFIG_SND_SOC_STAC9766) += snd-soc-stac9766.o
> obj-$(CONFIG_SND_SOC_STI_SAS) += snd-soc-sti-sas.o
> +obj-$(CONFIG_SND_SOC_SUNXI_HDMI) += snd-soc-sunxi-hdmi.o
> obj-$(CONFIG_SND_SOC_TAS2552) += snd-soc-tas2552.o
> obj-$(CONFIG_SND_SOC_TAS5086) += snd-soc-tas5086.o
> obj-$(CONFIG_SND_SOC_TAS571X) += snd-soc-tas571x.o
> diff --git a/sound/soc/codecs/sunxi-hdmi.c b/sound/soc/codecs/sunxi-hdmi.c
> new file mode 100644
> index 0000000..0d08676
> --- /dev/null
> +++ b/sound/soc/codecs/sunxi-hdmi.c
> @@ -0,0 +1,106 @@
> +/*
> + * Allwinner HDMI codec
> + *
> + * Copyright (C) 2016 Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <sound/soc.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <sound/pcm_drm_eld.h>
> +#include <sound/pcm_params.h>
> +
> +#include "sound/sunxi_hdmi.h"
> +
> +#define SUNXI_HDMI_FORMATS (SNDRV_PCM_FMTBIT_S8 | \
> + SNDRV_PCM_FMTBIT_S16_LE | \
> + SNDRV_PCM_FMTBIT_S20_3LE | \
> + SNDRV_PCM_FMTBIT_S24_LE | \
> + SNDRV_PCM_FMTBIT_S32_LE)
> +
> +static int sunxi_hdmi_codec_startup(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + struct sunxi_hdmi_codec *priv = dev_get_drvdata(dai->dev);
> + u8 *eld;
> +
> + eld = priv->eld;
> + if (!eld)
> + return -ENODEV;
> +
> + return snd_pcm_hw_constraint_eld(runtime, eld);
> +}
> +
> +static int sunxi_hdmi_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *dai)
> +{
> + struct sunxi_hdmi_codec *priv = dev_get_drvdata(dai->dev);
> + unsigned sample_bit;
> +
> + if (params_format(params) == SNDRV_PCM_FORMAT_S16_LE)
> + sample_bit = 16;
> + else
> + sample_bit = 24;
> +
> + return priv->set_audio_input(dai->dev, true,
> + params_rate(params),
> + sample_bit);
> +}
> +
> +static void sunxi_hdmi_codec_shutdown(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct sunxi_hdmi_codec *priv = dev_get_drvdata(dai->dev);
> +
> + priv->set_audio_input(dai->dev, false, 0, 0);
> +}
> +
> +static const struct snd_soc_dai_ops sunxi_hdmi_codec_ops = {
> + .startup = sunxi_hdmi_codec_startup,
> + .hw_params = sunxi_hdmi_hw_params,
> + .shutdown = sunxi_hdmi_codec_shutdown,
> +};
> +
> +static struct snd_soc_dai_driver sunxi_hdmi_codec = {
> + .name = "hdmi", /* must be the name of the node in the DT */
> + .playback = {
> + .stream_name = "HDMI Playback",
> + .channels_min = 1,
> + .channels_max = 8,
> + .rates = SNDRV_PCM_RATE_CONTINUOUS,
> + .rate_min = 8000,
> + .rate_max = 192000,
> + .formats = SUNXI_HDMI_FORMATS,
> + },
> + .ops = &sunxi_hdmi_codec_ops,
> +};
> +
> +static const struct snd_soc_codec_driver sunxi_hdmi_codec_drv = {
> + .ignore_pmdown_time = true,
> +};
> +
> +/* functions called from the HDMI video driver */
> +int sunxi_hdmi_codec_register(struct device *dev)
> +{
> + return snd_soc_register_codec(dev, &sunxi_hdmi_codec_drv,
> + &sunxi_hdmi_codec, 1);
> +}
> +EXPORT_SYMBOL_GPL(sunxi_hdmi_codec_register);
> +
> +void sunxi_hdmi_codec_unregister(struct device *dev)
> +{
> + snd_soc_unregister_codec(dev);
> +}
> +EXPORT_SYMBOL_GPL(sunxi_hdmi_codec_unregister);
> +
> +MODULE_AUTHOR("Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>");
> +MODULE_DESCRIPTION("Allwinner HDMI CODEC");
> +MODULE_LICENSE("GPL");
> --
> 2.10.1
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [RFC PATCH 13/13] of: Remove unused variable overlay_symbols
From: Frank Rowand @ 2016-10-27 16:53 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Rob Herring, devicetree, linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <58122AF9.3040709-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 10/27/16 09:27, Frank Rowand wrote:
> On 10/27/16 07:41, Pantelis Antoniou wrote:
>> Hi Frank,
>>
>>
>>> On Oct 25, 2016, at 23:59 , frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>>
>>> From: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
>>>
>>> This unused variable is a reminder that symbols in overlays are
>>> not available to subsequent overlays. If such a feature is
>>> desired then there are several ways it could be implemented.
>>>
>>
>> Please don’t apply that. There’s a patch that actually imports
>> the symbol table from overlays that subsequent operations
>> work.
>>
>> Please see:
>>
>> https://patchwork.kernel.org/patch/9104701/
>
> Thanks for the pointer! When the import symbols patch is applied
> then the comment in my patch header becomes incorrect. I will
> change the patch comment to act is if the import symbols patch
> is in place.
>
> But the node pointer that my patch removes is still not used
> for anything, even if the import symbols patch is applied.
>
> Am I missing something?
I was missing a later patch in the symbol import patch set that
updated resolver.c to use the imported symbols. I'll go look
at that.
--
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 2/4] mfd: pv88080: MFD core support
From: kbuild test robot @ 2016-10-27 16:52 UTC (permalink / raw)
To: Eric Jeong
Cc: kbuild-all-JC7UmRfGjtg, LINUX-KERNEL, Lee Jones,
Alexandre Courbot, DEVICETREE, LINUX-GPIO, Liam Girdwood,
Linus Walleij, Mark Brown, Mark Rutland, Rob Herring,
Support Opensource
In-Reply-To: <6dcaf788e4ebffd5c753e5fc5ae67668f74f7c32.1477530195.git.eric.jeong-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]
Hi Eric,
[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.9-rc2 next-20161027]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Eric-Jeong/Documentation-pv88080-Move-binding-document/20161027-091356
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: alpha-allmodconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=alpha
All errors (new ones prefixed by >>):
drivers/built-in.o: In function `pv88080_i2c_probe':
>> drivers/mfd/pv88080-i2c.o:(.text+0x11652c): undefined reference to `__devm_regmap_init_i2c'
drivers/mfd/pv88080-i2c.o:(.text+0x116548): undefined reference to `__devm_regmap_init_i2c'
drivers/built-in.o: In function `pv88080_i2c_driver_init':
>> drivers/mfd/pv88080-i2c.o:(.init.text+0xa010): undefined reference to `i2c_register_driver'
drivers/mfd/pv88080-i2c.o:(.init.text+0xa020): undefined reference to `i2c_register_driver'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47625 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 00/13] of: Make drivers/of/resolver.c more readable
From: Frank Rowand @ 2016-10-27 16:36 UTC (permalink / raw)
To: Rob Herring
Cc: Pantelis Antoniou, Pantelis Antoniou,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqKX4nahQ4o1FtPLfqqbYM2-pm9QbsobaaRzo8z7arJEjQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 10/27/16 05:03, Rob Herring wrote:
> On Tue, Oct 25, 2016 at 4:02 PM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 10/25/16 13:58, frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>> From: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
>>>
>>> drivers/of/resolve.c is a bit difficult to read. Clean it up so
>>> that review of future overlay related patches will be easier.
>>
>> < snip >
>>
>> Hi Pantelis,
>>
>> Can you test this patch series on some real hardware?
>
> Did you run unit tests?
Yes.
But I will be much happier after Pantelis runs tests on real
hardware (which he agrees to do in another reply).
--
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: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from local function names
From: Frank Rowand @ 2016-10-27 16:35 UTC (permalink / raw)
To: Rob Herring
Cc: Pantelis Antoniou, Pantelis Antoniou,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqL9KWw7Mkz8nwZWDot+3OPBC2AesYuvu3dEofiB7BGeGQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 10/27/16 05:47, Rob Herring wrote:
> On Tue, Oct 25, 2016 at 3:58 PM, <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
>
> I prefer to leave the prefixes and this is getting into pointless churn.
>
>>
>> Signed-off-by: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
>> ---
>> drivers/of/resolver.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>
If I was just submitting this as a single patch, I would agree.
But since I am making so many other changes, I think it makes
sense to do this as part of this series. It is broken apart
as a separate patch to be easy to review and not pollute any
of the other patches in the series.
The prefixes add no value for a local function, but they do
add noise when reading code.
The changes are local to this file and do not impact anything
else.
Looking at the single patch, it does seem like churn. But
looking at the entire file before the set of changes and
after the set of changes, I find the file much easier to
read afterwards. Each individual patch may make a small
contribution to the end result, but the combination of all
of them is significant.
--
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: [RFC PATCH 13/13] of: Remove unused variable overlay_symbols
From: Frank Rowand @ 2016-10-27 16:27 UTC (permalink / raw)
To: Pantelis Antoniou; +Cc: Rob Herring, devicetree, linux-kernel, Frank Rowand
In-Reply-To: <27823D48-C6B1-486C-8460-9B3C33DDC62F@antoniou-consulting.com>
On 10/27/16 07:41, Pantelis Antoniou wrote:
> Hi Frank,
>
>
>> On Oct 25, 2016, at 23:59 , frowand.list@gmail.com wrote:
>>
>> From: Frank Rowand <frank.rowand@am.sony.com>
>>
>> This unused variable is a reminder that symbols in overlays are
>> not available to subsequent overlays. If such a feature is
>> desired then there are several ways it could be implemented.
>>
>
> Please don’t apply that. There’s a patch that actually imports
> the symbol table from overlays that subsequent operations
> work.
>
> Please see:
>
> https://patchwork.kernel.org/patch/9104701/
Thanks for the pointer! When the import symbols patch is applied
then the comment in my patch header becomes incorrect. I will
change the patch comment to act is if the import symbols patch
is in place.
But the node pointer that my patch removes is still not used
for anything, even if the import symbols patch is applied.
Am I missing something?
>
>> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
>> ---
>> drivers/of/resolver.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
>> index 3f7cf569c7ea..b48d16200ccd 100644
>> --- a/drivers/of/resolver.c
>> +++ b/drivers/of/resolver.c
>> @@ -272,7 +272,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
>> int of_resolve_phandles(struct device_node *overlay)
>> {
>> struct device_node *child, *local_fixups, *refnode;
>> - struct device_node *tree_symbols, *overlay_symbols, *overlay_fixups;
>> + struct device_node *tree_symbols, *overlay_fixups;
>> struct property *prop;
>> const char *refpath;
>> phandle phandle, phandle_delta;
>> @@ -302,12 +302,9 @@ int of_resolve_phandles(struct device_node *overlay)
>> if (err)
>> goto err_out;
>>
>> - overlay_symbols = NULL;
>> overlay_fixups = NULL;
>>
>> for_each_child_of_node(overlay, child) {
>> - if (!of_node_cmp(child->name, "__symbols__"))
>> - overlay_symbols = child;
>> if (!of_node_cmp(child->name, "__fixups__"))
>> overlay_fixups = child;
>> }
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Regards
>
> — Pantelis
>
>
^ permalink raw reply
* Re: Add Allwinner Q8 tablets hardware manager
From: Pantelis Antoniou @ 2016-10-27 16:27 UTC (permalink / raw)
To: Hans de Goede
Cc: Pierre-Hugues Husson, Mark Rutland, devicetree, Arnd Bergmann,
Greg Kroah-Hartman, Chen-Yu Tsai, Rob Herring, Maxime Ripard,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <4cfdd415-1965-7be9-b204-86a9931683a6-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hi Hans,
Nice to see other people coming up with similar problems.
It’s not a new thing at all, hacks like this have been around
since for ever (but safely tucked away in product specific areas).
> On Oct 27, 2016, at 17:53 , Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> Hi,
>
> On 27-10-16 14:57, Pierre-Hugues Husson wrote:
>> 2016-10-27 11:14 GMT+02:00 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
>>> In my experience with these cheap boards, there is a mix of auto-probing +
>>> device / revision specific os-image modifications. I keep coming back to
>>> the touchscreen controller firmware (but also the orientation), for the
>>> gsl1680 controller I need at least 2 different firmware files (per gsl1680
>>> revision) to make all q8 tablets I have working. This is simply not solved
>>> by the vendor android code, they just shove the right firmware into the
>>> os-image. Likewise for the touchscreen orientation (x-mirored, y-mirored,
>>> etc) too is just a hard-coded setting in the os-image.
>> Reading your patch, it looks like to handle the two different firmware
>> files, you're simply adding a command-line switch, there is no
>> detection involved.
>> Am I understanding correctly?
>
> No, the firmware-name (and matching resolution as different firmwares
> report different axis-ranges for the same digitizer) is selected
> primarily by the touchscreen_variant which sets: touchscreen_fw_name,
> touchscreen_width and touchscreen_height.
>
> The touchscreen_variant module option defaults to -1 which means "auto",
> when it is auto it gets set based on the touchscreen / accelerometer
> combination (which more or less uniquely identifies boards sofar),
> likewise all the other touchscreen module options default to -1,
> but can be overridden from the commandline.
>
> The intention is for things to just work, the commandline options are
> there as a fallback.
>
>> If this is the case, two things:
>> 1. I'm not too sure having the user choose this via cmdline is the
>> right way. I think I'd rather have it set by userspace. (though that's
>> not a strong opinion).
>> Or if cmdline is being changed... how about having DTS (or just an
>> overlay on top of it) being changed instead?
>>
>> 2. This could still be declared by DTS. For instance, assuming your
>> i2c-probe-stop-at-first-match:
>> &i2c0 {
>> touchscreen1: gsl1680@40 {
>> reg = <0x40>;
>> compatible = "silead,gsl1680";
>> enable-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
>> touchscreen-size = <1024 600>;
>> touchscreen-fw = "gsl1680-a082-q8-700.fw";
>> filter-names = "touchscreen_variant";
>> filter-0 = "none", "gsl1680-a082-q8-700";
>> id = <0xa0820000>;
>> status = "disabled";
>> };
>> touchscreen2: gsl1680@40 {
>> reg = <0x40>;
>> compatible = "silead,gsl1680";
>> enable-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
>> touchscreen-size = <480 800>;
>> touchscreen-fw = "gsl1680-a082-q8-a70.fw";
>> filter-names = "touchscreen_variant";
>> filter-0 = "gsl1680-a082-q8-a70";
>> id = <0xa0820000>;
>> status = "disabled";
>> };
>> touchscreen2: gsl1680@40 {
>> reg = <0x40>;
>> compatible = "silead,gsl1680";
>> enable-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
>> touchscreen-size = <960 640>;
>> touchscreen-fw = "gsl1680-b482-q8-d702.fw";
>> filter-names = "touchscreen_variant";
>> filter-0 = "gsl1680-b482-q8-d702";
>> id = <0xb4820000>;
>> status = "disabled";
>> };
>> i2c-probe-stop-at-first-match = <&touchscreen1>,
>> <&touchscreen2>, <&touchscreen3>;
>> }
>>
>> With "none" value being the value when the "touchscreen_variant"
>> option is not defined in cmdline.
>>
>> Please note that I'm not too sure whether SILEAD_REG_ID represents an
>> OTP which can be changed by OEM, or if it's more of a hardware
>> revision. Depending on this, this would either fit into a id =
>> <0xa0820000> DTS line, or a compatible = "silead,gsl1680_a082",
>> "silead,gsl1680"; DTS line.
>>
>>> Sofar I've only seen this with one type of touchscreen so an easy cop-out
>>> would be to add an "optional-vddio-supply" to the the bindings for the
>>> specific touchscreen use and put all the necessary logic in the driver.
>>>
>>> This does require propagating the learned need for the regulator
>>> from the drivers detect() callback to probe() or alternatively I'm
>>> thinking we should just use probe() instead of detect()to begin with,
>>> that will save a lot of duplication with things
>>> like code for enable gpio-s and regulators.
>>>
>>> So assuming we go for the cop-out option for 3. (I'm ok with that),
>>> this would be a pretty clean solution adding just the 2 new:
>>> i2c-probe-stop-at-first-match and i2c-probe-all properties to
>>> the i2c-bus bindings. One problem here is that we may want to have
>>> multiple i2c-probe-stop-at-first-match phandle lists on a single bus
>>> (e.g. try 3 touchscreens + 6 accelerometers on the same bus, stop at
>>> first touchscreen / first accelerometer), anyone have any ideas for
>>> that?
>> How about something like:
>>
>> &i2c1 {
>> touchscreen1....
>> touchscreen2....
>> touchscreen3....
>> accelerometer1....
>> accelerometer2....
>> accelerometer3....
>> accelerometer4....
>>
>> select-one {
>> compatible = "i2c-select;
>> group-names = "touchscreen", "accelerometer";
>> group-0 = <&touchscreen1>, <&touchscreen2>, <&touchscreen3>;
>> group-1 = <&accelerometer1>, <&accelerometer2>,
>> <&accelerometer3>, <&accelerometer4>;
>> };
>> };
>
> We could just have:
>
> i2c-probe-stop-at-first-match-0 = <&touchscreen1>, <&touchscreen2>, <&touchscreen3>;
> i2c-probe-stop-at-first-match-1 = <&accelerometer1>, <&accelerometer2>;
>
> And have the i2c bus code look for an i2c-probe-stop-at-first-match-[i++] property
> until it is not found. Having a child-node with its own compatible for this
> feels wrong, as it uses a hierarchy where there really is none.
>
>>>> When it comes to detection, I've witnessed various things.
>>>> It can be kernel-side or bootloader-side "global setting" reading (like an
>>>> ADC/resistor value, or an OTP), it can be bootloader doing the
>>>> "brute-force", or it can be the kernel doing all the probes.
>>>>
>>>> For instance, as of today, on a Spreadtrum ODM tree, the bootloader will
>>>> detect the screen by testing all knowns screens, the screen-drivers declare
>>>> a get_id function, and the bootloader probes until the get_id matches the id
>>>> declared by the screen driver.
>>>> And then the bootloader tells the kernel, via cmdline, which screen is
>>>> actually there (but auto-detection is also coded in kernel).
>>>> Finally all possible sensors/touchscreen/camera are declared in DTS, and
>>>> probe will filter-out N/C ones in the kernel.
>>>>
>>>> Now the big difference between my experience and what Hans is trying to
>>>> do, is that I've always worked with devices with "safely" queriable IDs,
>>>> either on i2c or dsi. I've never encountered SPI. This makes probing
>>>> inherently more dangerous, but I believe the question roughly remains the
>>>> same.
>>>
>>>
>>> I'm dealing with i2c too, Mark mistakenly used SPI in his reply,
>>> which I think is what got you thinking I've SPI.
>> Right, so let's concentrate on reasonable bus-es first then. (I can
>> think of I2C and DSI)
>>
>>> See above, I think that we can make this work by delegating the actual
>>> detection to the driver (so each compatible can have a different detect
>>> method / code).
>>> So with this we can remove a big part of drivers/misc/q8-hardwaremgr.c, but
>>> not all
>>> of it. We still need board specific code somewhere to deal with things like
>>> picking
>>> the right touchscreen firmware and touchscreen orientation. This is all
>>> somewhat
>>> gsl1680 specific.
>>> I actually have the same problem on x86 where the ACPI description of the
>>> device
>>> basically says: "There is a gsl1680 at this bus at this address" and does
>>> not say
>>> anything about firmware / orientation (again this is simply hardcoded
>>> in the os-image these devices ship with).
>>>
>>> For x86 my plan is to have an array of configs in the driver and select the
>>> right
>>> one based on DMI strings, which is in essence putting board specific info in
>>> the
>>> driver.
>>>
>>> I can imagine mirroring this for ARM, and have an array of configs in the
>>> driver
>>> there too (for cases where cannot simply hardcode everything in dt only) and
>>> have
>>> some board specific code (activated by of_machine_is_compatible()) to select
>>> the
>>> right config.
>> I do believe this can all be done in DTS
>
> Well x86 does not have DTS.
>
Says who? :)
x86 _can_ have DT and there’s no problem using it at all.
For custom boards no one wants to go through the insane ACPI limitations.
FWIW all the x86 mobile parts used it (but are now RIP).
>> and at the moment, what
>> you're describing seem to happen often enough to be worth writing
>> generic code for.
>
> Let me quote some of the auto-code currently in q8-hardwaremgr.c :
>
> /*
> * These accelerometer based heuristics select the best
> * default based on known q8 tablets.
> */
> switch (data->accelerometer.model) {
> case da280:
> if (data->accelerometer.addr == 0x27)
> ; /* No-op */
> else if (data->has_rda599x)
> data->touchscreen_invert_x = 1;
> else
> data->touchscreen_invert_y = 1;
> break;
> case dmard09:
> data->touchscreen_invert_x = 1;
> break;
> case mxc6225:
> data->touchscreen_variant = 1;
> break;
> }
>
> (Non set data->touchscreen_foo are left at 0).
>
> So this would require us to be able to filter (to use your example)
> on if another i2c device is found and on which address it is found,
> that does not even take the rda559x check into account and is
> going to cause interesting ordering issues, how do we know when
> we can actually do the filtering if some of the variables we are
> filtering on are set by other auto-detected paths. Which auto-detect /
> i2c-probe-stop-at-first-match list do we execute first ? Worse
> actually for accelerometer orientation I will likely need to
> set the mount-matrix based on the detected touchscreen ...
>
> The rda559x here is a sdio wifi chip, which is also connected to the
> i2c, and currently is detected through i2c to be able to separately
> identify 2 q8 boards which share the same touchscreen + accelerometer
> combination and who knows what other checks I or other people can
> come up with to differentiate board variants which do not have
> a simple eeprom to uniquely id them.
>
> So as said before, no this cannot be all done in dt without
> adding a turing complete language to dt, and that is just to
> select which touchscreen_variant to use.
>
> Then there also the probem of the combinatorial explosion having
> not only 2 firmware files but also invert-x and invert-y flags causes:
> We have 2 revisions with each 2 different firmware-files (more actually
> but I've reduced the set since some firmwares are compatible) with each
> both the x- and / or y axis as normal or inverted, for a total of:
> 2 (revision) * 2 (firmware-files) * 2 (x-inverted or not) * 2 (y...) = 16
> touchscreen variants, which means dt nodes for touchscreen1 to touchscreen16
> and that is just the silead gsl1680, some of these tablets also have
> elan or zeitec touchscreen controllers.
>
There is a combinatorial explosion, but only a finite number of _known_
board versions.
> Now imagine what happens if a new board comes out which needs a 3th firmware
> file... I hope you can understand this is not a route I want to go.
>
> Another problem is that if a user encounters the need for a new firmware
> variant he can now not easily try this (where as before we had
> module options to separately override firmware-name, the size, etc.
>
> As written in my previous mail, this is all rather gsl1680 specific,
> and esp. being able to override the firmware-name, the size, etc.
> through module options is going to be useful (to ask endusers to test
> stuff without recompiling) on x86 too. So we will likely want to add
> most of the necessary stuff to the silead driver anyways.
>
>> But then, I can't really tell which makes the most sense between
>> source-based and devicetree-based.
>> I prefer doing it in device-tree, since it means that any OEM can have
>> his device supported by only providing DTB, and won't need to provide
>> kernel patches.
>
> If the OEM provides a DTB the OEM can just directly have the right
> parameters in there without relying on any auto-detection, this is
> already supported and the e.g. gsl1680 driver already happily
> works on several tablets where there is not so much hardware
> variance.
>
> Even if the OEM needs to deal with e.g. different touchscreens on
> different board revisions, hopefully the simple auto-detect code will
> be enough, and he does not need e.g. different firmware-name settings
> for otherwise the same touchscreen controller. If that is not the
> case then he the OEM will have to provide a separate static
> (non probing) DTB per variant.
>
>>> 2) miscellaneous extra config on top of figuring out which ICs are
>>> connected,
>>> basically the kind of stuff many vendors simply hard-code in their device
>>> specific os-image. This one is much more difficult to deal with and I think
>>> we need to figure this out on a case by case basis. This will require board
>>> specific code (just like the kernel has tons of DMI string activated board
>>> specific code on x86) and what is the best code for this place to live will
>>> be a case by case thing too.
>>
>> With things like mount-matrix devicetree property, the goal is to have
>> such informations in the DTS.
>
> Right and all the info I'm talking about can already be in the DTS and
> is already specified this way for various existing boards, this is
> obviously how we want things to work, this is the normal case /
> the straight code path.
>
> Now lets get back to your mount-matrix example, the problem here is 2 board
> variants where the same accelerometer is used, but on a newer revision
> of the board it is mounted with a different orientation and otherwise
> almost nothing is changed on the board, certainly not something as
> useful as an id eeprom.
>
> Lets assume that we can however still somehow differ the 2 revisions,
> then try to imagine how many different ways there are to differ
> between 2 board revisions if there is no easy way to do so,
> some crazy examples:
> -The 2nd revision has an external loopback on unused audio out / in
> pins for testing purposes, we could play + record sound and do
> a (rough) waveform match to see if the loopback is present
> -On the 2nd revision a pin from a pin compatible part which
> allows putting it in fully compatible mode, or allow new features
> mode, is now hooked up to a gpio instead of hardwired to compatible
> mode, we could change the device to new features mode and try
> to read/modify/write some register bit on the chip which is only
> writable in this mode
> -Etc.
>
> Now try to design a way to express this in dt and we're back to
> needing a turing complete language (with a library for accessing
> various busses) again.
>
No, not turing complete. This is going to require a little bit more
digging up but I think we can handle this with the method I described.
> Regards,
>
> Hans
Regards
— Pantelis
P.S. Damn too much content to read.--
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 0/3] mtd: s3c2410: add device tree support
From: Boris Brezillon @ 2016-10-27 16:12 UTC (permalink / raw)
To: Sergio Prado
Cc: kgene, krzk, javier, linux, linux, richard, dwmw2,
computersforpeace, linux-arm-kernel, linux-samsung-soc,
linux-kernel, linux-mtd, robh+dt, mark.rutland, devicetree
In-Reply-To: <1477526395-13103-1-git-send-email-sergio.prado@e-labworks.com>
On Wed, 26 Oct 2016 21:59:52 -0200
Sergio Prado <sergio.prado@e-labworks.com> wrote:
> This series adds support for configuring Samsung's s3c2410 and
> compatible flash memory controller via devicetree.
>
> Tested on FriendlyARM mini2440, based on s3c2440 SoC.
>
> Patch 3 depends on patch 1.
Applied.
Thanks,
Boris
>
> Changes since v3:
> - apply the timings in the end of ->setup_data_interface so the
> timings can be changed at runtime by the MTD core.
>
> Changes since v2:
> - conditionally assign chip->setup_data_interface to
> s3c2410_nand_setup_data_interface if booting via device tree
>
> Changes since v1:
> - automate timing selection when booting with a device tree
> - make s3c24XX_nand_devtype_data structs "static const"
> - removing samsung,s3c6400-nand compatible since it is equivalent to
> samsung,s3c2412-nand
>
> Changes since initial version:
> - patch converted to a patch series
> - read timings from nand_sdr_timings when booting with a device tree
> - naming improvements in the device tree binding
> (s/nand/nand-controller/, s/_/-, s/children/child)
> - dropped property samsung,ignore_unset_ecc
> - remove @0 from nand device node
> - checking pdev->dev.of_node instead of using ifdef CONFIG_OF_MTD
> - preventing from parsing device tree properties twice
> - increment the nand controller child node refcount, since we
> maintain a reference to it and its name field
> - using of_device_get_match_data() instead of of_match_device()
> to make the code simpler
> - remove CONFIG_MTD_NAND_S3C2410_HWECC compile option so we can
> select ECC mode using nand-ecc-mode property in the device tree
>
> Sergio Prado (3):
> mtd: s3c2410: make ecc mode configurable via platform data
> dt-bindings: mtd: add DT binding for s3c2410 flash controller
> mtd: s3c2410: parse the device configuration from OF node
>
> .../devicetree/bindings/mtd/samsung-s3c2410.txt | 56 +++++
> arch/arm/mach-s3c24xx/common-smdk.c | 1 +
> arch/arm/mach-s3c24xx/mach-anubis.c | 1 +
> arch/arm/mach-s3c24xx/mach-at2440evb.c | 1 +
> arch/arm/mach-s3c24xx/mach-bast.c | 1 +
> arch/arm/mach-s3c24xx/mach-gta02.c | 1 +
> arch/arm/mach-s3c24xx/mach-jive.c | 1 +
> arch/arm/mach-s3c24xx/mach-mini2440.c | 1 +
> arch/arm/mach-s3c24xx/mach-osiris.c | 1 +
> arch/arm/mach-s3c24xx/mach-qt2410.c | 1 +
> arch/arm/mach-s3c24xx/mach-rx1950.c | 1 +
> arch/arm/mach-s3c24xx/mach-rx3715.c | 1 +
> arch/arm/mach-s3c24xx/mach-vstms.c | 1 +
> arch/arm/mach-s3c64xx/mach-hmt.c | 1 +
> arch/arm/mach-s3c64xx/mach-mini6410.c | 1 +
> arch/arm/mach-s3c64xx/mach-real6410.c | 1 +
> drivers/mtd/nand/Kconfig | 9 -
> drivers/mtd/nand/s3c2410.c | 277 +++++++++++++++------
> include/linux/platform_data/mtd-nand-s3c2410.h | 7 +-
> 19 files changed, 278 insertions(+), 86 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt
>
^ permalink raw reply
* Re: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter
From: Frank Rowand @ 2016-10-27 16:09 UTC (permalink / raw)
To: Pantelis Antoniou, Rob Herring
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <022037BA-60BC-4A85-B51A-04F96E854952@konsulko.com>
On 10/27/16 06:51, Pantelis Antoniou wrote:
> Hi Rob, Frank,
>
>> On Oct 27, 2016, at 15:21 , Rob Herring <robh+dt@kernel.org> wrote:
>>
>> On Tue, Oct 25, 2016 at 3:58 PM, <frowand.list@gmail.com> wrote:
>>> From: Frank Rowand <frank.rowand@am.sony.com>
>>
>> Maybe some should be debug?
>>
>
> Yes, please do not get rid of them completely.
> Leave them at least as debug level so that if there’s a problem
> there’s a way to figure out why something happened.
After patch "Add back an error message, restructured" is applied,
a lot of the messages return, but hopefully keeping readability.
Note that the one message added back covers a number of error
locations.
Are there any additional key messages that you think I missed in
the add back an error message patch?
Keep in mind that many of the debug messages address malformed
dtb, which would be a bug in dtc. It made sense for these to
exist while dtc was being modified, but now that you have
created and tested the dtc changes, I think most of those
debug messages no longer make sense for mainline code.
>>> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
>>> ---
>>> drivers/of/resolver.c | 28 ----------------------------
>>> 1 file changed, 28 deletions(-)
>>>
>>> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
>>> index 4ff0220d7aa2..93a7ca0bf98c 100644
>>> --- a/drivers/of/resolver.c
>>> +++ b/drivers/of/resolver.c
>>> @@ -116,8 +116,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
>>>
>>> propval = kmalloc(rprop->length, GFP_KERNEL);
>>> if (!propval) {
>>> - pr_err("%s: Could not copy value of '%s'\n",
>>> - __func__, rprop->name);
>>> return -ENOMEM;
>>> }
>>
>> I would remove the brackets in this patch rather than separately.
>>
>>> memcpy(propval, rprop->value, rprop->length);
>
>
> Regards
>
> — Pantelis
>
>
^ permalink raw reply
* Re: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter
From: Frank Rowand @ 2016-10-27 16:04 UTC (permalink / raw)
To: Rob Herring
Cc: Pantelis Antoniou, Pantelis Antoniou, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq+TWZw+VcbYZp_xmFR-yitY8bn+gqe1Q2N42UFZAYsZKg@mail.gmail.com>
On 10/27/16 05:21, Rob Herring wrote:
> On Tue, Oct 25, 2016 at 3:58 PM, <frowand.list@gmail.com> wrote:
>> From: Frank Rowand <frank.rowand@am.sony.com>
>
> Maybe some should be debug?
>
>> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
>> ---
>> drivers/of/resolver.c | 28 ----------------------------
>> 1 file changed, 28 deletions(-)
>>
>> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
>> index 4ff0220d7aa2..93a7ca0bf98c 100644
>> --- a/drivers/of/resolver.c
>> +++ b/drivers/of/resolver.c
>> @@ -116,8 +116,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
>>
>> propval = kmalloc(rprop->length, GFP_KERNEL);
>> if (!propval) {
>> - pr_err("%s: Could not copy value of '%s'\n",
>> - __func__, rprop->name);
>> return -ENOMEM;
>> }
>
> I would remove the brackets in this patch rather than separately.
>
>> memcpy(propval, rprop->value, rprop->length);
> .
>
OK, I will collapse the "remove braces" patch into this patch.
^ 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