Re: [PATCH 2/2] Input: atmel_mxt_ts - Add keycodes array example
On 08/15/2014 10:08 AM, Nick Dyer wrote:
On 15/08/14 13:01, Javier Martinez Canillas wrote:
By passing all these keycodes the touchpad worked as expected for me and the
driver did the same than the Chrome OS driver that has these keycodes hardcoded
when is_tp is true.
at the protocol guide for T19.
I don't have access to proper documentation and I wouldn't expect people to have
access to non-public docs in order to use a Device Tree binding.
That's why I wanted to document an example, so using this property could be
easier for others and they shouldn't have to look at the driver in order to
figure it out (and getting it wrong as you said :) )
So it would be great if you could provide an example on how this is supposed to
be used.
Any comments on this? I would really appreciate if you can expand on how
this DT property is supposed to be used so I can re-spin the atmel support
patch for Peach boards.
The below patch improves the documentation for the gpio-property.
That patch makes sense, and is a nice description,
Acked-by: Stephen Warren swar...@nvidia.com
diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
Example:
touch@4b {
Perhaps it makes sense to add a linux,gpio-keymap property into the
example too though; IIRC there was an earlier patch to the docs that did
this?
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] ARM: dts: Create fragment for tps65090 PMU
On 08/12/2014 10:44 AM, Javier Martinez Canillas wrote: The tps65090 is a Power Management Unit (PMU) used in several boards so the same information is described on different DTS. It is better to create a .dtsi fragment that can be included. To be honest, I'm not sure that this file is useful. The entire content of this file (with the exception of the compatible value and possibly the charger sub-node) needs to be repeated in any file that includes it, in order to add properties into all the nodes. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] ARM: dts: Add tps65090 FETs constraints
On 08/12/2014 10:44 AM, Javier Martinez Canillas wrote:
The tps65090 PMU data manual [0] has a table that list the
Recommended operating conditions for each regulator. Add
the information about the FET constraints to its dtsi file.
[0]: http://www.ti.com/lit/ds/symlink/tps65090.pdf
I'm worried that this file represents the limits of the PMIC itself,
whereas the DT should be representing the limits of the circuits that
the various PMIC regulators are attached to on the board.
For example:
diff --git a/arch/arm/boot/dts/tps65090.dtsi b/arch/arm/boot/dts/tps65090.dtsi
tps65090_fet3: fet3 {
+ regulator-min-microvolt = 300;
+ regulator-max-microvolt = 550;
};
I guess that on some boards, this output rail might be attached to
devices that must run at 3.3V exactly, and on other boards it might be
attached to devices that must run at 5V exactly. The DT for those two
boards should each have regulator-{min,max}-microvolt set to the same
value, which describes the board requirements.
It feels dangerous/misleading to define the PMIC range by default. It
might lead people to think that since the property already has a defined
value, they don't need to think about what the correct value for their
board is, and hence not change the value in their board file.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: convert all mov.* pc, reg to bx reg for ARMv6+ (part1)
On 07/01/2014 10:19 AM, Russell King wrote: ARMv6 and greater introduced a new instruction (bx) which can be used to return from function calls. Recent CPUs perform better when the bx lr instruction is used rather than the mov pc, lr instruction, and this sequence is strongly recommended to be used by the ARM architecture manual (section A.4.1.1). We provide a new macro ret with all its variants for the condition code which will resolve to the appropriate instruction. Rather than doing this piecemeal, and miss some instances, change all the mov pc instances to use the new macro, with the exception of the movs instruction and the kprobes code. This allows us to detect the mov pc, lr case and fix it up - and also gives us the possibility of deploying this for other registers depending on the CPU selection. Tested-by: Stephen Warren swar...@nvidia.com (On an NVIDIA Tegra Jetson TK1 board, both CPU hotplug and system sleep were tested, which are the use-cases that actually use the edited assembly files) -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: Add mask-tpm-reset to the device tree
On 06/27/2014 06:17 AM, Tomasz Figa wrote:
Hi Doug,
On 26.06.2014 17:25, Doug Anderson wrote:
Tomasz,
On Thu, Jun 26, 2014 at 2:52 AM, Tomasz Figa t.f...@samsung.com wrote:
Hi Vikas, Doug,
On 26.06.2014 11:15, Vikas Sajjan wrote:
From: Doug Anderson diand...@chromium.org
The mask-tpm-reset GPIO is used by the kernel to prevent the TPM from
being reset across sleep/wake. If we don't set it to anything then
the TPM will be reset. U-Boot will detect this as invalid
and will reset the system on resume time. This GPIO can always be low
and not hurt anything. It will get pulled back high again during a
normal warm reset when it will default back to an input.
To properly preserve the TPM state across suspend/resume and to make
the chrome U-Boot happy, properly set the GPIO to mask the
reset to the TPM.
Signed-off-by: Doug Anderson diand...@chromium.org
Signed-off-by: Vikas Sajjan vikas.saj...@samsung.com
---
arch/arm/boot/dts/exynos5420-peach-pit.dts | 20
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts
b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 7649982..8fd990a 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -87,6 +87,18 @@
pinctrl-0 = usb301_vbus_en;
enable-active-high;
};
+
+ /* We need GPX0_6 to be low at sleep time; just keep it low always */
+ mask_tpm_reset_regulator: mask-tpm-reset-regulator {
+ compatible = regulator-fixed;
+ regulator-name = mask-tpm-reset ;
+ gpio = gpx0 6 0;
+ enable-active-low;
+ regulator-boot-on;
+ regulator-always-on;
+ pinctrl-names = default;
+ pinctrl-0 = mask_tpm_reset;
+ };
I don't think this pin is supposed to be a real regulator. If I'm right,
you should just add a hog for it, if you don't have a proper driver to
handle it.
Yes, I agree that it shouldn't really be a regulator, but there's not
a whole lot of choice. The pin needs to actually be driven low, not
just pulled low. Without your proposed patch (pinctrl: samsung: Allow
pin value to be initialized using pinfunc) I don't think it's possible
to actually drive a pin low with a hog. I could be wrong, though.
Surely there's a driver (or could be a driver) for the TPM chip, and
that driver should have a reset-mask-gpios property, so the driver can
call gpio_get() and gpio_set_output() on the GPIO?
Faking this out via a not-really-a-regulator or pinctrl hogs seems like
an abuse of those features to me.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: Add mask-tpm-reset to the device tree
On 06/27/2014 10:45 AM, Doug Anderson wrote: Stephen, On Fri, Jun 27, 2014 at 9:10 AM, Stephen Warren swar...@wwwdotorg.org wrote: Surely there's a driver (or could be a driver) for the TPM chip, and that driver should have a reset-mask-gpios property, so the driver can call gpio_get() and gpio_set_output() on the GPIO? Faking this out via a not-really-a-regulator or pinctrl hogs seems like an abuse of those features to me. This totally doesn't belong in the TPM chip driver. This is an unabashed HACK at the board level. Without a hog we need a board driver for this. To be more specific: * The TPM needs to be reset when the full system gets reset. This unlocks the TPM and allows certain types of updates by the firmware. The firmware then locks the TPM before jumping to the kernel. * The TPM is hooked up to the reset out line of the CPU so that when the system does a warm reset it will reset the TPM. * Unfortunately the CPU asserts the reset out line when it's sleeping (because, of course, sleep is a reset). This would allow the kernel to unlock the TPM which it's not supposed to be able to do. To me, this does sound precisely like something that should be in the TPM driver. I guess an overall board driver would be reasonable too, since the issue probably doesn't to all boards with the TPM. This kind of thing is *exactly* the kind of thing that's been discussed in the past re: doing it in pinmux hogs, or GPIO initialization tables that aren't specific to a driver, and has been rejected. I guess if people want to change the decisions that's fine, but doing so will open up the door to all the previously rejected similar use-cases. I'm not sure how much I care either way, but we really should be consistent instead of flip-flopping on this kind of issue. As an aside, why do we even care about this? The kernel clearly can unlock the TPM simply by failing to set the GPIO up correctly, so at best this is security through obscurity. If someone actively wanted to do something bad to the TPM, they'd simply comment out this piece of code in the kernel. At least that's true with usptream kernels which aren't validated at all during boot. For a downstream signed kernel, perhaps this patch makes sense (although I haven't thought about all the security angles), but then it'd make sense to keep this patch downstream. * To solve the problem, it's up to the kernel to mask out the reset line before going to sleep. Then it's up to the read only firmware to validate that the kernel properly masked the reset before resuming from sleep. If the firmware finds that the kernel cheated and didn't mask the reset then it will not resume to the kernel and will instead restart the system. The above is not beautiful in the least sense. Getting suspend/resume working happened very late in the exynos5250-snow project and the above workaround was the best that we could come up with without slipping schedules. It also had the side effect of being less expensive than other solutions. Given that the solution was proven out for exynos5250-snow, it was kept in place for similar products. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: Add mask-tpm-reset to the device tree
On 06/27/2014 12:30 PM, Doug Anderson wrote: Stephen, On Fri, Jun 27, 2014 at 11:20 AM, Stephen Warren swar...@wwwdotorg.org wrote: On 06/27/2014 10:45 AM, Doug Anderson wrote: Stephen, On Fri, Jun 27, 2014 at 9:10 AM, Stephen Warren swar...@wwwdotorg.org wrote: Surely there's a driver (or could be a driver) for the TPM chip, and that driver should have a reset-mask-gpios property, so the driver can call gpio_get() and gpio_set_output() on the GPIO? Faking this out via a not-really-a-regulator or pinctrl hogs seems like an abuse of those features to me. ... As an aside, why do we even care about this? The kernel clearly can unlock the TPM simply by failing to set the GPIO up correctly, so at best this is security through obscurity. If someone actively wanted to do something bad to the TPM, they'd simply comment out this piece of code in the kernel. At least that's true with usptream kernels which aren't validated at all during boot. For a downstream signed kernel, perhaps this patch makes sense (although I haven't thought about all the security angles), but then it'd make sense to keep this patch downstream. Check out the bullet point about the firmware checking for kernel cheats. At resume time the chip actually re-loads read only firmware out of SPI flash (no choice about this). This read only firmware can check the state of the mask pin (which is preserved across sleep wake) to see if the kernel cheated. Ah, that covers the security issues then. I'd argue that the RO firmware should be setting up GPIOs like this myself (and the pinmux, from a nice big table), and the kernel simply not touch it all since it has no direct use for the pin. But I suppose if the firmware is already baked and read only, that's not possible. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/7] mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources
On 06/17/2014 02:53 AM, Paul Bolle wrote: Doug, On Fri, 2014-06-13 at 08:22 -0700, Doug Anderson wrote: On Fri, Jun 13, 2014 at 1:08 AM, Paul Bolle pebo...@tiscali.nl wrote: On Wed, 2014-06-11 at 08:11 -0700, Doug Anderson wrote: This is a config option on the ChromeOS EC https://chromium.googlesource.com/chromiumos/platform/ec. Doing a grep there: board/samus/board.h:#define CONFIG_CHARGER_PROFILE_OVERRIDE common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE driver/battery/samus.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE driver/battery/samus.c:#endif /* CONFIG_CHARGER_PROFILE_OVERRIDE */ include/config.h:#undef CONFIG_CHARGER_PROFILE_OVERRIDE include/ec_commands.h: /* Range for CONFIG_CHARGER_PROFILE_OVERRIDE params */ test/test_config.h:#define CONFIG_CHARGER_PROFILE_OVERRIDE I see. So this is not a Kconfig macro but a general macro with a CONFIG_ prefix. There are quite a bit of those in the tree already, but still, would another prefix also do? Given that it's an entirely separate project and this is a valid CONFIG option in that project, it seems a lot to ask them not to use the CONFIG_ prefix. Also: the part you are objecting to is only a comment, right? So all the hits you quoted above are actually from code that's never going to be included in the kernel tree, right? If so, then yes, we're only discussing a single comment. We could certainly add extra wording in the comment to make it obvious that this is a CONFIG option for the EC and not the kernel. Would that be enough? ...or are you trying to use some scripts to automatically process files to look for CONFIG options? Yes, I'm using a script to check for Kconfig macros, among other things. It doesn't care about comments (because every now and then mistakes are made in comments too, and some of those can get surprisingly confusing). Anyhow, the CONFIG_ prefix used in the kernel tree is quite generic, but we're stuck with it. Would it be bothersome to drop it in that comment? Mentioning a preprocessor macro from a separate project is a bit confusing to begin with. How is one supposed to know that this is a reference to something out of tree? So, in summary, while we're apparently only discussing a single comment, I would appreciate it if it could be reworded, preferably by dropping that the CONFIG_ prefix. But other people might care very little, as they don't share this particular pet peeve. Can't your tool maintain a whitelist or ignore list? There are many cases where the kernel can pull in headers/data from other projects (Firmware interfaces to an arbitrarily large set of HW, Device trees, IO/network protocools, perhaps more). It feels quite unreasonable for the kernel to decide that it exclusively owns the CONFIG_* namespace even in comments, and that every other project it interacts with must not use that namespace. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 7/7] ARM: tegra: Add the EC i2c tunnel to tegra124-venice2
On 04/30/2014 11:44 AM, Doug Anderson wrote: This adds the EC i2c tunnel (and devices under it) to the tegra124-venice2 device tree. I've applied this to Tegra's for-3.17/dt branch. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ASoC: free jack GPIOs before the sound card is freed
From: Stephen Warren swar...@nvidia.com
This is the same change as commit fb6b8e71448a ASoC: tegra: free jack
GPIOs before the sound card is freed, but applied to all other ASoC
machine drivers where code inspection indicates the same problem exists.
That commit's description is:
==
snd_soc_jack_add_gpios() schedules a work queue item to poll the GPIO to
generate an initial jack status report. If sound card initialization
fails, that work item needs to be cancelled, so it doesn't run after the
card has been freed. Specifically, freeing the card calls
snd_jack_dev_free() which calls snd_jack_dev_disconnect() which sets
jack-input_dev = NULL, and input_dev is used by snd_jack_report(), which
is called from the work queue item.
snd_soc_jack_free_gpios() cancels the work item. The Tegra ASoC machine
drivers do call this function in the platform driver remove() callback.
However, this happens after the sound card is freed, at least when the
card is freed due to errors late during snd_soc_instantiate_card(). This
leaves a window where the work item can execute after the card is freed.
In next-20140522, sound card initialization does fail for unrelated
reasons, and hits the problem described above.
To solve this, fix the Tegra ASoC machine drivers to clean up the Jack
GPIOs during the snd_soc_card's .remove() callback, which is executed
before the overall card object is freed. also, guard the cleanup call
based on whether we actually setup up the GPIOs in the first place.
Ideally, we'd do the cleanup in a struct snd_soc_dai_link .fini/remove
function to match where the GPIOs get set up. However, there is no such
callback.
==
Note that I have not even compile-tested this in most cases, since most
of the drivers rely on specific mach-* support I don't have enabled, and
don't support COMPILE_TEST. Testing by the relevant board maintainers
would be useful.
Cc: Peter Ujfalusi peter.ujfal...@ti.com (maintainer:OMAP AUDIO SUPPORT)
Cc: Jarkko Nikula jarkko.nik...@bitmer.com (maintainer:OMAP AUDIO SUPPORT)
Cc: Philipp Zabel philipp.za...@gmail.com (maintainer:ARM/H4700 (HP IPA...)
Cc: Paul Parsons lost.dista...@yahoo.com (maintainer:ARM/H4700 (HP IPA...)
Cc: Eric Miao eric.y.m...@gmail.com (maintainer:PXA2xx/PXA3xx SUP...)
Cc: Russell King li...@arm.linux.org.uk (maintainer:PXA2xx/PXA3xx SUP...)
Cc: Haojian Zhuang haojian.zhu...@gmail.com (maintainer:PXA2xx/PXA3xx SUP...)
Cc: Ben Dooks ben-li...@fluff.org (maintainer:ARM/SAMSUNG ARM A...)
Cc: Kukjin Kim kgene@samsung.com (maintainer:ARM/SAMSUNG ARM A...)
Cc: Sangbeom Kim sbki...@samsung.com (supporter:SAMSUNG AUDIO (AS...)
Cc: linux-o...@vger.kernel.org (open list:OMAP AUDIO SUPPORT)
Cc: linux-samsung-soc@vger.kernel.org (moderated list:ARM/SAMSUNG ARM A...)
Signed-off-by: Stephen Warren swar...@nvidia.com
---
Mark, do you need this patch split up by architecture for topic branches?
I assume not if you've already sent your pull requests. This applies fine
on top of tag asoc-v3.16.
---
sound/soc/omap/ams-delta.c | 14 ++
sound/soc/omap/omap-twl4030.c | 28 ++--
sound/soc/omap/rx51.c | 18 +-
sound/soc/pxa/hx4700.c | 9 -
sound/soc/samsung/h1940_uda1380.c | 11 +--
sound/soc/samsung/rx1950_uda1380.c | 12 ++--
sound/soc/samsung/smartq_wm8987.c | 11 +--
7 files changed, 69 insertions(+), 34 deletions(-)
diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c
index bb243c663e6b..1f41951d8b7f 100644
--- a/sound/soc/omap/ams-delta.c
+++ b/sound/soc/omap/ams-delta.c
@@ -527,6 +527,15 @@ static int ams_delta_cx20442_init(struct
snd_soc_pcm_runtime *rtd)
return 0;
}
+static int ams_delta_card_remove(struct snd_soc_pcm_runtime *rtd)
+{
+ snd_soc_jack_free_gpios(ams_delta_hook_switch,
+ ARRAY_SIZE(ams_delta_hook_switch_gpios),
+ ams_delta_hook_switch_gpios);
+
+ return 0;
+}
+
/* DAI glue - connects codec -- CPU */
static struct snd_soc_dai_link ams_delta_dai_link = {
.name = CX20442,
@@ -543,6 +552,7 @@ static struct snd_soc_dai_link ams_delta_dai_link = {
static struct snd_soc_card ams_delta_audio_card = {
.name = AMS_DELTA,
.owner = THIS_MODULE,
+ .remove = ams_delta_card_remove,
.dai_link = ams_delta_dai_link,
.num_links = 1,
@@ -579,10 +589,6 @@ static int ams_delta_remove(struct platform_device *pdev)
dev_warn(pdev-dev,
failed to unregister V253 line discipline\n);
- snd_soc_jack_free_gpios(ams_delta_hook_switch,
- ARRAY_SIZE(ams_delta_hook_switch_gpios),
- ams_delta_hook_switch_gpios);
-
snd_soc_unregister_card(card);
card-dev = NULL;
return 0;
diff --git a/sound/soc/omap/omap-twl4030.c b/sound/soc/omap/omap-twl4030.c
index 64141db311b2..b4e282871658 100644
--- a/sound/soc/omap
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On 05/23/2014 02:36 PM, Thierry Reding wrote: From: Thierry Reding tred...@nvidia.com This commit introduces a generic device tree binding for IOMMU devices. Only a very minimal subset is described here, but it is enough to cover the requirements of both the Exynos System MMU and Tegra SMMU as discussed here: https://lkml.org/lkml/2014/4/27/346 Signed-off-by: Thierry Reding tred...@nvidia.com --- Apologies for the noise, but apparently I mistyped one of the email addresses, should be fixed now. Changes in v2: - add notes about dma-ranges property (drop note from commit message) - document priorities of iommus property vs. dma-ranges property - drop #iommu-cells in favour of #address-cells and #size-cells I think this is a mistake. address-cells/size-cells are for transactions flowing down the bus (from the CPU to date). Describing a connection from a device to an IOMMU is something completely different, and should therefore simply use an iommu-cells property to describe any necessary information. If we start re-using properties for different things in different contexts, how is anyone going to know what they mean, and how will conflicts be resolved. For example, what if there's a single HW module that both acts as a regular register bus with children (where address-cells/size-cells defines how transactions reach the children from the parent), and is also an IOMMU (where according to this binding proposal, address-cells/size-cells represent some aspect of the IOMMU feature). Using different properties for different things is the only sane way to keep different concepts separate. Another alternative would be to represent the single HW module as separate nodes in DT, but I think that will only make our lives harder, and where I've done that in the past, I've regretted it. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] ARM: tegra: Add the EC i2c tunnel to tegra124-venice2
On 04/17/2014 11:59 AM, Doug Anderson wrote: This adds the EC i2c tunnel (and devices under it) to the tegra124-venice2 device tree. Did the MFD patches at the start of this series get applied yet? I was hoping to apply this one patch to the Tegra tree for 3.16, and that needs to happen by tomorrow morning at the latest since -rc6 is looming fast. However, I'm holding off until the rest of the series is applied to the MFD tree so I can be sure the DT binding is accepted first... -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/6] usb: host: ehci-tegra: Use devm_ioremap_resource instead of devm_ioremap
On 05/10/2014 06:00 AM, Vivek Gautam wrote: Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. This patch on its own works OK on Tegra. However, if a similar patch were to be applied to the Tegra PHY driver, then I expect that would break USB on Tegra. The reason is that the Tegra USB controller and PHY registers are interleaved a bit randomly within the same address range, and rather than call out which individual addresses are relevant to the controller and the PHY, the DT for both devices just specifies the same whole range, and the drivers only touch the appropriate registers within the range. Perhaps we should have described that as an MFD rather than separate DT nodes and devices, but that's not what we ended up with. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] i2c: ChromeOS EC tunnel driver
On 04/30/2014 11:44 AM, Doug Anderson wrote: On ARM Chromebooks we have a few devices that are accessed by both the AP (the main Application Processor) and the EC (the Embedded Controller). These are: * The battery (sbs-battery). * The power management unit tps65090. ... On the Samsung ARM Chromebook 2 the scheme is changed yet again, now: * The AP/EC comms are now using SPI for faster speeds. * The EC's i2c bus is exposed to the AP through a full i2c tunnel. The upstream tegra124-venice2 uses the same scheme as the Samsung ARM Chromebook 2, though it has a different set of components on the other side of the bus. This driver supports the scheme used by the Samsung ARM Chromebook 2. Future patches to this driver could add support for the battery tunnel on the HP Chromebook 11 (and perhaps could even be used to access tps65090 on the HP Chromebook 11 instead of using a special driver, but I haven't researched that enough). The binding looks reasonable to me, so that part, Acked-by: Stephen Warren swar...@nvidia.com -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 7/7] ARM: tegra: Add the EC i2c tunnel to tegra124-venice2
On 04/30/2014 11:44 AM, Doug Anderson wrote: This adds the EC i2c tunnel (and devices under it) to the tegra124-venice2 device tree. I'll happily take this into the Tegra tree once the patch containing the binding it uses is applied. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 51/97] ARM: l2c: remove platforms/SoCs setting early BRESP
On 04/28/2014 06:02 PM, Simon Horman wrote: On Mon, Apr 28, 2014 at 08:30:32PM +0100, Russell King wrote: Since we now automatically enable early BRESP in core L2C-310 code when we detect a Cortex-A9, we don't need platforms/SoCs to set this bit explicitly. Instead, they should seek to preserve the value of bit 30 in the auxiliary control register. Acked-by: Tony Lindgren t...@atomide.com Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk I would prefer if this patch was broken out into individual patches for each board or SoC file and that they were then picked up by their respective platform maintainers. Likewise for patch 66/97. Although it is only for shmobile I would prefer it broken out. There are far too many dependencies in this series to break out the board file patches to be merged separately; it'd take either a whole bunch of kernel releases to merge it all that way, or a twisty maze of tiny topic branches cross-merged all over the place. Neither option is realistic. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 11/31] documentation: iommu: add binding document of Exynos System MMU
On 04/28/2014 05:18 AM, Thierry Reding wrote: On Mon, Apr 28, 2014 at 12:56:03PM +0200, Arnd Bergmann wrote: ... A lot of drivers probably only support one master, so they can just set #iommu-cells=0, others might require IDs that do not fit into one cell. You mean #iommu-cells = 1 for devices that only require one master? There still has to be one cell to specify which master. Unless perhaps if they can be arbitrarily assigned. I guess even if there's a fixed mapping that applies to one SoC generation, it might be good to still employ a specifier and have the mapping in DT for flexibility. #iommu-cells doesn't include the phandle, so if you want the client references to be: property = iommu; then that's #iommu-cells=0, whereas: property = iommu N; is #iommu-cells=1. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] base: power: Add generic OF-based power domain look-up
On 04/23/2014 10:46 AM, Tomasz Figa wrote: This patch introduces generic code to perform power domain look-up using device tree and automatically bind devices to their power domains. Generic device tree binding is introduced to specify power domains of devices in their device tree nodes. Backwards compatibility with legacy Samsung-specific power domain bindings is provided, but for now the new code is not compiled when CONFIG_ARCH_EXYNOS is selected to avoid collision with legacy code. This will change as soon as Exynos power domain code gets converted to use the generic framework in further patch. diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt +==Power domain consumers== + +Required properties: + - power-domain : A phandle and power domain specifier as defined by bindings + of power controller specified by phandle. It seems quite likely that a single logical device could have components in multiple power domains. Consider an HDMI controller with different power domains for the HDMI core, CEC communication, DDC/I2C communication, and the I/O pads, with no clear separation between those two components of the module (no separate register spaces, but the bits/registers are interleaved all together). As such, I think that rather than a power-domain property, we need a pair of power-domains, and power-domain-names properties, and preferably with mandatory usage of name-based lookups, rather than allowing a random mix of name-based and index-based lookups like we have with some existing resource bindings. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/16] Another 16 L2C patches
On 04/28/2014 11:12 AM, Stephen Warren wrote: On 04/28/2014 10:56 AM, Russell King - ARM Linux wrote: So, in response to Matt Porter's complaint about breaking prima2, here's another 16 patches which changes the way the L2 cache is initialised on many platforms. This series moves towards a situation where the generic code initialises the L2 cache itself, with as little help as possible from board specific code. A number of platforms are left alone because they're more complex - these should still eventually be converted. At some point in the near future, I will see about sorting out their ordering wrt the previous patch set. For the time being, they apply on top of the existing l2c changes. Are the existing l2c changes in next-20140428? If not, is there a git branch I can pull to test the whole thing, rather than tracking down and applying the existing l2c changes first? I guess they must be in linux-next, since this series applies cleanly on top of it. So, patches 2/16 (ARM: l2c: add platform independent core L2 cache initialisation) and 7/16 (ARM: l2c: convert tegra to generic l2c initialisation), Tested-by: Stephen Warren swar...@nvidia.com (On an NVIDIA Tegra20 Seaboard/Springbank board, on top of next-20140428) I do see one error in dmesg during boot, but it doesn't appear to negatively affect operation in brief testing, and is present in linux-next without this series anyway. Is this message a problem? [0.00] L2C: platform modifies aux control register: 0x0208 - 0x3e480001 [0.00] L2C: DT/platform modifies aux control register: 0x0208 - 0x3e480001 [0.00] L2C-310 errata 727915 769419 enabled [0.00] L2C-310 enabling early BRESP for Cortex-A9 [0.00] L2C-310: enabling full line of zeros but not enabled in Cortex-A9 ^^ this is logged at error level [0.00] L2C-310 ID prefetch enabled, offset 1 lines [0.00] L2C-310 dynamic clock gating disabled, standby mode disabled [0.00] L2C-310 cache controller enabled, 8 ways, 1024 kB [0.00] L2C-310: CACHE_ID 0x41c4, AUX_CTRL 0x7e480001 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 51/97] ARM: l2c: remove platforms/SoCs setting early BRESP
On 04/28/2014 01:30 PM, Russell King wrote: Since we now automatically enable early BRESP in core L2C-310 code when we detect a Cortex-A9, we don't need platforms/SoCs to set this bit explicitly. Instead, they should seek to preserve the value of bit 30 in the auxiliary control register. Acked-by: Stephen Warren swar...@nvidia.com -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 49/97] ARM: l2c: fix register naming
On 04/28/2014 01:30 PM, Russell King wrote: We have a mixture of different devices with different register layouts, but we group all the bits together in an opaque mess. Split them out into those which are L2C-310 specific and ones which refer to earlier devices. Provide full auxiliary control register definitions. Acked-by: Stephen Warren swar...@nvidia.com -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] Add cros_ec changes for newer boards
On 04/23/2014 06:32 AM, Lee Jones wrote: On Tue, 22 Apr 2014, Doug Anderson wrote: This series adds the most critical cros_ec changes for newer boards using cros_ec. Specifically: * Fixes timing/locking issues with the previously upstreamed (but never used upstream) cros_ec_spi driver. * Updates the cros_ec header file to the latest version which allows us to use newer EC features like i2c tunneling. * Adds an i2c tunnel driver to allow communication to the EC's i2c devices. This _doesn't_ get the EC driver fully up to speed with what's in the current Chromium OS trees. There are a whole slew of cleanup patches there, an addition of an LPC transport mode, and exports of functions to userspace. Once these patches land and we have functionality we can continue to pick more cleanup patches. ... Need to wait for the ARM, DT and I2C guys to review, at which point I'll be happy to take in and supply a branch for them to pull from if required. If there are no _true_ dependencies and the MFD changes can be added independently without fear of build breakages, let me know and I'll apply them separately. I believe there aren't direct dependencies between the patches. So, the MFD patches can be applied to the MFD tree and the DT patch applied to the Tegra tree. I'm simply waiting for the MFD patches to be applied before applying the DT patch so that I know the DT binding definition is fully accepted before applying a patch that uses it. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 6/7] i2c: ChromeOS EC tunnel driver
On 04/17/2014 12:36 PM, Doug Anderson wrote: On ARM Chromebooks we have a few devices that are accessed by both the AP (the main Application Processor) and the EC (the Embedded Controller). These are: * The battery (sbs-battery). * The power management unit tps65090. ... On the Samsung ARM Chromebook 2 the scheme is changed yet again, now: * The AP/EC comms are now using SPI for faster speeds. * The EC's i2c bus is exposed to the AP through a full i2c tunnel. ... This driver supports the scheme used by the Samsung ARM Chromebook 2. Future patches to this driver could add support for the battery tunnel on the HP Chromebook 11 (and perhaps could even be used to access tps65090 on the HP Chromebook 11 instead of using a special driver, but I haven't researched that enough). diff --git a/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt +I2C bus that tunnels through the ChromeOS EC (cros-ec) +== +On some ChromeOS board designs we've got a connection to the EC (embedded +controller) but no direct connection to some devices on the other side of +the EC (like a battery and PMIC). To get access to those devices we need +to tunnel our i2c commands through the EC. + +The node for this device should be under a cros-ec node like google,cros-ec-spi +or google,cros-ec-i2c. + + +Required properties: +- compatible: google,cros-ec-i2c-tunnel +- google,remote-bus: The EC bus we'd like to talk to. It's probably worth mentioning here that the node represents a single I2C bus, and hence is expected to contain child nodes representing I2C devices. Perhaps: Optional child nodes: - One node per I2C device connected to the tunnelled I2C bus. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] ARM: tegra: Add the EC i2c tunnel to tegra124-venice2
On 04/17/2014 11:59 AM, Doug Anderson wrote:
This adds the EC i2c tunnel (and devices under it) to the
tegra124-venice2 device tree.
The series,
Tested-by: Stephen Warren swar...@nvidia.com
I can apply this one patch once the other patches in the series are
acked or applied (in order to make sure the DT binding is acceptable to
others).
I guess I'll send separate patches for tegra_defconfig and
multi_v7_defconfig to add the required options once I've applied this,
unless you beat me to it.
diff --git a/arch/arm/boot/dts/tegra124-venice2.dts
b/arch/arm/boot/dts/tegra124-venice2.dts
aliases {
+ i2c20 = /spi@0,7000d400/cros-ec@0/i2c-tunnel;
Is that needed? I'd prefer not to add it unless there's a specific
reason. I don't think I2C buses need specific names, do they?
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] ARM: tegra: Add the EC i2c tunnel to tegra124-venice2
On 04/21/2014 01:35 PM, Doug Anderson wrote:
Stephen,
On Mon, Apr 21, 2014 at 11:18 AM, Stephen Warren swar...@wwwdotorg.org
wrote:
On 04/17/2014 11:59 AM, Doug Anderson wrote:
This adds the EC i2c tunnel (and devices under it) to the
tegra124-venice2 device tree.
diff --git a/arch/arm/boot/dts/tegra124-venice2.dts
b/arch/arm/boot/dts/tegra124-venice2.dts
aliases {
+ i2c20 = /spi@0,7000d400/cros-ec@0/i2c-tunnel;
Is that needed? I'd prefer not to add it unless there's a specific
reason. I don't think I2C buses need specific names, do they?
It is not strictly needed, but from a usability standpoint it is
terribly helpful. It serves to make it obvious to someone looking at
the device that it's _not_ an i2c bus associated with the main SoC.
If you don't include a number I believe that the i2c core will pick
the first available number.
It seems worth it to save a few people a few hours of head scratching.
...but this is your dts and if you think it's a terrible idea then
I'll remove it. It looks to be less critical on tegra than it is on
exynos (which has ~9 i2c busses, they are numbered in the user manual,
and if you have one set to disable in the dts then the tunnel will
end up getting a very confusing number).
My opinion is that the in-kernel I2C bus numbering is an entirely
unrelated numbering space to the HW controller numbering space precisely
because of issues like that. DT aliases are more useful for user-visible
port numbering (e.g. HDMI 0, 1 connectors on a case) than purely
internal details like this. So, I would leave it out.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation
On 03/13/2014 11:40 AM, Tim Harvey wrote: When using interrupt-maps, the size of a map entry is #address-cells + #interrupt-cells for the parent interrupt controller. For the ARM GIC address-cells should be 0 as this is not used. This patch fixes the example by correctly specifying #address-cells = 0. If the #address-cells property is required (well, or even optional...) in the node, shouldn't it be included in the list of required/optional properties above, and not solely included in the example? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/6] PCI: tegra: use new OF interrupt mapping when possible
On 03/05/2014 06:25 AM, Lucas Stach wrote: This is the recommended method of doing the IRQ mapping. For old devicetrees we fall back to the previous practice. Tested-by: Stephen Warren swar...@nvidia.com I tested both with and without patch 1/6, and the PCIe-based NIC on Beaver worked fine either way. Without patch 1/6, I do see: pci :00:01.0: of_irq_parse_pci() failed with rc=-22 ... but that seems reasonable given that the DT that of_irq_parse_pci() parses is missing, and did correctly trigger the fallback path, so everything still worked. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/6] ARM: dts: tegra: add PCIe interrupt mapping properties
On 03/05/2014 06:25 AM, Lucas Stach wrote: Those are defined by the common PCI binding. I've applied this to Tegra's for-3.15/dt branch. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/6] ARM: dts: tegra: add PCIe interrupt mapping properties
On 03/05/2014 06:25 AM, Lucas Stach wrote: Those are defined by the common PCI binding. It sounds like there's no dependency between pathces 1/6 and 2/6, so I should apply 1/6 to the Tegra tree, and Bjorn apply 2/6 to the PCI tree? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] ARM: dts: tegra: add PCIe interrupt mapping properties
On 02/28/2014 10:28 AM, Lucas Stach wrote: Those are defined by the common PCI binding. I have no reason to object to the two Tegra patches, but I'll wait for Thierry to take a closer look. I expect once he does, I would apply patch 1/7 through the Tegra tree, and Bjorn would take patch 2/7 through the PCI tree. Does that make sense? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv1] usb: dwc2: Combine the dwc2 and s3c_hsotg into a single USB DRD driver.
On 02/04/2014 02:45 PM, dingu...@altera.com wrote: From: Dinh Nguyen dingu...@altera.com This means that the driver can be in host or peripheral mode when the appropriate connector is used. When an A-cable is plugged in, the driver behaves in host mode, and when a B-cable is used, the driver will be in peripheral mode. Sorry for the slow response. When building ARCH=arm bcm2835_defconfig, I get build errors: drivers/built-in.o: In function `dwc2_gadget_init': drivers/usb/dwc2/s3c-hsotg.c:3335: undefined reference to `usb_add_gadget_udc' drivers/built-in.o: In function `s3c_hsotg_remove': drivers/usb/dwc2/s3c-hsotg.c:3358: undefined reference to `usb_del_gadget_udc' drivers/usb/dwc2/s3c-hsotg.c:3364: undefined reference to `usb_gadget_unregister_driver' -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] regulator: fixed: update to devm_* API
On 01/27/2014 08:12 PM, Manish Badarkhe wrote: Update the code to use devm_* API so that driver core will manage resources. I'm not sure why this patch is sent to linux-te...@vger.kernel.org; it seems nothing to do with Tegra (or Samsung or OMAP for that matter). -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
On 12/03/2013 02:29 AM, Linus Walleij wrote: ... So I guess what you're after is a kind of hog that will be pushed aside and ignored if a struct device with an associated state appears that will use the same pin? That probably would be useful. Perhaps we should just make all hogs not exclusively own the pins they configure? It is true that we currently require the tables to be strict and not overlap like this, so ideally you should remove the hogs when you have a device driver, but you're actually describing an interesting case here: What if I have a driver for this IP block, and it was supposed to take care of a few pins, but I decide not to compile it into my kernel? Or if I have it as a module and only modprobe it later at runtime? Indeed, that's the nasty hole in pushing even static per-device pinctrl configuration into each device's node; the device may not appear. Related, I prefer to put /all/ static pinctrl configuration into the pinctrl device's default state (i.e. use a hog) rather than configuring the static pinctrl setup per device, for another reason too: If a particular IO controller's signals can be routed to n different (sets of) pins, then we need to do *both* of the following when setting up the pinmux: a) Configure the pins we want to host those signals to route to/from that particular IO controller. b) Configure any other pins that could route to/from that particular IO controller as some other function; either disabled, or routed to/from some different IO controller. That is so that the IO controller's RX/input signals are not connected from two different sets of pins at once, which would cause two things to driver them. Depending on HW, this could cause on of: 1) Multiple drivers - high power usage, or even Silicon damage. 2) Inconsistent configuration, with the wrong set of pins driving the IO controller's inputs, and hence the signals on the correct pins being ignored - hard to find bug. Now, (a) could easily happen when the driver for the IO controller is probed. However, (b) can't, because some other IO controller may need to use those pins, and the two drivers (or pinctrl states for different devices) can't both set up those pins. The only way to solve this is to set up all pinmux state using a single global table (e.g. the pin controller's default state, or hog) that is applied early on. If we rely on resolving these conflicts in per-device default/... states, then that means the conflicts won't get resolved until a driver gets probed, if it ever does, which is too late. ... and as such, I prefer only putting *dynamic* configuration into per-device (non-hog) nodes. (e.g. an I2C bus mux driver which actively changes the pinmux at run-time to move an I2C controller between different sets of SoC pins). -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
On 11/19/2013 10:15 AM, Tomasz Figa wrote: This patch extends the range of settings configurable via pinfunc API to cover pin value as well. This allows configuration of default values of pins. Shouldn't there be a driver that acquires the GPIO that's output to the pin, and configures the output value? IIRC there have been previous discussions re: having a list of e.g. initial GPIO output values in DT, and that was rejected, and this patch seems to be doing almost the exact same thing, just at the pinctrl level rather than GPIO level. That all said, I admit this could be a useful feature... -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pinctrl: samsung: Allow grouping multiple pinmux/pinconf nodes
On 11/19/2013 10:10 AM, Tomasz Figa wrote:
One of remaining limitations of current pinctrl-samsung driver was
the inability to parse multiple pinmux/pinconf group nodes grouped
inside a single device tree node. It made defining groups of pins for
single purpose, but with different parameters very inconvenient.
This patch implements Tegra-like support for grouping multiple pinctrl
groups inside one device tree node, by completely changing the way
pin groups and functions are parsed from device tree.
The code creating
pinctrl maps from DT nodes has been borrowed from pinctrl-tegra,
A lot of the Tegra code has been slightly generalized and put into
pinconf-generic.c. Can the Samsung driver be converted to use that core
code rather than adding another copy of it? Perhaps this isn't possible
given the backwards-compatibility requirements that allow either 1- or
2-level nodes though, although I imagine that could be added to the core
code. One thing you'd certainly need to do is enhance the code in
pinconf-generic.c so that you could substitute your own
pinconf_generic_parse_dt_config() function or dt_params[] table, to
allow for the SoC-specific property names, but I doubt that's too hard.
Tegra could be converted then too:-)
while
the initial creation of groups and functions has been completely
rewritten with following assumptions:
- each group consists of just one pin and does not depend on data
from device tree,
- each function is represented by a device tree child node of the
pin controller, which in turn can contain multiple child nodes
for pins that need to have different configuration values.
OK, I think that sounds reasonable.
Device Tree bindings are fully backwards compatible. New functionality
can be used by defining a new pinctrl group consisting of several child
nodes, as on following example:
sd4_bus8: sd4-bus-width8 {
part-1 {
samsung,pins = gpk0-3, gpk0-4,
gpk0-5, gpk0-6;
samsung,pin-function = 3;
samsung,pin-pud = 3;
samsung,pin-drv = 3;
};
part-2 {
samsung,pins = gpk1-3, gpk1-4,
gpk1-5, gpk1-6;
samsung,pin-function = 4;
samsung,pin-pud = 4;
samsung,pin-drv = 3;
};
};
OK, that all looks great!
diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
The DT changes fully, and the code a little briefly,
Reviewed-by: Stephen Warren swar...@nvidia.com
Just a minor comment below,
diff --git a/drivers/pinctrl/pinctrl-samsung.c
b/drivers/pinctrl/pinctrl-samsung.c
+static int samsung_pinctrl_create_function(struct device *dev,
+ struct samsung_pinctrl_drv_data *drvdata,
+ struct device_node *func_np,
+ struct samsung_pmx_func *func)
...
+ for (i = 0; i npins; ++i) {
+ const char *gname;
+ char *gname_copy;
+
+ ret = of_property_read_string_index(func_np, samsung,pins,
+ i, gname);
+ if (ret) {
+ dev_err(dev,
+ failed to read pin name %d from %s node\n,
+ i, func_np-name);
+ return ret;
}
+
+ gname_copy = devm_kzalloc(dev, strlen(gname) + 1, GFP_KERNEL);
+ if (!gname_copy)
+ return -ENOMEM;
+ strcpy(gname_copy, gname);
Is the lifetime of the string returned by
of_property_read_string_index() really so short that you must copy the
string? I'd be tempted just to store the pointer, although perhaps you
need to get() the node so that's safe.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
On 11/19/2013 11:59 AM, Doug Anderson wrote: On Tue, Nov 19, 2013 at 10:46 AM, Stephen Warren swar...@wwwdotorg.org wrote: On 11/19/2013 10:15 AM, Tomasz Figa wrote: This patch extends the range of settings configurable via pinfunc API to cover pin value as well. This allows configuration of default values of pins. Shouldn't there be a driver that acquires the GPIO that's output to the pin, and configures the output value? IIRC there have been previous discussions re: having a list of e.g. initial GPIO output values in DT, and that was rejected, and this patch seems to be doing almost the exact same thing, just at the pinctrl level rather than GPIO level. That all said, I admit this could be a useful feature... I haven't followed all of the previous discussions, but I know I've run into scenarios where something like this would be useful. The one that comes to mind is: * We've got GPIOs that default at bootup to a pulled up input since the default state of the pin should be high. * These pins are really intended to be outputs, like an enable, reset, or power down line for a peripheral. The pullup is strong enough to give us a good default state but we really want outputs. * We'd like to provide this GPIO to a peripheral through device tree. ...and we'd like all the pinmux to be setup automatically so we use pinctrl-names = default. * If we set the pinmux up as output then there's a chance that the line will glitch at bootup since the pinmux happens (changing the pin to output) before the driver has a chance to run. I think that last point should be addressed by having a driver that owns the GPIO set it to the desired output level, and the implementation of the SoC's GPIO driver communicate with the pinctrl driver (which might be the same driver; not sure here) so that gpio_direction_output() causes the pinctrl HW to be programmed as output only after the GPIO HW is programmed as output and with the correct output value. In this scenario, the pinctrl default state wouldn't touch the pin's input/output setting; that operation would be deferred until the driver set up the GPIO as an output. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
On 11/19/2013 05:02 PM, Kyungmin Park wrote: On Wed, Nov 20, 2013 at 4:16 AM, Stephen Warren swar...@wwwdotorg.org wrote: On 11/19/2013 11:59 AM, Doug Anderson wrote: On Tue, Nov 19, 2013 at 10:46 AM, Stephen Warren swar...@wwwdotorg.org wrote: On 11/19/2013 10:15 AM, Tomasz Figa wrote: This patch extends the range of settings configurable via pinfunc API to cover pin value as well. This allows configuration of default values of pins. Shouldn't there be a driver that acquires the GPIO that's output to the pin, and configures the output value? IIRC there have been previous discussions re: having a list of e.g. initial GPIO output values in DT, and that was rejected, and this patch seems to be doing almost the exact same thing, just at the pinctrl level rather than GPIO level. That all said, I admit this could be a useful feature... I haven't followed all of the previous discussions, but I know I've run into scenarios where something like this would be useful. The one that comes to mind is: * We've got GPIOs that default at bootup to a pulled up input since the default state of the pin should be high. * These pins are really intended to be outputs, like an enable, reset, or power down line for a peripheral. The pullup is strong enough to give us a good default state but we really want outputs. * We'd like to provide this GPIO to a peripheral through device tree. ...and we'd like all the pinmux to be setup automatically so we use pinctrl-names = default. * If we set the pinmux up as output then there's a chance that the line will glitch at bootup since the pinmux happens (changing the pin to output) before the driver has a chance to run. I think that last point should be addressed by having a driver that owns the GPIO set it to the desired output level, and the implementation of Some pins are not connected (NC). At that cases, there's no drivers to handle it. To reduce power leakage, it sets proper configuration with values instead of reset values. Hmm. Shouldn't board firmware configure that kind of thing? (Of course, some firmware is starting to use DT to configure itself, so that just shifts the DT discussion, but anyway). -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On 11/17/2013 08:59 AM, Catalin Marinas wrote: On 17 November 2013 08:49, Alexandre Courbot acour...@nvidia.com wrote: The ARM tree includes a firmware_ops interface that is designed to implement support for simple, TrustZone-based firmwares but could also cover other use-cases. It has been suggested that this interface might be useful to other architectures (e.g. arm64) and that it should be moved out of arch/arm. NAK. I'm for code sharing with arm via common locations but this API goes against the ARMv8 firmware standardisation efforts like PSCI, encouraging each platform to define there own non-standard interface. Surely PSCI is *an* implementation of firmware_ops? Couldn't firmware_ops be relevant to non-ARM architectures too? If so, that would support my previous point; we're presumably not requiring non-ARM architectures to implement PSCI? On a practical note, unless ARM mandates by ARM architecture licensing condition that mechanisms other than PSCI are not allowed, then they're going to exist even if the upstream Linux community doesn't like it. History has certainly shown that. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On 11/18/2013 04:58 AM, Catalin Marinas wrote: ... Of course, trusted foundations interface could be plugged into cpu_ops on arm64 but I will NAK it on the grounds of not using the PSCI API, nor the SMC calling convention (and it's easy to fix when porting to ARMv8). If a supported standard API is used, then there is no need for additional code in the kernel. What happens when someone takes an existing working secure-mode SW stack and simply re-uses it on some new ARMv8 SoC. Are you going to force people working on upstream to re-write the secure mode firmware in shipped hardware before allowing upstream kernel support? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On 11/18/2013 10:10 AM, Russell King - ARM Linux wrote: On Mon, Nov 18, 2013 at 10:03:37AM -0700, Stephen Warren wrote: On 11/18/2013 04:58 AM, Catalin Marinas wrote: ... Of course, trusted foundations interface could be plugged into cpu_ops on arm64 but I will NAK it on the grounds of not using the PSCI API, nor the SMC calling convention (and it's easy to fix when porting to ARMv8). If a supported standard API is used, then there is no need for additional code in the kernel. What happens when someone takes an existing working secure-mode SW stack and simply re-uses it on some new ARMv8 SoC. Are you going to force people working on upstream to re-write the secure mode firmware in shipped hardware before allowing upstream kernel support? I'll tell you what will happen. They'll say screw mainline, we're doing our own thing. Vendors have been doing that for years, this will be no different and require no additional effort from them. Yes, that's my point. However, what does that mean for those people within the company trying to move the SW stack towards mainline? If the answer from upstream is: no, you can't support the shipping HW in mainline, rather than a practical approach that recognizes that the HW/SW/firmware really does exist and might be useful to support in mainline, then that just makes the life of people trying to push for mainline that much harder. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On 11/18/2013 10:30 AM, Catalin Marinas wrote: On Mon, Nov 18, 2013 at 05:03:37PM +, Stephen Warren wrote: On 11/18/2013 04:58 AM, Catalin Marinas wrote: ... Of course, trusted foundations interface could be plugged into cpu_ops on arm64 but I will NAK it on the grounds of not using the PSCI API, nor the SMC calling convention (and it's easy to fix when porting to ARMv8). If a supported standard API is used, then there is no need for additional code in the kernel. What happens when someone takes an existing working secure-mode SW stack and simply re-uses it on some new ARMv8 SoC. Are you going to force people working on upstream to re-write the secure mode firmware in shipped hardware before allowing upstream kernel support? Don't confuse the secure stack with the secure monitor running at EL3. If you want AArch64 support for lower levels (EL2, EL1, EL0), your monitor _must_ be AArch64. You can't run legacy AArch32 code at EL3 and have lower levels in AArch64 mode (architectural constraint). I was assuming that vendors would take the existing source code and simply rebuild it to create the AArch64 secure world. As such, the same SMC IDs, same structures, etc. would be used. The only source difference would be to perhaps change some 32-bit registers/struct-fields up to 64-bit. Naively that sounds like the lowest-effort way to get an AArch64 secure world, so I'm purely guessing that that's what vendors will do. You can still keep the secure services at S-EL1 in AArch32, only that the SMCs are handled by EL3 (and that's another aspect the SMC calling convention spec is trying to address, mixed register-width secure/non-secure OSes). I'm not sure of the implications of that statement. Since you mention SMCs being handled by EL3, I think the quick-and-dirty conversion I mention above is still likely to be used. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] pwm-backlight: Refactor backlight power on/off
On 09/23/2013 03:40 PM, Thierry Reding wrote:
In preparation for adding an optional regulator and enable GPIO to the
driver, split the power on and power off sequences into separate
functions to reduce code duplication at the multiple call sites.
diff --git a/drivers/video/backlight/pwm_bl.c
b/drivers/video/backlight/pwm_bl.c
+static void pwm_backlight_power_off(struct pwm_bl_data *pb)
+{
+ pwm_disable(pb-pwm);
Both the call-sites you're replacing do the following before pwm_disable():
pwm_config(pb-pwm, 0, pb-period);
While I agree that probably shouldn't be necessary, I think it's at
least worth mentioning that in the commit description just to make it
obvious that it was a deliberate change. Splitting that change into a
separate patch might be reasonable in order to keep refactoring and
functional changes separate, although perhaps it's not worth it.
There are also a couple unrelated whitespace changes thrown in here.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/10] ARM: SAMSUNG: Initialize PWM backlight enable_gpio field
On 09/23/2013 03:41 PM, Thierry Reding wrote:
The GPIO API defines 0 as being a valid GPIO number, so this field needs
to be initialized explicitly.
static void __init smdkv210_map_io(void)
@@ -70,6 +70,7 @@ static struct samsung_bl_drvdata samsung_dfl_bl_data
__initdata = {
.max_brightness = 255,
.dft_brightness = 255,
.pwm_period_ns = 78770,
+ .enable_gpio= -1,
.init = samsung_bl_init,
.exit = samsung_bl_exit,
},
@@ -121,6 +122,10 @@ void __init samsung_bl_set(struct samsung_bl_gpio_info
*gpio_info,
samsung_bl_data-lth_brightness = bl_data-lth_brightness;
if (bl_data-pwm_period_ns)
samsung_bl_data-pwm_period_ns = bl_data-pwm_period_ns;
+ if (bl_data-enable_gpio)
+ samsung_bl_data-enable_gpio = bl_data-enable_gpio;
+ if (bl_data-enable_gpio_flags)
+ samsung_bl_data-enable_gpio_flags = bl_data-enable_gpio_flags;
Won't this cause the core pwm_bl driver to request/manipulate the GPIO,
whereas this driver already does that inside the samsung_bl_init/exit
callbacks? I think you either need to adjust those callbacks, or not set
the new standard GPIO property in samsung_bl_data.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] pwm-backlight: Use new enable_gpio field
On 09/23/2013 03:41 PM, Thierry Reding wrote:
Make use of the new enable_gpio field and allow it to be set from DT as
well. Now that all legacy users of platform data have been converted to
initialize this field to an invalid value, it is safe to use the field
from the driver.
diff --git
a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
Optional properties:
+ - enable-gpios: a list of GPIOs to enable and disable the backlight
That seems to imply that multiple GPIOs are legal. Was that intended? If
not, I would suggest:
- enable-gpios: contains a single GPIO specifier for the GPIO which
enables/disables the backlight. See [1] for the format.
[0]: Documentation/devicetree/bindings/pwm/pwm.txt
+ [1]: Documentation/devicetree/bindings/gpio/gpio.txt
diff --git a/drivers/video/backlight/pwm_bl.c
b/drivers/video/backlight/pwm_bl.c
@@ -51,12 +55,27 @@ static void pwm_backlight_power_on(struct pwm_bl_data
*pb, int brightness,
pb-lth_brightness;
pwm_config(pb-pwm, duty_cycle, pb-period);
+
+ if (gpio_is_valid(pb-enable_gpio)) {
+ if (pb-enable_gpio_flags PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
+ gpio_set_value(pb-enable_gpio, 0);
+ else
+ gpio_set_value(pb-enable_gpio, 1);
+ }
Feel completely free to ignore this, but when the difference in two
code-paths is solely in function parameters, I prefer to calculate the
parameter inside the if statement, then call the function outside the
conditional code, to highlight the common/different parts:
int value;
/* or an if statement for the next 1 line, if you don't like ?: */
value = (pb-enable_gpio_flags PWM_BACKLIGHT_GPIO_ACTIVE_LOW) ? 0 : 1;
gpio_set_value((pb-enable_gpio, value);
@@ -148,11 +168,10 @@ static int pwm_backlight_parse_dt(struct device *dev,
+ data-enable_gpio = of_get_named_gpio_flags(node, enable-gpios, 0,
+ flags);
+ if (gpio_is_valid(data-enable_gpio) (flags OF_GPIO_ACTIVE_LOW))
+ data-enable_gpio_flags |= PWM_BACKLIGHT_GPIO_ACTIVE_LOW;
This doesn't seem to handle deferred probe correctly; I would expect
something more like:
data-enable_gpio = of_get_named_gpio_flags(...);
if (data-enable_gpio == -EPROBE_DEFERRED)
return data-enable_gpio;
if (gpio_is_valid(...))
...
return 0;
I suppose it's possible that the value filters down to
gpio_request_one() and this actually does work out OK, but that feels
very accidental/implicit to me.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/10] pwm-backlight: Use an optional power supply
On 09/23/2013 03:41 PM, Thierry Reding wrote:
Many backlights require a power supply to work properly. This commit
uses a power-supply regulator, if available, to power up and power down
the panel.
I think that all backlights require a power supply, albeit the supply
may not be SW-controllable. Hence, shouldn't the regulator be mandatory
in the binding, yet the driver be defensively coded such that if one
isn't specified, the driver continues to work?
diff --git a/drivers/video/backlight/pwm_bl.c
b/drivers/video/backlight/pwm_bl.c
@@ -253,6 +264,16 @@ static int pwm_backlight_probe(struct platform_device
*pdev)
}
}
+ pb-power_supply = devm_regulator_get_optional(pdev-dev, power);
... so I think that should be devm_regulator_get(), since the regulator
isn't really optional.
+ if (IS_ERR(pb-power_supply)) {
+ if (PTR_ERR(pb-power_supply) != -ENODEV) {
+ ret = PTR_ERR(pb-power_supply);
+ goto err_gpio;
+ }
+
+ pb-power_supply = NULL;
If devm_regulator_get_optional() returns an error value or a valid
value, then I don't think that this driver should transmute error values
into NULL; NULL might be a perfectly valid regulator value. Related, I
think the if (pb-power_supply) tests should be replaced with if
(!IS_ERR(pb-power_supply)) instead.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/10] pwm-backlight: Allow backlight to remain disabled on boot
On 09/23/2013 03:41 PM, Thierry Reding wrote:
The default for backlight devices is to be enabled immediately when
registering with the backlight core. This can be useful for setups that
use a simple framebuffer device and where the backlight cannot otherwise
be hooked up to the panel.
However, when dealing with more complex setups, such as those of recent
ARM SoCs, this can be problematic. Since the backlight is usually setup
separately from the display controller, the probe order is not usually
deterministic. That can lead to situations where the backlight will be
powered up and the panel will show an uninitialized framebuffer.
Furthermore, subsystems such as DRM have advanced functionality to set
the power mode of a panel. In order to allow such setups to power up the
panel at exactly the right moment, a way is needed to prevent the
backlight core from powering the backlight up automatically when it is
registered.
This commit introduces a new boot_off field in the platform data (and
also implements getting the same information from device tree). When set
the initial backlight power mode will be set to off.
diff --git
a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
+ - backlight-boot-off: keep the backlight disabled on boot
A few thoughts:
1) Does this property indicate that:
a) The current state of the backlight at boot. In which case, this will
need bootloader involvement to modify the value in the DT at boot time
based on whether the bootloader turned on the backlight:-(
b) That the driver should not turn on the backlight immediately? That
seems to describe driver behaviour more than HW. Is it appropriate to
put that into DT?
Your suggestion to make the backlight not turn on by default might be a
better fix?
2) Should the driver instead attempt to read the current state of the
GPIO output to determine whether the backlight is on? This may not be
possible on all HW.
3) Doesn't the following code in probe() (added in a previous patch)
need to be updated?
+ if (gpio_is_valid(pb-enable_gpio)) {
+ if (pb-enable_gpio_flags PWM_BACKLIGHT_GPIO_ACTIVE_LOW)
+ gpio_set_value(pb-enable_gpio, 0);
+ else
+ gpio_set_value(pb-enable_gpio, 1);
+ }
... That assumes that the backlight is on at boot, and hence presumably
after this patch still always turns on the backlight, only to turn it
off very quickly once the following code in this patch executes:
(and perhaps we also need to avoid turning the backlight off then on if
it was already on at boot)
diff --git a/drivers/video/backlight/pwm_bl.c
b/drivers/video/backlight/pwm_bl.c
@@ -318,6 +320,12 @@ static int pwm_backlight_probe(struct platform_device
*pdev)
}
bl-props.brightness = data-dft_brightness;
+
+ if (data-boot_off)
+ bl-props.power = FB_BLANK_POWERDOWN;
+ else
+ bl-props.power = FB_BLANK_UNBLANK;
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/10] ARM: SAMSUNG: Initialize PWM backlight enable_gpio field
On 10/01/2013 02:43 PM, Thierry Reding wrote:
On Tue, Oct 01, 2013 at 12:31:04PM -0600, Stephen Warren wrote:
On 09/23/2013 03:41 PM, Thierry Reding wrote:
The GPIO API defines 0 as being a valid GPIO number, so this
field needs to be initialized explicitly.
static void __init smdkv210_map_io(void)
@@ -70,6 +70,7 @@ static struct samsung_bl_drvdata
samsung_dfl_bl_data __initdata = { .max_brightness = 255,
.dft_brightness = 255, .pwm_period_ns = 78770, + .enable_gpio
= -1, .init = samsung_bl_init, .exit =
samsung_bl_exit, }, @@ -121,6 +122,10 @@ void __init
samsung_bl_set(struct samsung_bl_gpio_info *gpio_info,
samsung_bl_data-lth_brightness = bl_data-lth_brightness; if
(bl_data-pwm_period_ns) samsung_bl_data-pwm_period_ns =
bl_data-pwm_period_ns; + if (bl_data-enable_gpio) +
samsung_bl_data-enable_gpio = bl_data-enable_gpio; + if
(bl_data-enable_gpio_flags) +
samsung_bl_data-enable_gpio_flags =
bl_data-enable_gpio_flags;
Won't this cause the core pwm_bl driver to request/manipulate the
GPIO, whereas this driver already does that inside the
samsung_bl_init/exit callbacks? I think you either need to adjust
those callbacks, or not set the new standard GPIO property in
samsung_bl_data.
I don't think so. The samsung_bl_data is a copy of
samsung_dfl_bl_data augmented by board-specific settings. So in
fact copying these values here is essential to allow boards to
override the enable_gpio and flags fields. Currently no board sets
the enable_gpio to a valid GPIO so it's all still handled by the
callbacks only.
Oh yes, you're right. I was confusing the new enable_gpio field in
pwm_bl's platform data with some other field in a custom data structure.
One minor point though:
+ if (bl_data-enable_gpio) + samsung_bl_data-enable_gpio =
bl_data-enable_gpio;
That assumes that enable_gpio==0 means none, whereas you've gone to
great pains in the rest of the series to allow 0 to be a valid GPIO
ID. right now, the default value of samsung_bl_data-enable_gpio is
-1, and if !bl_data-enable_gpio, that value won't be propagated across.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/10] pwm-backlight: Use an optional power supply
On 10/01/2013 02:53 PM, Thierry Reding wrote:
On Tue, Oct 01, 2013 at 12:43:57PM -0600, Stephen Warren wrote:
On 09/23/2013 03:41 PM, Thierry Reding wrote:
Many backlights require a power supply to work properly. This
commit uses a power-supply regulator, if available, to power up
and power down the panel.
I think that all backlights require a power supply, albeit the
supply may not be SW-controllable. Hence, shouldn't the regulator
be mandatory in the binding, yet the driver be defensively coded
such that if one isn't specified, the driver continues to work?
That has already changed in my local version of this patch.
diff --git a/drivers/video/backlight/pwm_bl.c
b/drivers/video/backlight/pwm_bl.c
@@ -253,6 +264,16 @@ static int pwm_backlight_probe(struct
platform_device *pdev) } }
+ pb-power_supply = devm_regulator_get_optional(pdev-dev,
power);
... so I think that should be devm_regulator_get(), since the
regulator isn't really optional.
+ if (IS_ERR(pb-power_supply)) { + if
(PTR_ERR(pb-power_supply) != -ENODEV) { + ret =
PTR_ERR(pb-power_supply); +goto err_gpio; +
} + +
pb-power_supply = NULL;
If devm_regulator_get_optional() returns an error value or a
valid value, then I don't think that this driver should transmute
error values into NULL; NULL might be a perfectly valid regulator
value. Related, I think the if (pb-power_supply) tests should be
replaced with if (!IS_ERR(pb-power_supply)) instead.
All of that is already done in my local tree. This actually turns
out to work rather smoothly with the new support for optional
regulators. The regulator core will give you a dummy regulator
(assuming it's there physically but hasn't been wired up in
software) that's always on, so the driver doesn't even have to
special case it anymore.
OK, hopefully it (the regulator core) complains about the missing DT
property though; I assume you're using regulator_get() not
regulator_get_optional(), since the supply really is not optional.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] clk: exynos5410: register clocks using common clock framework
On 10/01/2013 10:17 AM, Vyacheslav Tyrtov wrote: From: Tarek Dakhran t.dakh...@samsung.com The EXYNOS5410 clocks are statically listed and registered using the Samsung specific common clock helper functions. diff --git a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt + [Core Clocks] + [Clock Gate for Special Clocks] + [Peripheral Clock Gates] These headers/titles for the sections/lists aren't consistently aligned. + [Clock Gate for Special Clocks] + + Clock ID + + sclk_uart0 128 + sclk_uart1 129 + sclk_uart2 130 + sclk_uart3 131 + sclk_mmc0 132 + sclk_mmc1 133 + sclk_mmc2 134 + + [Peripheral Clock Gates] + + Clock ID + + + uart0 257 + uart1 258 + uart2 259 + uart3 260 + mct315 + mmc0 351 + mmc1 352 + mmc2 353 That's not many clocks. I assume you're planning on adding more IDs later, in a backwards-compatible fashion? I suppose that's OK since it won't break any existing usage, as long as there's no need to renumber any existing values. On that topic, are any of those clock IDs derived from HW, e.g. register numbers, or bit numbers in an array of registers? Numbering clocks in a HW-derived fashion would make it easier or more obvious how to add new clock IDs later while maintaining some consistency and without introducing the desire to break any ABI. Finally, how about creating a header file such as include/dt-bindings/clock/exynos5410.h to define those clock IDs, so that both *.dts and the clock driver can share the values without having to manually write them? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] ARM: provide common arch init for DT clocks
On 09/18/2013 01:52 PM, Sebastian Hesselbarth wrote: On 09/18/2013 09:47 PM, Jason Cooper wrote: On Wed, Sep 18, 2013 at 07:53:33PM +0200, Sebastian Hesselbarth wrote: ... Sebastian Hesselbarth (26): ARM: nomadik: move mtu setup to clocksource init ... How would you like to handle this series? Jason, honestly I don't really know, yet. I was hoping for Arnd and Olof decide on that. Maybe they also create a topic branch up to where arch-wide of_clk_init is introduced. Then each removal patch can go through the independent sub-trees. There may be more machs introduced before, that can then also depend on the common branch. Oh, I was assuming you'd just take the whole thing through one tree, likely in arm-soc. That's why I created the topic branch for the Tegra PMIC patch that's a dependency of this... However, I guess if you just merge the core into a topic branch in arm-soc and everyone merges it back, that's fine. I assume the Tegra topic branch I created won't be necessary in that case. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 12/12] V4L: Add driver for s5k4e5 image sensor
On 09/13/2013 06:55 AM, Philipp Zabel wrote:
Hi Arun,
Am Donnerstag, den 12.09.2013, 17:37 +0530 schrieb Arun Kumar K:
This patch adds subdev driver for Samsung S5K4E5 raw image sensor.
Like s5k6a3, it is also another fimc-is firmware controlled
sensor. This minimal sensor driver doesn't do any I2C communications
as its done by ISP firmware. It can be updated if needed to a
regular sensor driver by adding the I2C communication.
... [untrimmed patch] ...
+Example:
+
+i2c-isp@1313 {
+s5k4e5@20 {
+compatible = samsung,s5k4e5;
+reg = 0x20;
+gpios = gpx1 2 1;
This probably should be 'reset-gpios', too.
... [untrimmed patch] ...
regards
Philipp
Please delete unnecessary context when replying so that people don't
have to scroll through hundreds of lines of patch to see a 1-line comment.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: dts: Disable I2C controllers by default on Exynos5250
On 09/12/2013 06:58 AM, Tomasz Figa wrote:
Hi Mark,
On Thursday 12 of September 2013 11:40:27 Mark Brown wrote:
From: Mark Brown broo...@linaro.org
Ensure that unused I2C controllers are not activated, causing problems
due to inappropriate pinmuxing or similar, by marking the controllers as
disabled by default and requiring boards to explicitly enable those that
are in use.
diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts
i2c@12C6 {
+status = okay;
I would keep the status properties at the end of all properties, indicating
that at this point the description is complete and also to be consistent
with other dts files around.
DT doesn't define any kind of ordering for the properties AFAIK, so the
order shouldn't matter in practice; it conveys no semantic
representation as far as the parsing code is concerned even if a human
may be influenced otherwise:-)
Just as an FYI, the rule I've been trying to follow in Tegra DT files is
that properties that are overridden from any included .dtsi file come
first, followed by any new properties. Still, there's no particular
reason anyone else has to follow that layout.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 1/7] irqchip: vic: Parse interrupt and resume masks from device tree
On 08/22/2013 05:22 PM, Tomasz Figa wrote: This patch extends vic_of_init to parse valid interrupt sources and resume sources masks from device tree. If mask values are not specified in device tree, all sources are assumed to be valid, as before this patch. Can you explain further why the VIC needs this information up-front? Presumably it can accumulate it as devices request interrupts. diff --git a/Documentation/devicetree/bindings/arm/vic.txt b/Documentation/devicetree/bindings/arm/vic.txt +- valid-mask : Bit mask of valid interrupt sources (defaults to all valid) Until a device requests an interrupt, it can be left disabled. Once something does request it, it gets enabled. If nothing requests it, it never gets enabled. Doesn't the result of that logic end up being the same thing as valid-mask represents? The only difference would be if some device requests an invalid interrupt source, but then why not just fix the interrupt client instead of adding this property to reject the request? +- wakeup-mask : Bit mask of interrupt sources that can wake up the system + (defaults to all allowed) Shouldn't drivers for end-devices request interrupts, and then set wake enable on those interrupts, which would then trickle up the IRQ chain to tell the VIC which interrupts to enable wakeup on? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] s5p-jpeg: Add initial device tree support for S5PV210/Exynos4210 SoCs
On 08/23/2013 10:01 AM, Sylwester Nawrocki wrote: On 08/18/2013 10:14 PM, Sylwester Nawrocki wrote: This patch enables the JPEG codec on S5PV210 and Exynos4210 SoCs. There are some differences in newer versions of the JPEG codec IP on SoCs like Exynos4x12 and Exynos5 series and support for them will be added in subsequent patches. Cc: Andrzej Pietrasiewicz andrze...@samsung.com Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Could a DT maintainer review/Ack the binding in this patch ? The binding looks reasonable to me, so, Acked-by: Stephen Warren swar...@nvidia.com -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 1/7] irqchip: vic: Parse interrupt and resume masks from device tree
On 08/23/2013 05:04 PM, Tomasz Figa wrote: On Friday 23 of August 2013 16:11:18 Stephen Warren wrote: On 08/22/2013 05:22 PM, Tomasz Figa wrote: This patch extends vic_of_init to parse valid interrupt sources and resume sources masks from device tree. If mask values are not specified in device tree, all sources are assumed to be valid, as before this patch. Can you explain further why the VIC needs this information up-front? Presumably it can accumulate it as devices request interrupts. It does not need this information just for operation, but this makes the hardware description more detailed and allows better sanity checking of interrupts being requested. Ah, OK. It may be worth mentioning the intent of the properties. I suppose this is purely a representation of HW then, so it's reasonable to include it in the binding. I'm not sure /quite/ how useful it is; after all the error-checking that it enables will never trigger assuming the rest of the DT is written to request the correct interrupts. However, I guess there is little harm in allowing these properties. Bikeshedding a bit, but perhaps rename wakeup-mask to valid-wakeup-mask (and perhaps valid-mask to valid-source-mask for consistency, although I see you already renamed that to be consistent with other bindings...)? To me, wakeup-mask sounds like a configuration of which sources should be configured to wakeup the system; something to do with configuration/policy rather than HW capabilities. Either way, the binding, Acked-by: Stephen Warren swar...@nvidia.com (I assume those 2 new properties always have 1 cell since the controller can only support up to 32 IRQ sources? If not, then perhaps add wording to describe how long the properties should be). -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] s5k5baf: add camera sensor driver
On 08/21/2013 08:41 AM, Andrzej Hajda wrote: Driver for Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP. The driver exposes the sensor as two V4L2 subdevices: - S5K5BAF-CIS - pure CMOS Image Sensor, fixed 1600x1200 format, no controls. - S5K5BAF-ISP - Image Signal Processor, formats up to 1600x1200, pre/post ISP cropping, downscaling via selection API, controls. The binding, Acked-by: Stephen Warren swar...@nvidia.com (although it would be great if another DT binding maintainer gave it a quick look-over to make sure I didn't miss anything!) -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Documentation: devicetree: Update Exynos MCT bindings description
On 08/20/2013 07:52 AM, Tomasz Figa wrote:
This patch updates description of device tree bindings for Exynos MCT
(multicore timers). Namely:
- added note about simplified specification of local timer interrupts,
when using single per-processor interrupt for all local timers,
- changed first example that was incorrectly suggesting that global
timer interrupts are optional,
- simplified example interrupt map,
- added example showing simplified local timer interrupt specification.
diff --git
a/Documentation/devicetree/bindings/timer/samsung,exynos4210-mct.txt
b/Documentation/devicetree/bindings/timer/samsung,exynos4210-mct.txt
-Example 1: In this example, the system uses only the first global timer
-interrupt generated by MCT and the remaining three global timer
-interrupts are unused. Two local timer interrupts have been
-specified.
+ For MCT block that uses a per-processor interrupt for local timers, such
+ as ones compatible with samusng,exynos4412-mct, only one local timer
samsung is typo'd there.
+Example 2: In this example, the timer interrupts are connected to two
separate
+interrupt controllers. Hence, an interrupt-map is created to map
+the interrupts to the respective interrupt controllers.
mct@101C {
compatible = samsung,exynos4210-mct;
reg = 0x101C 0x800;
- interrupt-controller;
- #interrups-cells = 2;
interrupt-parent = mct_map;
- interrupts = 0 0, 1 0, 2 0, 3 0,
- 4 0, 5 0;
+ interrupts = 0, 1, 2, 3, 4, 5;
mct_map: mct-map {
- #interrupt-cells = 2;
+ #interrupt-cells = 1;
#address-cells = 0;
#size-cells = 0;
I don't believe you need either of those two properties in a node solely
used as an interrupt map.
Also, why not put the interrupt-map property directly into the main mct
node; I don't believe there's any requirement nor advantage to it being
a separate node.
- interrupt-map = 0x0 0 combiner 23 3,
- 0x4 0 gic 0 120 0,
- 0x5 0 gic 0 121 0;
+ interrupt-map = 0 gic 0 57 0,
+ 1 gic 0 69 0,
+ 2 combiner 12 6,
+ 3 combiner 12 7,
+ 4 gic 0 42 0,
+ 5 gic 0 48 0;
};
};
+Example 3: In this example, the IP contains four local timers, but using
+a per-processor interrupt to handle them. Either all the local
+timer interrupts can be specified, with the same interrupt specifier
+value or just the first one.
That sounds like it should be two separate examples.
Actually, there's already a 2-timer example above using separate
interrupts, so why not make this example *just* be for the
single-interrupt case?
+ mct@1005 {
+ compatible = samsung,exynos4412-mct;
+ reg = 0x1005 0x800;
+ interrupts = 0 57 0, 0 69 0, 0 70 0, 0 71 0,
+ 0 42 0/*, 0 42 0, 0 42 0, 0 42 0*/;
+ };
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/16] pwm: samsung: Update DT bindings documentation to cover clocks
On 08/20/2013 11:31 AM, Tomasz Figa wrote: PWM driver consumes at least one and up to three clocks, which need to be specified in device tree when used. This patch updates bindings documentation to add information about clocks. diff --git a/Documentation/devicetree/bindings/pwm/pwm-samsung.txt b/Documentation/devicetree/bindings/pwm/pwm-samsung.txt +- clock-names: should contain all following required clock names: +- timers - PWM base clock used to generate PWM signals, + and any subset of following optional clock names: +- pwm-tclk0 - first external PWM clock source, +- pwm-tclk1 - second external PWM clock source. + Note that not all IP variants allow using all external clock sources. + Refer to SoC documentation to learn which clock source configurations + are available. It might be nice to explicitly enumerate which variants (or rather, I suppose which exact compatible values) support which optional clocks. However, I suppose it's fine to just say go read the HW manual instead. So, this patch, Acked-by: Stephen Warren swar...@nvidia.com (although this patch isn't backwards-compatible since before now, DT nodes didn't need to provide any entries in clocks) -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Documentation: devicetree: Update Exynos MCT bindings description
On 08/20/2013 11:12 AM, Tomasz Figa wrote:
On Tuesday 20 of August 2013 11:00:53 Stephen Warren wrote:
On 08/20/2013 07:52 AM, Tomasz Figa wrote:
This patch updates description of device tree bindings for Exynos MCT
(multicore timers). Namely:
- added note about simplified specification of local timer interrupts,
when using single per-processor interrupt for all local timers,
- changed first example that was incorrectly suggesting that global
timer interrupts are optional,
- simplified example interrupt map,
- added example showing simplified local timer interrupt
specification.
diff --git
a/Documentation/devicetree/bindings/timer/samsung,exynos4210-mct.txt
b/Documentation/devicetree/bindings/timer/samsung,exynos4210-mct.txt
-Example 1: In this example, the system uses only the first global
timer
- interrupt generated by MCT and the remaining three global timer
- interrupts are unused. Two local timer interrupts have been
- specified.
+ For MCT block that uses a per-processor interrupt for local timers,
such + as ones compatible with samusng,exynos4412-mct, only one
local timer
samsung is typo'd there.
Oops. ;)
+Example 2: In this example, the timer interrupts are connected to two
separate + interrupt controllers. Hence, an interrupt-map is
created to map + the interrupts to the respective interrupt
controllers.
mct@101C {
compatible = samsung,exynos4210-mct;
reg = 0x101C 0x800;
- interrupt-controller;
- #interrups-cells = 2;
interrupt-parent = mct_map;
- interrupts = 0 0, 1 0, 2 0, 3 0,
-4 0, 5 0;
+ interrupts = 0, 1, 2, 3, 4, 5;
mct_map: mct-map {
- #interrupt-cells = 2;
+ #interrupt-cells = 1;
#address-cells = 0;
#size-cells = 0;
I don't believe you need either of those two properties in a node solely
used as an interrupt map.
Well, you don't need #size-cells, as it is not used for interrupt-map
property.
As for #address-cell property, you need it, as it defines how many cells
are used in interrupt map specifier for unit address. See ePAPR 2.4.3.1 or
[1] for a description of interrupt-map property format.
[1] - http://devicetree.org/Device_Tree_Usage#Advanced_Interrupt_Mapping
Uggh. Yes, you're right.
Also, why not put the interrupt-map property directly into the main mct
node; I don't believe there's any requirement nor advantage to it being
a separate node.
It is more readable, as you don't mix virtual (helper) properties, with
those describing the hardware. Otherwise both ways are technically correct,
but not for all cases, i.e. only when #address-cells and #interrupt-cells
properties aren't used for device's own purposes.
Hmm. I see the argument.
+Example 3: In this example, the IP contains four local timers, but
using +a per-processor interrupt to handle them. Either all the
local +timer interrupts can be specified, with the same interrupt
specifier +value or just the first one.
That sounds like it should be two separate examples.
Actually, there's already a 2-timer example above using separate
interrupts, so why not make this example *just* be for the
single-interrupt case?
Well, I wanted to show that both ways of specification would be equivalent
here. If you insist on making it a single example, then I can send next
version with this changed.
Oh! I didn't see the /* */ at all in the third example...
I think it'd be more obvious if you wrote the whole property out twice:
+ interrupts = 0 57 0, 0 69 0, 0 70 0, 0 71 0,
+0 42 0/*, 0 42 0, 0 42 0, 0 42 0*/;
+ /* or: */
+ interrupts = 0 57 0, 0 69 0, 0 70 0, 0 71 0,
+0 42 0;
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] s5k5baf: add camera sensor driver
On 08/20/2013 10:03 AM, Andrzej Hajda wrote: Driver for Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP. The driver exposes the sensor as two V4L2 subdevices: - S5K5BAF-CIS - pure CMOS Image Sensor, fixed 1600x1200 format, no controls. - S5K5BAF-ISP - Image Signal Processor, formats up to 1600x1200, pre/post ISP cropping, downscaling via selection API, controls. diff --git a/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt +Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP + + +Required properties: + +- compatible : samsung,s5k5baf; +- reg : I2C slave address of the sensor; +- vdda-supply : analog power supply 2.8V (2.6V to 3.0V); +- vddreg-supply: regulator input power supply 1.8V (1.7V to 1.9V) + or 2.8V (2.6V to 3.0); +- vddio-supply : I/O power supply 1.8V (1.65V to 1.95V) + or 2.8V (2.5V to 3.1V); +- stbyn-gpios : GPIO connected to STDBYN pin; +- rstn-gpios : GPIO connected to RSTN pin; +- clocks : the sensor's master clock specifier (from the common + clock bindings); +- clock-names : must be mclk; That all looks sane. +Optional properties: + +- clock-frequency : master clock frequency in Hz; if this property is + not specified default 24 MHz value will be used. I /think/ the explanation you gave Mark on this property makes sense. However, it's not clear from the description what this does; in many other cases a clock-frequency describes a fixed/actual input clock rate to a device, rather than a frequency which the device believes it should operate at, and hence the driver should request. Perhaps the following would describe this: - clock-frequency : The frequency at which the mclk clock should be configured to operate, in Hz. If this property is not specified default 24 MHz value will be used. To me, this more strongly implies that the user of the node should configure the clock, rather than the property reporting the rate at which the clock is already configured to operate. I think the rest of the binding doc (below this point) seems reasonable too. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v5] s5k5baf: add camera sensor driver
On 08/19/2013 11:25 AM, Sylwester Nawrocki wrote: On 08/19/2013 03:25 PM, Pawel Moll wrote: On Mon, 2013-08-19 at 14:18 +0100, Andrzej Hajda wrote: +++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt @@ -0,0 +1,51 @@ +Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP +- + +Required properties: + +- compatible : samsung,s5k5baf; +- reg: I2C slave address of the sensor; +- vdda-supply: analog power supply 2.8V (2.6V to 3.0V); +- vddreg-supply : regulator input power supply 1.8V (1.7V to 1.9V) +or 2.8V (2.6V to 3.0); +- vddio-supply : I/O power supply 1.8V (1.65V to 1.95V) +or 2.8V (2.5V to 3.1V); +- gpios : GPIOs connected to STDBYN and RSTN pins, +in order: STBYN, RSTN; You probably want to use the [name-]gpios convention here (see Documentation/devicetree/bindings/gpio/gpio.txt), so something like stbyn-gpios and rstn-gpios. Unless using multiple named properties is really preferred over a single gpios property I would like to keep the single property containing a list of GPIOs. ... Yes, a separate property for each type of GPIO is typical. Multiple entries in the same property are allowed if they're used for the same purpose/type, whereas here they're clearly different things. Inconsistent with (some) other properties, admittedly... -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v5] s5k5baf: add camera sensor driver
On 08/19/2013 04:53 PM, Tomasz Figa wrote: On Monday 19 of August 2013 16:30:45 Stephen Warren wrote: On 08/19/2013 11:25 AM, Sylwester Nawrocki wrote: On 08/19/2013 03:25 PM, Pawel Moll wrote: On Mon, 2013-08-19 at 14:18 +0100, Andrzej Hajda wrote: +++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt @@ -0,0 +1,51 @@ +Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP +- + +Required properties: + +- compatible : samsung,s5k5baf; +- reg: I2C slave address of the sensor; +- vdda-supply: analog power supply 2.8V (2.6V to 3.0V); +- vddreg-supply : regulator input power supply 1.8V (1.7V to 1.9V) +or 2.8V (2.6V to 3.0); +- vddio-supply : I/O power supply 1.8V (1.65V to 1.95V) +or 2.8V (2.5V to 3.1V); +- gpios : GPIOs connected to STDBYN and RSTN pins, +in order: STBYN, RSTN; You probably want to use the [name-]gpios convention here (see Documentation/devicetree/bindings/gpio/gpio.txt), so something like stbyn-gpios and rstn-gpios. Unless using multiple named properties is really preferred over a single gpios property I would like to keep the single property containing a list of GPIOs. ... Yes, a separate property for each type of GPIO is typical. Multiple entries in the same property are allowed if they're used for the same purpose/type, whereas here they're clearly different things. Inconsistent with (some) other properties, admittedly... I'm not really convinced about the superiority of named gpio properties over a single gpios property with multiple entries in this case. I'd say it's more just a matter of preference. See the clock or interrupt bindings. They all specify all the clocks and interrupts in single property, without any differentiation based on their purposes. Also keep in mind that original GPIO bindings used only a single gpios property and was only extended to allow named ones. Well, it's not so much about what's best, but just being consistent with what's already there. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
On 08/12/2013 05:46 PM, Mark Brown wrote: On Mon, Aug 12, 2013 at 05:18:34PM -0600, Stephen Warren wrote: On 08/12/2013 05:13 PM, Mark Brown wrote: (although I dare say that at least samsung,supports-rstclr should be modified to use the new reset controller bindings) Really? That doesn't seem terribly sane - I had thought that was for bodging resets on the side of things that don't normally have them or need board specific logic. Also note that this is actually a magic register write done to reset the IP on some specific IPs. I believe that's exactly what the reset subsystem and associated DT bindings were designed for. That seems... interesting. It seems like this is fairly core device functionality I'd expect the driver to just be able to understand; the main thing this property was doing was deciding if the reset was needed. I'm not sure I see the benefit here? Sorry, I'm confused here. In this case, the reset is something internal to the IP module, not sourced by an external reset controller, so there's no need to involve the reset controller bindings or subsystem. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
On 08/12/2013 05:13 PM, Mark Brown wrote: On Mon, Aug 12, 2013 at 04:57:53PM -0600, Stephen Warren wrote: On 08/12/2013 03:49 AM, Padmavathi Venna wrote: +- compatible : should be one of the following. + - samsung,s3c6410-i2s: for 8/16/24bit stereo I2S. + - samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with + secondary fifo, s/w reset control and internal mux for root clk src. Those descriptions seem a little odd. If I have an SoC that isn't s5pv210, yet supports 8/16/24bit multichannel(5.1) I2S with secondary fifo, s/w reset control and internal mux for root clk src, will compatible=samsung,s5pv210-i2s work for my HW? I wonder if you should instead include the IP block version in the compatible value? We've been round this loop several times, I'd prefer the IP block versions too but they're at best patchily documented and so as a general policy the Samsung bindings use the name of the SoC an IP first appeared in as the version. In other words, I don't think we have an answer to the question: Should differences between similar HW blocks be encoded into DT properties, or should the driver encode them into some table, and look them up from compatible value? For usability it seems better to just be able to say which IP you've got, this also makes it easier to implement support for new IP features later on without having to go back and add new properties which would be sad. That seems quite reasonable, but I don't think everyone involved in DT has come out and agreed on that. I'm quite happy with the approach of looking up everything based on compatible. (although I dare say that at least samsung,supports-rstclr should be modified to use the new reset controller bindings) Really? That doesn't seem terribly sane - I had thought that was for bodging resets on the side of things that don't normally have them or need board specific logic. Also note that this is actually a magic register write done to reset the IP on some specific IPs. I believe that's exactly what the reset subsystem and associated DT bindings were designed for. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 5/5] ARM: dts: Add dt binding documentation for exynos rotator
On 08/09/2013 07:15 AM, Tomasz Figa wrote: Hi Chanho, On Friday 09 of August 2013 16:40:53 Chanho Park wrote: This patch describes each nodes of rotator and specifies a example how to bind it. diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt +* Samsung Image Rotator + +Required properties: + - compatible : value should be following: +(a) samsung,exynos4210-rotator for Rotator IP in Exynos4210 +(b) samsung,exynos4212-rotator for Rotator IP in Exynos4212/4412 +(c) samsung,exynos5250-rotator for Rotator IP in Exynos5250 Is rotator the name that the HW documentation uses to refer to this block? If so, those compatible values seem fine. If not, they seem slightly too generic for me; perhaps better to use the raw HW block name? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: Exynos: replace custom MFC reserved memory handling with generic code
On 08/08/2013 03:19 PM, Tomasz Figa wrote: Hi Stephen, On Thursday 08 of August 2013 15:00:52 Stephen Warren wrote: On 08/05/2013 06:26 AM, Marek Szyprowski wrote: MFC driver use custom bindings for managing reserved memory. Those bindings are not really specific to MFC device and no even well discussed. They can be easily replaced with generic, platform independent code for handling reserved and contiguous memory. Two additional child devices for each memory port (AXI master) are introduced to let one assign some properties to each of them. Later one can also use them to assign properties related to SYSMMU controllers, which can be used to manage the limited dma window provided by those memory ports. diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt +The MFC device is connected to system bus with two memory ports (AXI +masters) for better performance. Those memory ports are modelled as +separate child devices, so one can assign some properties to them (like +memory region for dma buffer allocation or sysmmu controller). + Required properties: - compatible : value should be either one among the following (a) samsung,mfc-v5 for MFC v5 present in Exynos4 SoCs (b) samsung,mfc-v6 for MFC v6 present in Exynos5 SoCs + and additionally simple-bus to correctly initialize child + devices for memory ports (AXI masters) simple-bus is wrong here; the child nodes aren't independent entities that can be instantiated separately from their parent and without depending on their parent. I fully agree, but I can't think of anything better. Could you suggest a solution that would cover our use case: The MFC IP has two separate bus masters (called here and there memports). On some SoCs, each master must use different memory bank to meet memory bandwidth requirements for higher bitrate video streams. This can be seen as MFC having two DMA subdevices, which have different DMA windows. On Linux, this is handled by binding two appropriate CMA memory regions to the memports, so the driver can do DMA allocations on behalf of particular memport and get appropriate memory for it. I don't see what that has to do with simple-bus. Whatever parses the node of the MFC can directly read from any contained property or child node; there's no need to try and get the core DT tree parser to enumerate the children. If you actually need a struct platform_device for each of these child nodes (which sounds wrong, but I'm not familiar with the code), then simply have the driver call of_platform_populate() itself at the appropriate time. But that's not going to work well unless the child nodes have compatible values, which doesn't seem right given their purpose. - - samsung,mfc-r : Base address of the first memory bank used by MFC - for DMA contiguous memory allocation and its size. - - - samsung,mfc-l : Base address of the second memory bank used by MFC - for DMA contiguous memory allocation and its size. These properties shouldn't be removed, but simply marked deprecated. The driver will need to continue to support them so that old DTs work with new kernels. The binding must therefore continue to document them so that the old DT content still makes sense. I tend to disagree on this. For Samsung platforms we've been trying to avoid DT bindings changes as much as possible, but I'd rather say that device tree was coupled with kernel version it came from, so Samsung- specific bindings haven't been fully stabilized yet, especially since we are still at discussion stage when it's about defining processes for binding staging and stabilization. Well, that's why everyone is shouting at ARM for abusing DT... I would rather see this patch as part of work on Samsung DT binding janitoring or maybe even sanitizing in some cases, like this one, when the old (and IMHO bad) MFC binding was introduced without proper review. I don't think we want to support this kind of brokenness anymore, especially considering the hacks which would be required by such support (see original implementation of this binding and code required in board file). +Two child nodes must be defined for MFC device. Their names must be +following: memport-r and memport-l (right and left). Required Node names shouldn't have semantic meaning. What about bus-master-0 and bus-master-1? Just bus-master for each might make sense. Use reg properties to differentiate the two? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: Exynos: replace custom MFC reserved memory handling with generic code
On 08/08/2013 04:10 PM, Tomasz Figa wrote: On Thursday 08 of August 2013 15:47:19 Stephen Warren wrote: On 08/08/2013 03:19 PM, Tomasz Figa wrote: On Thursday 08 of August 2013 15:00:52 Stephen Warren wrote: On 08/05/2013 06:26 AM, Marek Szyprowski wrote: MFC driver use custom bindings for managing reserved memory. Those bindings are not really specific to MFC device and no even well discussed. They can be easily replaced with generic, platform independent code for handling reserved and contiguous memory. Two additional child devices for each memory port (AXI master) are introduced to let one assign some properties to each of them. Later one can also use them to assign properties related to SYSMMU controllers, which can be used to manage the limited dma window provided by those memory ports. ... +Two child nodes must be defined for MFC device. Their names must be +following: memport-r and memport-l (right and left). Required Node names shouldn't have semantic meaning. What about bus-master-0 and bus-master-1? Just bus-master for each might make sense. Use reg properties to differentiate the two? What this reg property would mean in this case? My understanding of reg property was that it should be used for real addresses or IDs and for all other cases node names should be suffixed with -ID. Presumably it would hold the ID of the HW block as defined in the documentation. Or, it could be somewhat arbitrary. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: EXYNOS: add power domains support for EXYNOS5440
(CCing the other DT maintainers, hence quoting binding in full)
On 08/02/2013 07:23 AM, Bartlomiej Zolnierkiewicz wrote:
On EXYNOS5440 power domains are handled in a different way than on
the previous EXYNOS SoCs. Add support for them to EXYNOS pm_domains
driver. Then add device tree nodes for PCIe (controlling power for
PCIe host controller) and Conn2 (controlling power for Ethernet,
SATA and USB controllers) power domains to exynos5440.dtsi.
Currently if runtime Power Management is enabled and the driver
for devices under power domain is disabled then the power domain
will be disabled by EXYNOS pm_domains driver. To make more active
use of power domains (dynamically enable and disabled them as
needed) it is required to add runtime PM support to pci-exynos,
stmmac, ahci_platform, ohci-exynos and ehci-s5p drivers (to be
done later).
Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com
Signed-off-by: Myungjoo Ham myungjoo@samsung.com
Cc: Tomasz Figa t.f...@samsung.com
Cc: Stephen Warren swar...@wwwdotorg.org
---
Documentation/devicetree/bindings/arm/exynos/power_domain.txt | 33 ++
arch/arm/boot/dts/exynos5440.dtsi | 23 +
arch/arm/mach-exynos/Kconfig |1
arch/arm/mach-exynos/common.c |4
arch/arm/mach-exynos/pm_domains.c | 138
+-
5 files changed, 190 insertions(+), 9 deletions(-)
Index: b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
===
--- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
2013-08-02 14:45:53.551392396 +0200
+++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
2013-08-02 14:46:29.799391845 +0200
@@ -5,20 +5,47 @@ to gate power to one or more peripherals
Required Properties:
- compatible: should be one of the following.
-* samsung,exynos4210-pd - for exynos4210 type power domain.
+* samsung,exynos4210-pd - for Exynos4210 type power domain.
+* samsung,exynos5440-pd - for Exynos5440 type power domain.
- reg: physical base address of the controller and length of memory mapped
-region.
+region (Exynos4210 type power domain) or bit offset in the control
+register (Exynos5440 type power domain).
+
+Additional parent node must be created for Exynos5440 power domains with
+the following required properties:
+- compatible: samsung,exynos5440-power-domains, simple-bus
+- reg: physical base address of the XMU controller and length of memory
+mapped region
It's a little odd to describe the child nodes first. Given the 4210 and
5440 bindings work so differently, I'd suggest making separate binding
files for the two; samsung,exynos4210-pd.txt and samsung,exynos5440-pd.txt.
The node being describe here clearly is not a simple-bus; the child
nodes appear to have a specific need that their parent be compatible =
samsung,exynos5440-power-domains, hence they aren't the independent
devices that simple-bus would usually contain.
Note that I only briefly reviewed the low-level structural aspects of
the binding that I mentioned above; I haven't thought about the binding
at a higher level of e.g. does this binding conceptually make sense.
Node of a device using power domains must have a samsung,power-domain
property
defined with a phandle to respective power domain.
-Example:
+Example for Exynos4210 compatible power domain:
lcd0: power-domain-lcd0 {
compatible = samsung,exynos4210-pd;
reg = 0x10023C00 0x10;
};
+Example for Exynos5440 compatible power domains:
+
+ power-domains@0016 {
+ compatible = samsung,exynos5440-power-domains, simple-bus;
+ reg = 0x0016 0x1000;
+ #address-cells = 1;
+ #size-cells = 0;
+
+ pd_pcie: pcie-power-domain@6 {
+ compatible = samsung,exynos5440-pd;
+ reg = 6;
+ };
+
+ pd_conn2: conn2-power-domain@7 {
+ compatible = samsung,exynos5440-pd;
+ reg = 7;
+ };
+ };
+
Example of the node using power domain:
node {
Index: b/arch/arm/boot/dts/exynos5440.dtsi
===
--- a/arch/arm/boot/dts/exynos5440.dtsi 2013-08-02 14:45:53.599392397
+0200
+++ b/arch/arm/boot/dts/exynos5440.dtsi 2013-08-02 14:46:29.815391842
+0200
@@ -29,6 +29,23 @@
#clock-cells = 1;
};
+ power-domains@0016 {
+ compatible = samsung,exynos5440-power-domains, simple-bus;
+ reg = 0x0016 0x1000;
+ #address-cells = 1;
+ #size-cells = 0;
+
+ pd_pcie: pcie-power-domain@6 {
+ compatible
Re: [PATCH 01/11] spi: Provide core support for runtime PM during transfers
On 07/28/2013 08:43 AM, Mark Brown wrote: From: Mark Brown broo...@linaro.org Most SPI drivers that implement runtime PM support use identical code to do so: they acquire a runtime PM lock in prepare_transfer_hardware() and then they release it in unprepare_transfer_hardware(). The variations in this are mostly missing error checking and the choice to use autosuspend. Since these runtime PM calls are normally the only thing in the prepare and unprepare callbacks and the autosuspend API transparently does the right thing on devices with autosuspend disabled factor all of this out into the core with a flag to enable the behaviour. Patch 1, 9, 10, 11, Reviewed-by: Stephen Warren swar...@nvidia.com -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] ARM: exynos5420: dt: add clock entries to watchdog node
On 07/24/2013 07:14 AM, Tomasz Figa wrote: On Wednesday 24 of July 2013 16:51:06 Sachin Kamat wrote: ... This is contrary to the fact that we disable everything by default in the top level dt files and only enable them as required in the board dts files. No, we don't disable everything. We disable things that require board specific setup or can't work without other support from board side. If there is some hardware disabled in SoC level dtsi that can work without any support from board side, then it is a BUG() and must be fixed. To illustrate the problem please consider that in the end, a dtb file will be fused into board ROM (or at most flash memory) and passed to the kernel by the bootloader. If you disable some hardware on DT level even if it can be physically used on the board, there will be no way to reenable it, if some user wanted to use it, because that would require editing the fused dtb. I believe some h/w will be disabled in dt only if it should not be used for whatever reason. If there is no reason then ofcourse they would be enabled IMHO. Yes. This is what I meant. However the reason must be valid - e.g. hardware does not allow such configuration, not like some very important manager decided that this board should not use this. Whatever be the case the choice of enabling or disabling should be done at the leaf node (at board level). No? It depends. For components that don't require any support from board side it can be globally enabled on SoC level. If a SoC component requires support from board side (like regulators, GPIOs, etc.) then it should be disabled on SoC level and enabled on board level only if all the dependencies are provided. I tend to agree with Tomasz. One note though: This is talking about the *.dts files, which may be different from the DTB that gets passed to the kernel. In other words, I don't think that the SoC or board .dtsi file (at least public versions that are hosted outside company-internal/downstream branches) have any business disabling HW based on policy or use-cases, rather than actual HW issues such as requiring other HW to support it that isn't present or doesn't work. However, I don't think anyone can influence what e.g.a bootloader does to the DTB before passing it to the kernel; it could add status=disabled to some nodes based on policy, and nobody in the Linux kernel has any absolute right to influence it, although there's sure a right to complain about it and point out why it's a bad idea. Equally, if somebody is creating a BSP (ick!) for a specific product's production flash image, rather than contributing to upstream SW support for that HW board, we probably don't have too much say in what they do. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On 07/19/2013 12:36 AM, Kishon Vijay Abraham I wrote: Hi, On Friday 19 July 2013 11:59 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote: Hi, On Friday 19 July 2013 11:13 AM, Greg KH wrote: On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote: + ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id); Your naming is odd, no phy anywhere in it? You rely on the sender to never send a duplicate name.id pair? Why not create your own ids based on the number of phys in the system, like almost all other classes and subsystems do? hmm.. some PHY drivers use the id they provide to perform some of their internal operation as in [1] (This is used only if a single PHY provider implements multiple PHYS). Probably I'll add an option like PLATFORM_DEVID_AUTO to give the PHY drivers an option to use auto id. [1] - http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html No, who cares about the id? No one outside of the phy core ever should, because you pass back the only pointer that they really do care about, if they need to do anything with the device. Use that, and then you can hmm.. ok. rip out all of the search for a phy by a string logic, as that's not Actually this is needed for non-dt boot case. In the case of dt boot, we use a phandle by which the controller can get a reference to the phy. But in the case of non-dt boot, the controller can get a reference to the phy only by label. I don't understand. They registered the phy, and got back a pointer to it. Why can't they save it in their local structure to use it again later if needed? They should never have to ask for the device, as the One is a *PHY provider* driver which is a driver for some PHY device. This will use phy_create to create the phy. The other is a *PHY consumer* driver which might be any controller driver (can be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference to the phy (by *phandle* in the case of dt boot and *label* in the case of non-dt boot). device id might be unknown if there are multiple devices in the system. I agree with you on the device id part. That need not be known to the PHY driver. How does a consumer know which label to use in a non-dt system if there are multiple PHYs in the system? That should be passed using platform data. I don't think that's right. That's just like passing clock names in platform data, which I believe is frowned upon. Instead, why not take the approach that e.g. regulators have taken? Each driver defines the names of the objects that it requires. There is a table (registered by a board file) that has lookup key (device name, regulator name), and the output is the regulator device (or global regulator name). This is almost the same way that DT works, except that in DT, the table is represented as properties in the DT. The DT binding defines the names of those properties, or the strings that must be in the foo-names properties, just like a driver in non-DT Linux is going to define the names it expects to be provided with. That way, you don't need platform data to define the names. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
On 05/18/2013 10:30 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: On 16:57 Sat 18 May , Tomasz Figa wrote: ... Personally I'd prefer a solution with separate property for each parameter, because it's much more flexible and allows shorter lines, making device tree sources more readable. we already discuss this on the ML the one property perline will endup with 100s of node if not 1000s so we all do agree we do not want this in the DT If you introduce s second level of nodes into the DT like the Tegra bindings do, that's really not an issue. For Tegra, each pinctrl state is a node. Within this, there are a bunch of child nodes, each of which applies to n pins, and sets up an arbitrary set of parameters; some nodes can set up mux functions, some can set up e.g. pull-up, etc. The same pin can be affected by multiple of these nodes. This all allows you to group together common settings and avoid duplication and having too many nodes. Then, client drivers' pincrl-0 properties just reference a single top-level state node, not each of the 10/100/... child nodes. Take a look at something like nodes state_i2xmux_* in arch/arm/boot/dts/tegra20-seaboard.dts. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
On 05/19/2013 03:17 AM, Jean-Christophe PLAGNIOL-VILLARD wrote: ... how a pin can not have mux? Well, if that's the way HW is designed, that's just the way it is. There are certainly pins on Tegra which don't have a mux in HW, but have some configuration options such as drive strength that can be configured. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
On 05/15/2013 12:29 PM, Linus Walleij wrote:
On Wed, May 15, 2013 at 6:44 PM, Doug Anderson diand...@google.com wrote:
...
Here's how I need to do things when I'm using just an interrupt:
pinctrl@1140 {
cyapa_irq: cyapa-irq {
samsung,pins = gpx1-2;
samsung,pin-function = 0xf;
samsung,pin-pud = 0;
samsung,pin-drv = 0;
};
};
trackpad {
reg = 0x67;
compatible = cypress,cyapa;
interrupts = 2 0;
interrupt-parent = gpx1;
pinctrl-names = default;
pinctrl-0 = cyapa_irq;
wakeup-source;
};
I don't really see much disadvantage here; the interrupt bindings
specify things related to interrupts and the pinctrl bindings specify
thing related to pin configuration.
If you want to condense the DT, I'd suggest using a the pinctrl hogging
feature, i.e. don't put pinctrl-* properties in the trackpad node, but
rather define a system-wide default pinctrl state in the pin
controller node itself. That configuration will be applied as soon as
the pin controller driver is registered. That'd be the same as the
above, with the following added:
pinctrl@1140 {
pinctrl-names = default;
pinctrl-0 = cyapa_irq;
};
except that the pinctrl-0 property would probably end up configuring a
whole bunch of basic pinctrl state rather than just that one interrupt pin.
I prefer to put all the static pinctrl configuration in the pinctrl hog,
and only the dynamic stuff in the individual device nodes.
I know LinusW won't like this suggestion much though:-)
I really wish I could add a 3rd number to the interrupt specifier for
pud and skip the pinctrl bit:
trackpad {
reg = 0x67;
compatible = cypress,cyapa;
interrupts = 2 0 0;
interrupt-parent = gpx1;
wakeup-source;
};
I don't like that myself, since it makes the interrupt binding (and I
assume you'd want to go back to the similar misuse in the GPIO binding)
also configure pinctrl-related stuff.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
On 05/15/2013 06:13 PM, Tomasz Figa wrote:
On Wednesday 15 of May 2013 16:55:37 Doug Anderson wrote:
Tomasz / Linus,
On Wed, May 15, 2013 at 3:06 PM, Tomasz Figa tomasz.f...@gmail.com
wrote:
Yes. I don't like the current way too much either, duplication being
one of the reasons.
Do you have any other ideas? It sounds like Linus didn't like my
suggestion and makes some good points...
I don't have anything interesting at the moment. It's a bit late now here
(2 AM), so I'm going to get some sleep first.
Also after reading Stephen's reply, I'm wondering if hogging wouldn't
solve the problem indeed. (It might have to be fixed on pinctrl-samsung
first, as last time I tried to use it, it caused some errors from pinctrl
core, but haven't time to track them down, as it wasn't anything important
at that time).
One issue I noticed with the DT fragments earlier in this thread. It
looks like hogs in the Samsung pinctrl bingings end up looking like:
pinctrl {
pina {
samsung,pins = PIN_A PIN_B PIN_C;
samsung,pin-function = 0xf;
samsung,pin-pud = 0;
...
};
pinp {
samsung,pins = PIN_P PIN_Q;
samsung,pin-function = 0xe;
samsung,pin-pud = 1;
...
};
pinx {
samsung,pins = PIN_X PIN_Y PIN_Z;
samsung,pin-function = 0xd;
samsung,pin-pud = 2;
...
};
pinctrl-names = default;
pinctrl-0 = pina pinp pinx;
};
That pinctrl-0 property could get rather large (hard to write/maintain,
unwieldy) if it needs to set up lots of different configurations. That's
why I made the equivalent Tegra bindings be:
pinctrl {
pins_default {
pina {
samsung,pins = PIN_A PIN_B PIN_C;
samsung,pin-function = 0xf;
samsung,pin-pud = 0;
...
};
pinp {
samsung,pins = PIN_P PIN_Q;
samsung,pin-function = 0xe;
samsung,pin-pud = 1;
...
};
pinx {
samsung,pins = PIN_X PIN_Y PIN_Z;
samsung,pin-function = 0xd;
samsung,pin-pud = 2;
...
};
};
pinctrl-names = default;
pinctrl-0 = pins_default;
};
The extra level within the pinctrl configuration node (pins_default
here) makes the pinctrl-0 property a lot easier to write, and the
advantage happens at every use-site that needs to configure different
subsets of the relevant pins in different ways.
If you're changing all the bindings anyway, introducing this extra level
might be something to think about.
I did try to explain my philosophy here when we all got together to
design the pinctrl bindings, but I obviously didn't explain it well
enough, or people didn't like it anyway.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 23/42] drivers/spi: don't check resource with devm_ioremap_resource
On 05/10/2013 02:17 AM, Wolfram Sang wrote:
devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.
diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!r) {
- dev_err(pdev-dev, No IO memory resource\n);
- ret = -ENODEV;
- goto exit_free_master;
- }
tspi-phys = r-start;
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
The tspi-phy assignment is broken there now.
diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!r) {
- dev_err(pdev-dev, No IO memory resource\n);
- ret = -ENODEV;
- goto exit_free_master;
- }
tspi-phys = r-start;
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
Same here.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: GENERIC_GPIO considered deprecated
On 04/08/2013 01:31 AM, Kukjin Kim wrote: Alexandre Courbot wrote: On Wed, Apr 3, 2013 at 5:35 PM, Kukjin Kim kgene@samsung.com wrote: could you amend the patches that adds them such as they get changed into select ARCH_REQUIRE_GPIOLIB instead? You can grep for select I can do it for my tree but the branch already included in arm-soc tree so I think, it should be fixed with another patch. And GENERIC_GPIO in arch/arm to find the offending lines. We are removing GENERIC_GPIO and this work cannot be merged until you do this since it would break ARM builds. Thanks! So how about following? If you are OK, let me take into samsung tree. 88 From: Kukjin Kim kgene@samsung.com Subject: [PATCH] ARM: SAMSUNG: change GENERIC_GPIO to ARCH_REQUIRE_GPIOLIB When I applied regarding samsung-time patches, the select GENERIC_GPIO has been added wrong, so this patch fixes that. And since the GENERIC_GPIO in arch/arm/ will be gone away, this adds ARCH_REQUIRE_GPIOLIB for S3C24XX and S5PC100 instead. Reported-by: Alexandre Courbot gnu...@gmail.com Cc: Romain Naour romain.na...@openwide.fr Signed-off-by: Kukjin Kim kgene@samsung.com --- arch/arm/Kconfig |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 46fcfa8..a239c7e 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -770,10 +770,10 @@ config ARCH_SA1100 config ARCH_S3C24XX bool Samsung S3C24XX SoCs select ARCH_HAS_CPUFREQ + select ARCH_REQUIRE_GPIOLIB select CLKDEV_LOOKUP select CLKSRC_MMIO select GENERIC_CLOCKEVENTS - select GENERIC_GPIO select HAVE_CLK select HAVE_S3C2410_I2C if I2C select HAVE_S3C2410_WATCHDOG if WATCHDOG @@ -828,11 +828,11 @@ config ARCH_S5P64X0 config ARCH_S5PC100 bool Samsung S5PC100 + select ARCH_REQUIRE_GPIOLIB select CLKDEV_LOOKUP select CLKSRC_MMIO select CPU_V7 select GENERIC_CLOCKEVENTS - select GENERIC_GPIO select HAVE_CLK select HAVE_S3C2410_I2C if I2C select HAVE_S3C2410_WATCHDOG if WATCHDOG -- 1.7.10.4 Should do the trick, if we can make sure that your tree is merged prior to my patches. I'm not sure but I think, arm-soc tree should be merged into mainline before others... Can you put it into your tree for 3.10? I did, so it should be fine. Thanks. - Kukjin You may want to discuss how to handle this dependency with the arm-soc maintainers (CC'd). -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 8/9] ARM: tegra: Provide regulator to pwm-backlight
On 03/19/2013 12:59 PM, Andrew Chew wrote: The pwm-backlight driver now takes a mandatory regulator that is gotten during driver probe. Initialize a dummy regulator to satisfy this requirement. I assume these patches will get merged through the PWM tree? If so, Acked-by: Stephen Warren swar...@nvidia.com -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 9/9] pwm_bl: Add mandatory backlight enable regulator
On 03/19/2013 12:59 PM, Andrew Chew wrote: Many backlights need to be explicitly enabled. Typically, this is done with a GPIO. For flexibility, we generalize the enable mechanism to a regulator. If an enable regulator is not needed, then a dummy regulator can be given to the backlight driver. If a GPIO is used to enable the backlight, then a fixed regulator can be instantiated to control the GPIO. The backlight enable regulator can be specified in the device tree node for the backlight, or can be done with legacy board setup code in the usual way. diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt index 1e4fc72..7e2e089 100644 --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt @@ -10,6 +10,11 @@ Required properties: last value in the array represents a 100% duty cycle (brightest). - default-brightness-level: the default brightness level (index into the array defined by the brightness-levels property) + - enable-supply: A phandle to the regulator device tree node. This + regulator will be turned on and off as the pwm is enabled and disabled. + Many backlights are enabled via a GPIO. In this case, we instantiate + a fixed regulator and give that to enable-supply. If a regulator + is not needed, then provide a dummy fixed regulator. enable doesn't seem like the right name here; if this really is an enable input, then it's not a regulator. If you're calling it enable because the regulator is usually controlled by a GPIO that enables it, then what you really have is a regulator that provides power to the backlight, and the method that you enable that regulator is irrelevant. Put another way, wouldn't power be a better name, thus making the property power-supply? Although that property name migth be considered to have some negative correlation with other concepts, so if people object to that, perhaps e.g. vdd-supply? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 6/9] ARM: mxs: Provide regulator to pwm-backlight
On 03/19/2013 03:27 PM, Marek Vasut wrote: Dear Andrew Chew, The pwm-backlight driver now takes a mandatory regulator that is gotten during driver probe. Initialize a dummy regulator to satisfy this requirement. Signed-off-by: Andrew Chew ac...@nvidia.com Do we really need a mandatory regulator? Why can't it be optional? IIRC, the previous advice I've seen is that if a device (driver) uses a regulator, it must /require/ a regulator, and if a particular board doesn't actually have a SW-controlled regulator, then a fixed- or dummy- regulator should be provided to satisfy this requirement. CC'ing Mark Brown to make sure I really do Recall Correctly. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: add interrupt-names property to get interrupt resource by name
On 03/18/2013 09:50 AM, Rob Herring wrote: On 03/13/2013 05:42 PM, Sylwester Nawrocki wrote: Rob, On 03/13/2013 03:39 PM, Rob Herring wrote: I fail to see what the hack is. The order of interrupt properties must be defined by the binding. interrupt-names is auxiliary data and must not be required by an OS. Is that true for all foo-names properties, or only for interrupt-names? I was under the impression that foo-names was specifically invented so that the order of the entries didn't matter, and instead they could be requested by name. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: add interrupt-names property to get interrupt resource by name
On 03/18/2013 04:27 PM, Rob Herring wrote: On 03/18/2013 01:11 PM, Stephen Warren wrote: On 03/18/2013 09:50 AM, Rob Herring wrote: On 03/13/2013 05:42 PM, Sylwester Nawrocki wrote: Rob, On 03/13/2013 03:39 PM, Rob Herring wrote: I fail to see what the hack is. The order of interrupt properties must be defined by the binding. interrupt-names is auxiliary data and must not be required by an OS. Is that true for all foo-names properties, or only for interrupt-names? I was under the impression that foo-names was specifically invented so that the order of the entries didn't matter, and instead they could be requested by name. I think it depends on the specific name the property is tied too. For interrupt and reg properties which have a long history and convention, the order should be defined. IIRC, this was Grant's position too. For new bindings, perhaps we can be more lenient. OK, that makes sense for interrupts/reg. Can we decide that clock-namess are new-style and that order is not significant? I guess gpio-names too? I guess this should be documented in whatever binding describes the core interrupts/reg-names/gpio-names/clock-names/dma-names properties. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: State of pinctrl and exynos5250?
On 03/07/2013 09:27 AM, Doug Anderson wrote: +the proper address for Will. Sorry... On Thu, Mar 7, 2013 at 8:13 AM, Doug Anderson diand...@chromium.org mailto:diand...@chromium.org wrote: Linus, +dw_mmc folks and Stephen Warren : for context here, we are discussing device tree bindings for pinmux for dw_mmc. The issue at hand is whether they belong under the slot node or under the top-level device node. There's no need for dynamic pin-muxing for MMC AFAIK, so I'd expect a single pinctrl state default to exist that covers any/all requirements of both slots' pinmux configuration. On Wed, Mar 6, 2013 at 11:57 PM, Linus Walleij linus.wall...@linaro.org mailto:linus.wall...@linaro.org wrote: I don't quite understand the above. Is it such that there is one device, with two slots, and in the device model you have represented this two-slot device with a single struct device? Yes, that's the issue. That's dw_mmc that has been in the kernel for a bit of time now (looks like Jan 2011) and has had a single struct device for as long as I've been looking at it. Relevant links for convenience: * https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/mmc/host/dw_mmc.c * https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt * https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos5250.dtsi#n243 * https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/cros5250-common.dtsi#n92 Have you considered just registering one device for each slot? That would make things quite a lot simpler, just a single pinctrl handle per device, right? I don't know why the original decision was made to just have one struct device. ...that would be a pretty big code change at this point, I think. ...I think the most important issue at hand is the device tree bindings for pinmux on this device. It sounds like you are in agreement that the best thing is to have a pinmux specified per-slot. If the code is a bit awkward now (due to not having a struct device per slot) then that's just something we have to live with. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 02/10] s5p-fimc: Add device tree support for FIMC devices
On 02/12/2013 03:39 PM, Sylwester Nawrocki wrote: On 02/11/2013 10:50 PM, Stephen Warren wrote: On 02/09/2013 03:29 PM, Sylwester Nawrocki wrote: On 02/09/2013 01:32 AM, Stephen Warren wrote: On 02/08/2013 05:05 PM, Sylwester Nawrocki wrote: On 02/09/2013 12:21 AM, Stephen Warren wrote: On 02/08/2013 04:16 PM, Sylwester Nawrocki wrote: On 02/07/2013 12:40 AM, Stephen Warren wrote: diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt +Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC) +-- ... +For every fimc node a numbered alias should be present in the aliases node. +Aliases are of the form fimcn, wheren is an integer (0...N) specifying +the IP's instance index. ... Different compatible values might not work, when for example there are 3 IPs out of 4 of one type and the fourth one of another type. It wouldn't even by really different types, just quirks/little differences between them, e.g. no data path routed to one of other IPs. I was thinking of using feature-/quirk-oriented properties. For example, if there's a port on 3 of the 4 devices to connect to some other IP block, simply include a boolean property to indicate whether that port is present. It would be in 3 of the nodes but not the 4th. Yes, I could add several properties corresponding to all members of this [3] data structure. But still it is needed to clearly identify the IP block in a set of the hardware instances. Why? What decisions will be made based on the identify of the IP block instance that wouldn't be covered by DT properties that describe which features/bugs/... the IP block instance has? The whole subsystem topology is exposed to user space through the Media Controller API. OK, stable user-visible names are a reasonable use for device tree. I still don't think you should use those user-visible IDs for making any other kind of decision though. Then to connect e.g. MIPI-CSIS.0 to FIMC.2 at run time an index of the MIPI-CSIS needs to be written to the FIMC.2 data input control register. Even though MIPI-CSIS.N are same in terms of hardware structure they still need to be distinguished as separate instances. Oh, so you're using the alias ID as the value to write into the FIMC.2 register for that. I'm not 100% familiar with aliases, but they seem like a more user-oriented naming thing to me, whereas values for hooking up intra-SoC routing are an unrelated namespace semantically, even if the values happen to line up right now. Perhaps rather than a Boolean property I mentioned above, use a custom property to indicate the ID that the FIMC.2 object knows the MIPI-CSIS.0 object as? While this seems That could be 'reg' property in the MIPI-CSIS.0 'port' subnode that links it to the image sensor node ([4], line 165). Because MIPI-CSIS IP blocks are immutably connected to the SoC camera physical MIPI CSI-2 interfaces, and the physical camera ports have fixed assignment to the MIPI-CSIS devices.. This way we could drop alias ID for the MIPI-CSIS nodes. And their instance index that is required for the top level driver which exposes topology and the routing capabilities to user space could be restored from the reg property value by subtracting a fixed offset. I suppose that would work. It feels a little indirect, and I think means that the driver needs to go find some child node defining its end of some link, then find the node representing the other end of the link, then read properties out of that other node to find the value. That seems a little unusual, but I guess it would work. I'm not sure of the long-term implications of doing that kind of thing. You'd want to run the idea past some DT maintainers/experts. It's a bit simpler than that. We would need only to look for the reg property in a local port subnode. MIPI-CSIS correspond to physical MIPI CSI-2 bus interface of an SoC, hence it has to have specific reg values that identify each camera input interface. Oh I see. I guess if a device is using its own node to determine its own identify, that's reasonable. I thought you were talking about a situation like: FIMC -- XXX where FIMC wanted to determine what ID XXX knew that particular FIMC as. I can see aliases used in bindings of multiple devices: uart, spi, sound interfaces, gpio, ... And all bindings seem to impose some rules on how their aliases are created. Do you have specific examples? I really don't think the bindings should be dictating the alias values. I just grepped through the existing bindings documentation: ... I think correctly numbered in the above statements means there are some specific rules on how the aliases are created, however those seem not clearly communicated. A binding specifying that an alias must (or even should) exist for each node seems odd to me. In the absence of an explicit
Re: [PATCH v4 02/10] s5p-fimc: Add device tree support for FIMC devices
On 02/09/2013 03:29 PM, Sylwester Nawrocki wrote: On 02/09/2013 01:32 AM, Stephen Warren wrote: On 02/08/2013 05:05 PM, Sylwester Nawrocki wrote: On 02/09/2013 12:21 AM, Stephen Warren wrote: On 02/08/2013 04:16 PM, Sylwester Nawrocki wrote: On 02/07/2013 12:40 AM, Stephen Warren wrote: diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt +Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC) +-- ... +For every fimc node a numbered alias should be present in the aliases node. +Aliases are of the form fimcn, wherenis an integer (0...N) specifying +the IP's instance index. ... Different compatible values might not work, when for example there are 3 IPs out of 4 of one type and the fourth one of another type. It wouldn't even by really different types, just quirks/little differences between them, e.g. no data path routed to one of other IPs. I was thinking of using feature-/quirk-oriented properties. For example, if there's a port on 3 of the 4 devices to connect to some other IP block, simply include a boolean property to indicate whether that port is present. It would be in 3 of the nodes but not the 4th. Yes, I could add several properties corresponding to all members of this [3] data structure. But still it is needed to clearly identify the IP block in a set of the hardware instances. Why? What decisions will be made based on the identify of the IP block instance that wouldn't be covered by DT properties that describe which features/bugs/... the IP block instance has? Then to connect e.g. MIPI-CSIS.0 to FIMC.2 at run time an index of the MIPI-CSIS needs to be written to the FIMC.2 data input control register. Even though MIPI-CSIS.N are same in terms of hardware structure they still need to be distinguished as separate instances. Oh, so you're using the alias ID as the value to write into the FIMC.2 register for that. I'm not 100% familiar with aliases, but they seem like a more user-oriented naming thing to me, whereas values for hooking up intra-SoC routing are an unrelated namespace semantically, even if the values happen to line up right now. Perhaps rather than a Boolean property I mentioned above, use a custom property to indicate the ID that the FIMC.2 object knows the MIPI-CSIS.0 object as? While this seems That could be 'reg' property in the MIPI-CSIS.0 'port' subnode that links it to the image sensor node ([4], line 165). Because MIPI-CSIS IP blocks are immutably connected to the SoC camera physical MIPI CSI-2 interfaces, and the physical camera ports have fixed assignment to the MIPI-CSIS devices.. This way we could drop alias ID for the MIPI-CSIS nodes. And their instance index that is required for the top level driver which exposes topology and the routing capabilities to user space could be restored from the reg property value by subtracting a fixed offset. I suppose that would work. It feels a little indirect, and I think means that the driver needs to go find some child node defining its end of some link, then find the node representing the other end of the link, then read properties out of that other node to find the value. That seems a little unusual, but I guess it would work. I'm not sure of the long-term implications of doing that kind of thing. You'd want to run the idea past some DT maintainers/experts. ... I can see aliases used in bindings of multiple devices: uart, spi, sound interfaces, gpio, ... And all bindings seem to impose some rules on how their aliases are created. Do you have specific examples? I really don't think the bindings should be dictating the alias values. After all, what happens in some later SoC where you have two different types of module that feed into the common module, such that type A sources have IDs 0..3 in the common module, and type B sources have IDs 4..7 in the common module - you wouldn't want to require alias ISs 4..7 for the type B DT nodes. There is no need to write alias ID directly into registers, and actually it doesn't really happen. But we need to know that, for example camera A is connected to physical MIPI CSI-2 channel 0 and to capture video with DMA engine of FIMC.2 we need to set FIMC.2 input register to link it to MIPI-CSIS 0. OK, so the IDs are selecting which register to write, or which mux settings to access. That's pretty much semantically the same thing. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 02/10] s5p-fimc: Add device tree support for FIMC devices
On 02/08/2013 04:16 PM, Sylwester Nawrocki wrote: On 02/07/2013 12:40 AM, Stephen Warren wrote: diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt +Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC) +-- ... +For every fimc node a numbered alias should be present in the aliases node. +Aliases are of the form fimcn, wheren is an integer (0...N) specifying +the IP's instance index. Why? Isn't it up to the DT author whether they care if each fimc node is assigned a specific identification v.s. whether identification is assigned automatically? There are at least three different kinds of IPs that come in multiple instances in an SoC. To activate data links between them each instance needs to be clearly identified. There are also differences between instances of same device. Hence it's important these aliases don't have random values. Some more details about the SoC can be found at [1]. The aliases are also already used in the Exynos5 GScaler bindings [2] in a similar way. Hmmm. I'd expect explicit DT properties to represent the instance-specific configuration, or even different compatible values. Relying on the alias ID seems rather indirect; what if in e.g. Exynos6/... the mapping from instance/alias ID to feature set changes. With explicit DT properties, that'd just be a .dts change, whereas by requiring alias IDs now, you'd need a driver change to support this. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 01/10] s5p-csis: Add device tree support
On 02/08/2013 03:29 PM, Sylwester Nawrocki wrote: On 02/07/2013 12:36 AM, Stephen Warren wrote: On 02/01/2013 12:09 PM, Sylwester Nawrocki wrote: s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC device. This patch support for binding the driver to the MIPI-CSIS devices instantiated from device tree and for parsing all SoC and board specific properties. diff --git a/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt b/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt +Optional properties: + +- clock-frequency : The IP's main (system bus) clock frequency in Hz, default +value when this property is not specified is 166 MHz; Shouldn't this be a clocks property, so that the driver can call clk_get(), clk_prepare_enable(), clk_get_rate(), etc. on it? Hi Stephen, Thanks for your review! I also use clocks and clock-names property, but didn't specify it here, because the Exynos SoCs clocks driver is not in the mainline yet. I'm a bit sympathetic to this, since I've been trying to push Tegra towards the common clock framework and describing clock connectivity in DT, before adding new drivers that need clocks, specifically to avoid this kind of situation. The problem here is that if this gets merged now, then the clock driver comes along later, you'll have to change this binding definition to account for it, and DT bindings aren't supposed to be changed... Do you have any clock driver at all upstream yet? Or, could you add a dummy clock driver to satisfy the driver's clkl_get() needs? If you do, you can always set up some AUXDATA so that clk_get() works for your driver right now, and then remove that once the complete clock driver is in place with full device tree support. That's how we've ended up handling this for Tegra drivers. Anyway, this is all mainly food-for-thought rather than an objection to the patch; I'd leave that to Grant/Rob if applicable. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 02/10] s5p-fimc: Add device tree support for FIMC devices
On 02/08/2013 05:05 PM, Sylwester Nawrocki wrote: On 02/09/2013 12:21 AM, Stephen Warren wrote: On 02/08/2013 04:16 PM, Sylwester Nawrocki wrote: On 02/07/2013 12:40 AM, Stephen Warren wrote: diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt +Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC) +-- ... +For every fimc node a numbered alias should be present in the aliases node. +Aliases are of the form fimcn, wheren is an integer (0...N) specifying +the IP's instance index. Why? Isn't it up to the DT author whether they care if each fimc node is assigned a specific identification v.s. whether identification is assigned automatically? There are at least three different kinds of IPs that come in multiple instances in an SoC. To activate data links between them each instance needs to be clearly identified. There are also differences between instances of same device. Hence it's important these aliases don't have random values. Some more details about the SoC can be found at [1]. The aliases are also already used in the Exynos5 GScaler bindings [2] in a similar way. Hmmm. I'd expect explicit DT properties to represent the instance-specific configuration, or even different compatible values. Relying on the alias ID seems rather indirect; what if in e.g. Exynos6/... the mapping from instance/alias ID to feature set changes. With explicit DT properties, that'd just be a .dts change, whereas by requiring alias IDs now, you'd need a driver change to support this. In the initial version of this patch series I used cell-index property, but then Grant pointed out in some other mail thread it should be avoided. Hence I used the node aliases. To me, using cell-index is semantically equivalent to using the alias ID. Different compatible values might not work, when for example there are 3 IPs out of 4 of one type and the fourth one of another type. It wouldn't even by really different types, just quirks/little differences between them, e.g. no data path routed to one of other IPs. I was thinking of using feature-/quirk-oriented properties. For example, if there's a port on 3 of the 4 devices to connect to some other IP block, simply include a boolean property to indicate whether that port is present. It would be in 3 of the nodes but not the 4th. Then to connect e.g. MIPI-CSIS.0 to FIMC.2 at run time an index of the MIPI-CSIS needs to be written to the FIMC.2 data input control register. Even though MIPI-CSIS.N are same in terms of hardware structure they still need to be distinguished as separate instances. Oh, so you're using the alias ID as the value to write into the FIMC.2 register for that. I'm not 100% familiar with aliases, but they seem like a more user-oriented naming thing to me, whereas values for hooking up intra-SoC routing are an unrelated namespace semantically, even if the values happen to line up right now. Perhaps rather than a Boolean property I mentioned above, use a custom property to indicate the ID that the FIMC.2 object knows the MIPI-CSIS.0 object as? While this seems similar to using cell-index, my *guess* is that Grant's objection to using cell-index was more based on re-using cell-index for something other than its intended purpose than pushing you to use an alias ID rather than a property. After all, what happens in some later SoC where you have two different types of module that feed into the common module, such that type A sources have IDs 0..3 in the common module, and type B sources have IDs 4..7 in the common module - you wouldn't want to require alias ISs 4..7 for the type B DT nodes. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 01/10] s5p-csis: Add device tree support
On 02/01/2013 12:09 PM, Sylwester Nawrocki wrote: s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC device. This patch support for binding the driver to the MIPI-CSIS devices instantiated from device tree and for parsing all SoC and board specific properties. diff --git a/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt b/Documentation/devicetree/bindings/media/soc/samsung-mipi-csis.txt +Optional properties: + +- clock-frequency : The IP's main (system bus) clock frequency in Hz, default + value when this property is not specified is 166 MHz; Shouldn't this be a clocks property, so that the driver can call clk_get(), clk_prepare_enable(), clk_get_rate(), etc. on it? Other than that this binding seems fine to me at a quick glance. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 02/10] s5p-fimc: Add device tree support for FIMC devices
On 02/01/2013 12:09 PM, Sylwester Nawrocki wrote: This patch adds support for FIMC devices instantiated from devicetree for S5PV210 and Exynos4 SoCs. The FIMC IP features include colorspace conversion and scaling (mem-to-mem) and parallel/MIPI CSI2 bus video capture interface. Multiple SoC revisions specific parameters are defined statically in the driver and are used for both dt and non-dt. The driver's static data is selected based on the compatible property. Previously the platform device name was used to match driver data and a specific SoC/IP version. Aliases are used to determine an index of the IP which is essential for linking FIMC IP with other entities, like MIPI-CSIS (the MIPI CSI-2 bus frontend) or FIMC-LITE and FIMC-IS ISP. diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt +Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC) +-- + +The Exynos Camera subsystem comprises of multiple sub-devices that are +represented by separate platform devices. Some of the IPs come in different platform devices is a rather Linux-centric term, and DT bindings should be OS-agnostic. Perhaps use device tree nodes here? +variants across the SoC revisions (FIMC) and some remain mostly unchanged +(MIPI CSIS, FIMC-LITE). + +All those sub-subdevices are defined as parent nodes of the common device s/parent nodes/child node/ I think? +For every fimc node a numbered alias should be present in the aliases node. +Aliases are of the form fimcn, where n is an integer (0...N) specifying +the IP's instance index. Why? Isn't it up to the DT author whether they care if each fimc node is assigned a specific identification v.s. whether identification is assigned automatically? +Optional properties + + - clock-frequency - maximum FIMC local clock (LCLK) frequency Again, I'd expect a clocks property here instead. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 05/10] s5p-fimc: Add device tree based sensors registration
On 02/01/2013 12:09 PM, Sylwester Nawrocki wrote: The sensor (I2C and/or SPI client) devices are instantiated by their corresponding control bus drivers. Since the I2C client's master clock is often provided by a video bus receiver (host interface) or other than I2C/SPI controller device, the drivers of those client devices are not accessing hardware in their driver's probe() callback. Instead, after enabling clock, the host driver calls back into a sub-device when it wants to activate them. This pattern is used by some in-tree drivers and this patch also uses it for DT case. This patch is intended as a first step for adding device tree support to the S5P/Exynos SoC camera drivers. The second one is adding support for asynchronous sub-devices registration and clock control from sub-device driver level. The bindings shall not change when asynchronous probing support is added. diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt +The sensor device nodes should be added as their control bus controller I think as should be to? +(e.g. I2C0) child nodes and linked to a port node in the csis or parallel-ports +node, using common the common video interfaces bindings, i.e. port/endpoint +node pairs. The implementation of this binding requires clock-frequency +property to be present in the sensor device nodes. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 06/10] s5p-fimc: Use pinctrl API for camera ports configuration
On 02/01/2013 12:09 PM, Sylwester Nawrocki wrote: Before the camera ports can be used the pinmux needs to be configured properly. This patch adds a function to set the camera ports pinctrl to a default state within the media driver's probe(). The camera port(s) are then configured for the video bus operation. diff --git a/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt b/Documentation/devicetree/bindings/media/soc/samsung-fimc.txt +- pinctrl-names: pinctrl names for camera port pinmux control, at least + default needs to be specified. +- pinctrl-0...N : pinctrl properties corresponding to pinctrl-names + A reference to the binding document describing the pin control bindings would be appropriate here. Given that reference, I'm not sure if spelling out the property names makes sense since it feels a little like duplication; an alternative might be simply: The pinctrl bindings defined in ../../../pinctrl/pinctrl-bindings.txt must be used to define a pinctrl state named default. However, this isn't a big deal; it's fine either way. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] drm/exynos: Get HDMI version from device tree
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which
registers/functions to use in the HDMI driver. Also changes the
references from v13 to 4210 and v14 to 4212 to reflect the IP
block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF
static struct of_device_id hdmi_match_types[] = {
{
- .compatible = samsung,exynos5-hdmi,
- .data = (void *)HDMI_TYPE14,
+ .compatible = samsung,exynos4-hdmi,
}, {
/* end node */
}
Why not fill in all the base compatible values there (I think you need
this anyway so that DTs don't all have to be compatible with
samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS*
values, then ...
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
+
+ if (of_device_is_compatible(dev-of_node, samsung,exynos4210-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS4210;
+ if (of_device_is_compatible(dev-of_node, samsung,exynos4212-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS4212;
+ if (of_device_is_compatible(dev-of_node, samsung,exynos5250-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS5250;
Instead of that, do roughly:
match = of_match_device(hdmi_match_types, pdev-dev);
if (match)
hdata-version |= (int)match-data;
That way, it's all table-based. Any future additions to
hdmi_match_types[] won't require another if statement to be added to
probe().
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] drm/exynos: Get HDMI version from device tree
On 02/05/2013 05:37 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren swar...@wwwdotorg.org wrote:
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which
registers/functions to use in the HDMI driver. Also changes the
references from v13 to 4210 and v14 to 4212 to reflect the IP
block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF
static struct of_device_id hdmi_match_types[] = {
{
- .compatible = samsung,exynos5-hdmi,
- .data = (void *)HDMI_TYPE14,
+ .compatible = samsung,exynos4-hdmi,
}, {
/* end node */
}
Why not fill in all the base compatible values there (I think you need
this anyway so that DTs don't all have to be compatible with
samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS*
values, then ...
At the moment, all DTs have to be compatible with exynos4-hdmi since
it provides the base for the current driver. The driver uses 4210 and
4212 to differentiate between different register addresses and
features, but most things are just exynos4-hdmi compatible.
The DT nodes should include only the compatible values that the HW is
actually compatible with. If the HW isn't compatible with exynos4-hdmi,
that value shouldn't be in the compatible property, but instead whatever
the base value that the HW really is compatible with. The driver can
support multiple base compatible values from this table.
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
+
+ if (of_device_is_compatible(dev-of_node, samsung,exynos4210-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS4210;
+ if (of_device_is_compatible(dev-of_node, samsung,exynos4212-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS4212;
+ if (of_device_is_compatible(dev-of_node, samsung,exynos5250-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS5250;
Instead of that, do roughly:
match = of_match_device(hdmi_match_types, pdev-dev);
if (match)
hdata-version |= (int)match-data;
That way, it's all table-based. Any future additions to
hdmi_match_types[] won't require another if statement to be added to
probe().
I don't think it's that easy. of_match_device returns the first match
from the device table, so I'd still need to iterate through the
matches. I could still break this out into a table, but I don't think
of_match_device is the right way to probe it.
You shouldn't have to iterate over multiple matches. of_match_device()
is supposed to return the match for the first entry in the compatible
property, then if there was no match, move on to looking at the next
entry in the compatible property, etc. In practice, I think it's still
not implemented quite correctly for this, but you can make it work by
putting the newest compatible value first in the match table.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] drm/exynos: Get HDMI version from device tree
On 02/05/2013 05:56 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:42 PM, Stephen Warren swar...@wwwdotorg.org wrote:
On 02/05/2013 05:37 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren swar...@wwwdotorg.org
wrote:
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which
registers/functions to use in the HDMI driver. Also changes the
references from v13 to 4210 and v14 to 4212 to reflect the IP
block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF
static struct of_device_id hdmi_match_types[] = {
{
- .compatible = samsung,exynos5-hdmi,
- .data = (void *)HDMI_TYPE14,
+ .compatible = samsung,exynos4-hdmi,
}, {
/* end node */
}
Why not fill in all the base compatible values there (I think you need
this anyway so that DTs don't all have to be compatible with
samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS*
values, then ...
At the moment, all DTs have to be compatible with exynos4-hdmi since
it provides the base for the current driver. The driver uses 4210 and
4212 to differentiate between different register addresses and
features, but most things are just exynos4-hdmi compatible.
The DT nodes should include only the compatible values that the HW is
actually compatible with. If the HW isn't compatible with exynos4-hdmi,
that value shouldn't be in the compatible property, but instead whatever
the base value that the HW really is compatible with. The driver can
support multiple base compatible values from this table.
All devices that use this driver are compatible, at some level, with
exynos4-hdmi, so I think its usage is correct here.
But can a driver that only knows about the original exynos4-hdmi operate
any of the HW correctly without any additional knowledge? If not, the
new HW isn't compatible with the old.
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device
*pdev)
+
+ if (of_device_is_compatible(dev-of_node,
samsung,exynos4210-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS4210;
+ if (of_device_is_compatible(dev-of_node,
samsung,exynos4212-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS4212;
+ if (of_device_is_compatible(dev-of_node,
samsung,exynos5250-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS5250;
Instead of that, do roughly:
match = of_match_device(hdmi_match_types, pdev-dev);
if (match)
hdata-version |= (int)match-data;
That way, it's all table-based. Any future additions to
hdmi_match_types[] won't require another if statement to be added to
probe().
I don't think it's that easy. of_match_device returns the first match
from the device table, so I'd still need to iterate through the
matches. I could still break this out into a table, but I don't think
of_match_device is the right way to probe it.
You shouldn't have to iterate over multiple matches. of_match_device()
is supposed to return the match for the first entry in the compatible
property, then if there was no match, move on to looking at the next
entry in the compatible property, etc. In practice, I think it's still
not implemented quite correctly for this, but you can make it work by
putting the newest compatible value first in the match table.
I think the only way that works is if you hardcode the compatible
versions in the driver, like this:
static struct of_device_id hdmi_match_types[] = {
{
.compatible = samsung,exynos5250-hdmi,
.data = (void *)(HDMI_VER_EXYNOS5250 | HDMI_VER_EXYNOS4212);
}, {
.compatible = samsung,exynos4212-hdmi,
.data = (void *)HDMI_VER_EXYNOS4212;
}, {
.compatible = samsung,exynos4210-hdmi,
.data = (void *)HDMI_VER_EXYNOS4210;
}, {
/* end node */
}
};
In that case, it eliminates the benefit of using device tree to
determine the compatible bits. I hope I'm just being thick and missing
something.
The table above looks /almost/ exactly correct to me, although I'm
unsure why samsung,exynos5250-hdmi has *two* version values; surely
there's a 1:1 mapping between the compatible values and the HW
compatibility they represent? That's certainly the intent.
Perhaps the two values think is because you're representing quirks or
features rather than HW versions? Compatible is supposed to represent HW
versions. Each HW version has a set of features/quirks, and multiple HW
versions can have intersecting sets of features/quirks. However,
features/quirks aren't HW versions.
--
To unsubscribe from this list: send
Re: [PATCH v4 4/7] ARM: Exynos: allow dt based discovery of mct controller using clocksource_of_init
On 01/28/2013 01:57 PM, Stephen Warren wrote: On 01/21/2013 03:02 AM, Thomas Abraham wrote: Add entries to __clksrc_of_table so that Exynos MCT controller is discoverable using call to clocksource_of_init. With this change, it would be appropriate to rename the function 'exynos4_timer_init' as 'mct_init' since it aptly describes this function. Additionally, the 'init_time' callback of all machine descriptors for exynos platforms that were previously set to 'exynos4_timer_init' are now set to either 'mct_init' or 'clocksource_of_init'. diff --git a/arch/arm/mach-exynos/mct.c b/arch/arm/mach-exynos/mct.c +#ifdef CONFIG_CLKSRC_OF +CLOCKSOURCE_OF_DECLARE(exynos4210, samsung,exynos4210-mct, mct_init) +CLOCKSOURCE_OF_DECLARE(exynos4412, samsung,exynos4412-mct, mct_init) +#endif I suggested in another review (IIRC for a different SoC) that CLOCKSOURCE_OF_DECLARE() should always be declared so you don't need that ifdef. Would you care to send a patch to add to arm-soc's timer/cleanup branch to do that? If not, let me know and I can. I just sent a patch for this. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/7] ARM: Exynos: allow dt based discovery of mct controller using clocksource_of_init
On 01/21/2013 03:02 AM, Thomas Abraham wrote: Add entries to __clksrc_of_table so that Exynos MCT controller is discoverable using call to clocksource_of_init. With this change, it would be appropriate to rename the function 'exynos4_timer_init' as 'mct_init' since it aptly describes this function. Additionally, the 'init_time' callback of all machine descriptors for exynos platforms that were previously set to 'exynos4_timer_init' are now set to either 'mct_init' or 'clocksource_of_init'. diff --git a/arch/arm/mach-exynos/mct.c b/arch/arm/mach-exynos/mct.c +#ifdef CONFIG_CLKSRC_OF +CLOCKSOURCE_OF_DECLARE(exynos4210, samsung,exynos4210-mct, mct_init) +CLOCKSOURCE_OF_DECLARE(exynos4412, samsung,exynos4412-mct, mct_init) +#endif I suggested in another review (IIRC for a different SoC) that CLOCKSOURCE_OF_DECLARE() should always be declared so you don't need that ifdef. Would you care to send a patch to add to arm-soc's timer/cleanup branch to do that? If not, let me know and I can. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/7] ARM: Exynos: add device tree support for MCT controller driver
On 01/21/2013 03:02 AM, Thomas Abraham wrote: Allow the MCT controller base address and interrupts to be obtained from device tree and remove unused static definitions of these. The non-dt support for Exynos5250 is removed but retained for Exynos4210 based platforms. Patches 3 and later in this series, fairly quickly, Reviewed-by: Stephen Warren swar...@nvidia.com Sorry for the slow review. I'm not 100% sure if I like Mark's #global-interrupts suggestion or not, but I'd be fine with the binding either way, so choose as you see fit. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: dts: Fix compatible value of pinctrl module on EXYNOS5440
On 01/17/2013 01:34 AM, Linus Walleij wrote:
On Thu, Jan 3, 2013 at 1:20 AM, Kukjin Kim kgene@samsung.com wrote:
From: Thomas Abraham thomas...@samsung.com
Fix the incorrect compatible property value of pin-controller module
EXYNOS5440 SoC.
(...)
pinctrl {
- compatible = samsung,pinctrl-exynos5440;
+ compatible = samsung,exynos5440-pinctrl;
reg = 0xE 0x1000;
interrupt-controller;
#interrupt-cells = 2;
diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
index 953737d..cac4b45 100644
--- a/drivers/gpio/gpio-samsung.c
+++ b/drivers/gpio/gpio-samsung.c
@@ -3026,7 +3026,7 @@ static __init int samsung_gpiolib_init(void)
static const struct of_device_id exynos_pinctrl_ids[] = {
{ .compatible = samsung,pinctrl-exynos4210, },
{ .compatible = samsung,pinctrl-exynos4x12, },
- { .compatible = samsung,pinctrl-exynos5440, },
+ { .compatible = samsung,exynos5440-pinctrl, },
};
for_each_matching_node(pctrl_np, exynos_pinctrl_ids)
if (pctrl_np of_device_is_available(pctrl_np))
I am tempted to apply this patch just to annoy people with
obsessive-compulsive disorder (but that's a bit mean).
Reference:
http://izismile.com/2011/08/16/obnoxious_ways_to_drive_people_with_ocd_nuts_16_pics.html
Aargh. Those pencils are wrong:-)
But I'd like a word from Rob, Grant or Stephen so I have some
guidance here.
Yes, the existing order looks odd c.f. existing practice.
I notice that only one of the strangely ordered entries in
exynos_pinctrl_ids[] is fixed. Surely they should all be fixed?
It's probably worth adding the new names to exynos_pinctrl_ids[] rather
than replacing them. That allows the driver to be backwards-compatible
with older DTBs, and partially decouples the driver change from the
.dts/.dtsi file. Still, the whole backwards-compatible DTB thing seems a
little irrelevant when the .dts files are in the kernel source tree anyway.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c-core: Add gpio based bus arbitration implementation
On 12/13/2012 10:50 PM, Naveen Krishna Chatradhi wrote: The arbitrator is a general purpose function which uses two GPIOs to communicate with another device to claim/release a bus. diff --git a/Documentation/devicetree/bindings/i2c/arbitrator-i2c.txt b/Documentation/devicetree/bindings/i2c/arbitrator-i2c.txt new file mode 100644 index 000..cb91ea8 --- /dev/null +++ b/Documentation/devicetree/bindings/i2c/arbitrator-i2c.txt @@ -0,0 +1,56 @@ +Device-Tree bindings for i2c gpio based bus arbitrator + +bus-arbitration-gpios : + Two GPIOs to use with the GPIO-based bus arbitration protocol +(see below). +The first should be an output, and is used to claim the I2C bus, +the second should be an input, and signals that the other side (Client) +wants to claim the bus. This allows two masters to share the same I2C bus. I'm confused why this is even needed; the I2C protocol itself defines how multi-master is supposed to work, just using the regular SCL/SDA lines. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
