Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH net v2 2/3] dt: bindings: add ethernet phy eee-disable-advert option documentation
From: Jerome Brunet @ 2016-11-21 15:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479742524-30222-1-git-send-email-jbrunet@baylibre.com>

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 Documentation/devicetree/bindings/net/phy.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index bc1c3c8bf8fa..7f066b7c1e2c 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -35,6 +35,11 @@ Optional Properties:
 - broken-turn-around: If set, indicates the PHY device does not correctly
   release the turn around line low at the end of a MDIO transaction.
 
+- eee-advert-disable: Bits to clear in the MDIO_AN_EEE_ADV register to
+  disable EEE modes. Example
+    * 0x4: disable EEE for 1000T,
+    * 0x6: disable EEE for 100TX and 1000T
+
 Example:
 
 ethernet-phy at 0 {
-- 
2.7.4

^ permalink raw reply related

* [RFC PATCH net v2 1/3] net: phy: add an option to disable EEE advertisement
From: Jerome Brunet @ 2016-11-21 15:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479742524-30222-1-git-send-email-jbrunet@baylibre.com>

This patch adds an option to disable EEE advertisement in the generic PHY
by providing a mask of prohibited modes corresponding to the value found in
the MDIO_AN_EEE_ADV register.

On some platforms, PHY Low power idle seems to be causing issues, even
breaking the link some cases. The patch provides a convenient way for these
platforms to disable EEE advertisement and work around the issue.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/phy.c        |  3 ++
 drivers/net/phy/phy_device.c | 80 +++++++++++++++++++++++++++++++++++++++-----
 include/linux/phy.h          |  3 ++
 3 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f424b867f73e..a44ee14bd953 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1348,6 +1348,9 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
 {
 	int val = ethtool_adv_to_mmd_eee_adv_t(data->advertised);
 
+	/* Mask prohibited EEE modes */
+	val &= ~phydev->eee_advert_disabled;
+
 	phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, val);
 
 	return 0;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 1a4bf8acad78..74c628e046cb 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1116,6 +1116,43 @@ static int genphy_config_advert(struct phy_device *phydev)
 }
 
 /**
+ * genphy_config_eee_advert - disable unwanted eee mode advertisement
+ * @phydev: target phy_device struct
+ *
+ * Description: Writes MDIO_AN_EEE_ADV after disabling unsupported energy
+ *   efficent ethernet modes. Returns 0 if the PHY's advertisement hasn't
+ *   changed, and 1 if it has changed.
+ */
+static int genphy_config_eee_advert(struct phy_device *phydev)
+{
+	u32 disabled = phydev->eee_advert_disabled;
+	u32 old_adv, adv;
+
+	/* Nothing to disable */
+	if (!disabled)
+		return 0;
+
+	/* If the following call fails, we assume that EEE is not
+	 * supported by the phy. If we read 0, EEE is not advertised
+	 * In both case, we don't need to continue
+	 */
+	adv = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN);
+	if (adv <= 0)
+		return 0;
+
+	old_adv = adv;
+	adv &= ~disabled;
+
+	/* Advertising remains unchanged with the ban */
+	if (old_adv == adv)
+		return 0;
+
+	phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, adv);
+
+	return 1;
+}
+
+/**
  * genphy_setup_forced - configures/forces speed/duplex from @phydev
  * @phydev: target phy_device struct
  *
@@ -1173,15 +1210,20 @@ EXPORT_SYMBOL(genphy_restart_aneg);
  */
 int genphy_config_aneg(struct phy_device *phydev)
 {
-	int result;
+	int err, changed;
+
+	changed = genphy_config_eee_advert(phydev);
 
 	if (AUTONEG_ENABLE != phydev->autoneg)
 		return genphy_setup_forced(phydev);
 
-	result = genphy_config_advert(phydev);
-	if (result < 0) /* error */
-		return result;
-	if (result == 0) {
+	err = genphy_config_advert(phydev);
+	if (err < 0) /* error */
+		return err;
+
+	changed |= err;
+
+	if (changed == 0) {
 		/* Advertisement hasn't changed, but maybe aneg was never on to
 		 * begin with?  Or maybe phy was isolated?
 		 */
@@ -1191,16 +1233,16 @@ int genphy_config_aneg(struct phy_device *phydev)
 			return ctl;
 
 		if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
-			result = 1; /* do restart aneg */
+			changed = 1; /* do restart aneg */
 	}
 
 	/* Only restart aneg if we are advertising something different
 	 * than we were before.
 	 */
-	if (result > 0)
-		result = genphy_restart_aneg(phydev);
+	if (changed > 0)
+		return genphy_restart_aneg(phydev);
 
-	return result;
+	return 0;
 }
 EXPORT_SYMBOL(genphy_config_aneg);
 
@@ -1558,6 +1600,21 @@ static void of_set_phy_supported(struct phy_device *phydev)
 		__set_phy_supported(phydev, max_speed);
 }
 
+static void of_set_phy_eee_disable(struct phy_device *phydev)
+{
+	struct device_node *node = phydev->mdio.dev.of_node;
+	u32 disabled;
+
+	if (!IS_ENABLED(CONFIG_OF_MDIO))
+		return;
+
+	if (!node)
+		return;
+
+	if (!of_property_read_u32(node, "eee-advert-disable", &disabled))
+		phydev->eee_advert_disabled = disabled;
+}
+
 /**
  * phy_probe - probe and init a PHY device
  * @dev: device to probe and init
@@ -1595,6 +1652,11 @@ static int phy_probe(struct device *dev)
 	of_set_phy_supported(phydev);
 	phydev->advertising = phydev->supported;
 
+	/* Get the EEE modes we want to prohibit. We will ask
+	 * the PHY stop advertising these mode later on
+	 */
+	of_set_phy_eee_disable(phydev);
+
 	/* Set the state to READY by default */
 	phydev->state = PHY_READY;
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e25f1830fbcf..7f2ea0af16d1 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -401,6 +401,9 @@ struct phy_device {
 	u32 advertising;
 	u32 lp_advertising;
 
+	/* Energy efficient ethernet modes which should be prohibited */
+	u32 eee_advert_disabled;
+
 	int autoneg;
 
 	int link_timeout;
-- 
2.7.4

^ permalink raw reply related

* [RFC PATCH net v2 0/3] Fix OdroidC2 Gigabit Tx link issue
From: Jerome Brunet @ 2016-11-21 15:35 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
Initially reported as a low Tx throughput issue at gigabit speed, the
platform enters LPI too often. This eventually break the link (both Tx
and Rx), and require to bring the interface down and up again to get the
Rx path working again.

The root cause of this issue is not fully understood yet but disabling EEE
advertisement on the PHY prevent this feature to be negotiated.
With this change, the link is stable and reliable, with the expected
throughput performance.

The patchset adds options in the generic phy driver to disable EEE
advertisement, through device tree. The way it is done is very similar
to the handling of the max-speed property.

This V2 is posted is posted as an RFC. Since it changes the generic PHY
it propably requires to be a bit more careful.
If you are not confortable taking for the coming rc, I can rebase on
net-next instead.

Chnages since V1: [1]
 - Disable the advertisement of EEE in the generic code instead of the
   realtek driver.

[1] : http://lkml.kernel.org/r/1479220154-25851-1-git-send-email-jbrunet at baylibre.com

Jerome Brunet (3):
  net: phy: add an option to disable EEE advertisement
  dt: bindings: add ethernet phy eee-disable-advert option documentation
  ARM64: dts: meson: odroidc2: disable advertisement EEE for GbE.

 Documentation/devicetree/bindings/net/phy.txt      |  5 ++
 .../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 15 ++++
 drivers/net/phy/phy.c                              |  3 +
 drivers/net/phy/phy_device.c                       | 80 +++++++++++++++++++---
 include/linux/phy.h                                |  3 +
 5 files changed, 97 insertions(+), 9 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH] ARM: dts: socfpga: fine-tune L2 cache configuration
From: Dinh Nguyen @ 2016-11-21 15:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118185218.16653-1-marex@denx.de>

Hi Marek,

On 11/18/2016 12:52 PM, Marek Vasut wrote:
> Enable double-linefill and increase prefetch offset, which gives
> considerable read performance boost. The following numbers were
> obtained using lmbench 3.0 bw_mem tool, for easier comparison, the
> numbers are pasted in two columns. The test machine has Cyclone V
> SoC running at 800MHz MPU clock and 512MiB 333MHz 16bit DDR3 DRAM.
> 
> Without patch	| With patch
> $ for i in rd wr rdwr cp fwr frd fcp bzero bcopy ; do echo $i ; bw_mem 64M $i ; done
> rd		| rd
> 64.00 526.46	| 64.00 1151.06
> wr		| wr
> 64.00 329.95	| 64.00 346.14
> rdwr		| rdwr
> 64.00 342.07	| 64.00 367.24
> cp		| cp
> 64.00 239.79	| 64.00 322.47
> fwr		| fwr
> 64.00 1027.90	| 64.00 1025.38
> frd		| frd
> 64.00 322.36	| 64.00 641.89
> fcp		| fcp
> 64.00 256.99	| 64.00 408.41
> bzero		| bzero
> 64.00 1028.43	| 64.00 1025.07
> bcopy		| bcopy
> 64.00 294.73	| 64.00 357.19
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>

Applied!

Thanks,
Dinh

^ permalink raw reply

* spin_lock behavior with ARM64 big.Little/HMP
From: Sudeep Holla @ 2016-11-21 15:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <f9fc9549d801fece7422d97d5d89df8b@codeaurora.org>



On 18/11/16 20:22, Vikram Mulukutla wrote:
>
> Hi Sudeep,
>
> Thanks for taking a look!
>
> On 2016-11-18 02:30, Sudeep Holla wrote:
>> Hi Vikram,
>>
>> On 18/11/16 02:22, Vikram Mulukutla wrote:
>>> Hello,
>>>
>>> This isn't really a bug report, but just a description of a
>>> frequency/IPC
>>> dependent behavior that I'm curious if we should worry about. The
>>> behavior
>>> is exposed by questionable design so I'm leaning towards don't-care.
>>>
>>> Consider these threads running in parallel on two ARM64 CPUs running
>>> mainline
>>> Linux:
>>>
>>
>> Are you seeing this behavior with the mainline kernel on any platforms
>> as we have a sort of workaround for this ?
>>
>
> If I understand that workaround correctly, the ARM timer event stream is
> used
> to periodically wake up CPUs that are waiting in WFE, is that right? I
> think
> my scenario below may be different because LittleCPU doesn't actually wait
> on a WFE event in the loop that is trying to increment lock->next, i.e.
> it's
> stuck in the following loop:
>
>         ARM64_LSE_ATOMIC_INSN(
>         /* LL/SC */
> "       prfm    pstl1strm, %3\n"
> "1:     ldaxr   %w0, %3\n"
> "       add     %w1, %w0, %w5\n"
> "       stxr    %w2, %w1, %3\n"
> "       cbnz    %w2, 1b\n",
>

Interesting. Just curious if this is r0p0/p1 A53 ? If so, is the errata
819472 enabled ?

-- 
Regards,
Sudeep

^ permalink raw reply

* Synopsys Ethernet QoS Driver
From: Joao Pinto @ 2016-11-21 15:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <7318c77e-3011-03f3-e673-54e3bb0554aa@st.com>

On 21-11-2016 15:03, Giuseppe CAVALLARO wrote:
> On 11/21/2016 4:00 PM, Joao Pinto wrote:
>> On 21-11-2016 14:36, Giuseppe CAVALLARO wrote:
>>> Hello Joao
>>>
>>> On 11/21/2016 2:48 PM, Joao Pinto wrote:
>>>> Synopsys QoS IP is a separated hardware component, so it should be reusable by
>>>> all implementations using it and so have its own "core driver" and platform +
>>>> pci glue drivers. This is necessary for example in hardware validation, where
>>>> you prototype an IP and instantiate its drivers and test it.
>>>>
>>>> Was there a strong reason to integrate QoS features directly in stmmac and not
>>>> in synopsys/dwc_eth_qos.*?
>>>
>>> We decided to enhance the stmmac on supporting the QoS for several
>>> reasons; for example the common APIs that the driver already exposed and
>>> actually suitable for other SYNP chips. Then, PTP, EEE,
>>> S/RGMII, MMC could be shared among different chips with a minimal
>>> effort.  This meant a lot of code already ready.
>>>
>>> For sure, the net-core, Ethtool, mdio parts were reused. Same for the
>>> glue logic files.
>>> For the latter, this helped to easily bring-up new platforms also
>>> because the stmmac uses the HW cap register to auto-configure many
>>> parts of the MAC core, DMA and modules. This helped many users, AFAIK.
>>>
>>> For validation purpose, this is my experience, the stmmac helped
>>> a lot because people used the same code to validate different HW
>>> and it was easy to switch to a platform to another one in order to
>>> verify / check if the support was ok or if a regression was introduced.
>>> This is important for complex supports like PTP or EEE.
>>>
>>> Hoping this can help.
>>>
>>> Do not hesitate to contact me for further details
>>
>> Thanks for the highly detailed info.
>> My target application is to prototype the Ethernet QoS IP in a FPGA, with a PHY
>> attached and make hardware validation.
>>
>> In your opinion a refactored stmmac with the missing QoS features would be
>> suitable for it?
> 
> I think so; somebody also added code for FPGA.
> 
> In any case, step-by-step we can explore and understand
> how to proceed. I wonder if you could start looking at the internal
> of the stmmac. Then welcome doubts and open question...

Yes I am going to do that thanks... taking my first steps in this IP :)

> 
>>
>> Thanks.
> 
> welcome
> 
> peppe
> 
>>
>>>
>>> peppe
>>
>>
> 

^ permalink raw reply

* [PATCH 0/8] firmware: arm_scpi: add support for legacy SCPI protocol
From: Sudeep Holla @ 2016-11-21 15:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAD0U-hK982V+kUArjVSG7pTs5CLpNsgJaC5t8aPg8XuoiMRU0A@mail.gmail.com>



On 21/11/16 15:04, Ryan Harkin wrote:

[...]

> Switching to u-boot won't solve any thermal sensor calibration problems.
>

Indeed. The board Russell had was one of those early ones which was not
calibrated properly. He is now able to use the 16.10 release after
calibration.

-- 
Regards,
Sudeep

^ permalink raw reply

* Synopsys Ethernet QoS Driver
From: Joao Pinto @ 2016-11-21 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <ffd3b27c-0bc9-b53b-cc65-c7808595a553@st.com>

On 21-11-2016 14:25, Giuseppe CAVALLARO wrote:
> On 11/21/2016 2:28 PM, Lars Persson wrote:
>>
>>
>>> 21 nov. 2016 kl. 13:53 skrev Giuseppe CAVALLARO <peppe.cavallaro@st.com>:
>>>
>>> Hello Joao
>>>
>>>> On 11/21/2016 1:32 PM, Joao Pinto wrote:
>>>> Hello,
>>>>
>>>>> On 21-11-2016 05:29, Rayagond Kokatanur wrote:
>>>>>> On Sat, Nov 19, 2016 at 7:26 PM, Rabin Vincent <rabin@rab.in> wrote:
>>>>>>> On Fri, Nov 18, 2016 at 02:20:27PM +0000, Joao Pinto wrote:
>>>>>>> For now we are interesting in improving the synopsys QoS driver under
>>>>>>> /nect/ethernet/synopsys. For now the driver structure consists of a
>>>>>>> single file
>>>>>>> called dwc_eth_qos.c, containing synopsys ethernet qos common ops and
>>>>>>> platform
>>>>>>> related stuff.
>>>>>>>
>>>>>>> Our strategy would be:
>>>>>>>
>>>>>>> a) Implement a platform glue driver (dwc_eth_qos_pltfm.c)
>>>>>>> b) Implement a pci glue driver (dwc_eth_qos_pci.c)
>>>>>>> c) Implement a "core driver" (dwc_eth_qos.c) that would only have
>>>>>>> Ethernet QoS
>>>>>>> related stuff to be reused by the platform / pci drivers
>>>>>>> d) Add a set of features to the "core driver" that we have available
>>>>>>> internally
>>>>>>
>>>>>> Note that there are actually two drivers in mainline for this hardware:
>>>>>>
>>>>>> drivers/net/ethernet/synopsis/
>>>>>> drivers/net/ethernet/stmicro/stmmac/
>>>>>
>>>>> Yes the later driver (drivers/net/ethernet/stmicro/stmmac/) supports
>>>>> both 3.x and 4.x. It has glue layer for pci, platform, core etc,
>>>>> please refer this driver once before you start.
>>>>>
>>>>> You can start adding missing feature of 4.x in stmmac driver.
>>>>
>>>> Thanks you all for all the info.
>>>> Well, I think we are in a good position to organize the ethernet drivers
>>>> concerning Synopsys IPs.
>>>>
>>>> First of all, in my opinion, it does not make sense to have a ethernet/synopsis
>>>> (typo :)) when ethernet/stmicro is also for a synopsys IP. If we have another
>>>> vendor using the same IP it should be able to reuse the commonn operations. But
>>>> I would put that discussion for later :)
>>>>
>>>> For now I suggest that for we create ethernet/qos and create there a folder
>>>> called dwc (designware controller) where all the synopsys qos IP specific code
>>>> in order to be reused for example by ethernet/stmicro/stmmac/. We just have to
>>>> figure out a clean interface for "client drivers" like stmmac to interact with
>>>> the new qos driver.
>>>>
>>>> What do you think about this approach?
>>>
>>> The stmmac drivers run since many years on several platforms
>>> (sh4, stm32, arm, x86, mips ...) and it supports an huge of amount of
>>> configurations starting from 3.1x to 3.7x databooks.
>>>
>>> It also supports QoS hardware; for example, 4.00a, 4.10a and 4.20a
>>> are fully working.
>>>
>>> Also the stmmac has platform, device-tree and pcie supports and
>>> a lot of maintained glue-logic files.
>>>
>>> It is fully documented inside the kernel tree.
>>>
>>> I am happy to have new enhancements from other developers.
>>> So, on my side, if you want to spend your time on improving it on your
>>> platforms please feel free to do it!
>>>
>>> Concerning the stmicro/stmmac naming, these come from a really old
>>> story and have no issue to adopt new folder/file names.
>>>
>>> I am also open to merge fixes and changes from ethernet/synopsis.
>>> I want to point you on some benchmarks made by Alex some months ago
>>> (IIRC) that showed an stmmac winner (due to the several optimizations
>>> analyzed and reviewed in this mailing list).
>>>
>>> Peppe
>>>
>>
>> Hello Joao and others,
>>

Hi Lars,

>> As the maintainer of dwc_eth_qos.c I prefer also that we put efforts on the
>> most mature driver, the stmmac.
>>
>> I hope that the code can migrate into an ethernet/synopsys folder to keep the
>> convention of naming the folder after the vendor. This makes it easy for
>> others to find the driver.
>>
>> The dwc_eth_qos.c will eventually be removed and its DT binding interface can
>> then be implemented in the stmmac driver.

So your ideia is to pick the ethernet/stmmac and rename it to ethernet/synopsys
and try to improve the structure and add the missing QoS features to it?


> 
> Thanks Lars, I will be happy to support all you on this transition
> and I agree on renaming all.
> 
> peppe
> 
> 
>> - Lars
>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>> (See http://lists.openwall.net/netdev/2016/02/29/127)
>>>>>>
>>>>>> The former only supports 4.x of the hardware.
>>>>>>
>>>>>> The later supports 4.x and 3.x and already has a platform glue driver
>>>>>> with support for several platforms, a PCI glue driver, and a core driver
>>>>>> with several features not present in the former (for example: TX/RX
>>>>>> interrupt coalescing, EEE, PTP).
>>>>>>
>>>>>> Have you evaluated both drivers?  Why have you decided to work on the
>>>>>> former rather than the latter?
>>>>>
>>>>>
>>>>
>>>> Thanks.
>>>>
>>>>
>>>>
>>>>
>>>
>>
> 

^ permalink raw reply

* [PATCH 0/8] firmware: arm_scpi: add support for legacy SCPI protocol
From: Ryan Harkin @ 2016-11-21 15:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161108174622.GW1041@n2100.armlinux.org.uk>

On 8 November 2016 at 17:46, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Nov 08, 2016 at 05:37:25PM +0000, Sudeep Holla wrote:
>>
>>
>> On 08/11/16 16:06, Russell King - ARM Linux wrote:
>> >On Tue, Nov 08, 2016 at 03:40:38PM +0000, Russell King - ARM Linux wrote:
>> >>As it contains a zero sized Image and .dtb files, I tried copying my
>> >>Image and .dtb over, and also copied my original config.txt (only
>> >>change is AUTORUN: FALSE).  It still doesn't appear to boot beyond
>> >>this point.
>> >
>> >Well, it seems that I can't even go back to my original set of firmware
>> >as UEFI has stopped working:
>> >
>> >NOTICE:  Booting Trusted Firmware
>> >NOTICE:  BL1: v1.0(release):14b6608
>> >NOTICE:  BL1: Built : 14:15:51, Sep  1 2014
>> >NOTICE:  BL1: Booting BL2
>> >NOTICE:  BL2: v1.0(release):14b6608
>> >NOTICE:  BL2: Built : 14:15:51, Sep  1 2014
>> >NOTICE:  BL1: Booting BL3-1
>> >NOTICE:  BL3-1: v1.0(release):14b6608
>> >NOTICE:  BL3-1: Built : 14:15:53, Sep  1 2014
>> >UEFI firmware (version v2.1 built at 14:41:56 on Oct 23 2014)
>> >Warning: Fail to load FDT file 'juno.dtb'.
>> >
>>
>> This again because of incompatibility of the configuration data saved in
>> NOR flash. The erase command I gave early is to erase that when you
>> switched between the UEFI binaries. It's really horrible mess UEFI
>> created in the initial days of Juno, and hopefully they have moved to
>> some standard format these days.
>
> Yea, what it means is I've no possibility to go back to what was
> originally working now, since I don't understand how to get UEFI to
> behave (Will set the machine up for me, I don't have any information
> on how it was originally configured other than what was on the uSD
> card, which appears incomplete.)
>
>> >and UEFI is the most unfriendly thing going - the "boot manager" thing
>> >doesn't let you view the configuration:
>> >
>>
>> I completely agree. I had real bad times in past dealing with such
>> things in UEFI.
>>
>> >[1] Linux from NOR Flash
>> >[2] Shell
>> >[3] Boot Manager
>> >Start: 3
>> >[1] Add Boot Device Entry
>> >[2] Update Boot Device Entry
>> >[3] Remove Boot Device Entry
>> >[4] Reorder Boot Device Entries
>> >[5] Update FDT path
>> >[6] Set Boot Timeout
>> >[7] Return to main menu
>> >Choice:
>> >
>> >and dropping into the shell... well, I've no idea how to get a listing
>> >of what it thinks is in the NOR device (or even how to refer to the
>> >NOR device.)  "devices" shows nothing that's even remotely English.
>> >

Using UEFI is painful enough on Juno.  Using a 2 year old version with
"that" menu system is even worse.

So I'll just pitch in here and recommend you switch to using u-boot.
Then you can see your config with "printenv" and you'll probably
understand it too.

The board will boot my prebuilt kernel/DTB using u-boot if you erase
the uSD card and unzip this firmware image to it:

http://releases.linaro.org/members/arm/platforms/16.10/juno-latest-oe-uboot.zip

Then you can copy over your own kernel & DTB for your own testing,
configure u-boot for TFTP, or whatever you like.

Switching to u-boot won't solve any thermal sensor calibration problems.


>>
>> I think startup.nsh needs some edits. Just replace it with something like:
>>
>> "norkern dtb=board.dtb console=ttyAMA0,115200n8 root=/dev/nfs rw
>> rootwait ip=dhcp systemd.log_target=null nfsroot=..." or something
>> alike. Currently it just echos and stops.
>>
>> Regarding the new firmware stopping abruptly, I have no clue, except
>> asking you to erase the flash completely when switching between the
>> firmware versions. That has worked for me for all UEFI related issues in
>> the past. It's really annoying I understand.
>>
>> flash> eraseall
>
> I've tried that, it still stops at the same point, after:
>
> FV2 Hob           0xFE07B000 - 0xFE8253BF
>
> and remains unresponsive.
>
> I do notice that the uSD card becomes visible through USB at this point
> though.
>
> Okay, well, I'm going to have to disable Juno from the nightly boots
> until we get some kind of resolution on this, as my Juno is now
> incapable of booting anything.
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

^ permalink raw reply

* Synopsys Ethernet QoS Driver
From: Giuseppe CAVALLARO @ 2016-11-21 15:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <48776f8b-4e06-1456-1b52-3ea08a22b2a4@synopsys.com>

On 11/21/2016 4:00 PM, Joao Pinto wrote:
> On 21-11-2016 14:36, Giuseppe CAVALLARO wrote:
>> Hello Joao
>>
>> On 11/21/2016 2:48 PM, Joao Pinto wrote:
>>> Synopsys QoS IP is a separated hardware component, so it should be reusable by
>>> all implementations using it and so have its own "core driver" and platform +
>>> pci glue drivers. This is necessary for example in hardware validation, where
>>> you prototype an IP and instantiate its drivers and test it.
>>>
>>> Was there a strong reason to integrate QoS features directly in stmmac and not
>>> in synopsys/dwc_eth_qos.*?
>>
>> We decided to enhance the stmmac on supporting the QoS for several
>> reasons; for example the common APIs that the driver already exposed and
>> actually suitable for other SYNP chips. Then, PTP, EEE,
>> S/RGMII, MMC could be shared among different chips with a minimal
>> effort.  This meant a lot of code already ready.
>>
>> For sure, the net-core, Ethtool, mdio parts were reused. Same for the
>> glue logic files.
>> For the latter, this helped to easily bring-up new platforms also
>> because the stmmac uses the HW cap register to auto-configure many
>> parts of the MAC core, DMA and modules. This helped many users, AFAIK.
>>
>> For validation purpose, this is my experience, the stmmac helped
>> a lot because people used the same code to validate different HW
>> and it was easy to switch to a platform to another one in order to
>> verify / check if the support was ok or if a regression was introduced.
>> This is important for complex supports like PTP or EEE.
>>
>> Hoping this can help.
>>
>> Do not hesitate to contact me for further details
>
> Thanks for the highly detailed info.
> My target application is to prototype the Ethernet QoS IP in a FPGA, with a PHY
> attached and make hardware validation.
>
> In your opinion a refactored stmmac with the missing QoS features would be
> suitable for it?

I think so; somebody also added code for FPGA.

In any case, step-by-step we can explore and understand
how to proceed. I wonder if you could start looking at the internal
of the stmmac. Then welcome doubts and open question...

>
> Thanks.

welcome

peppe

>
>>
>> peppe
>
>

^ permalink raw reply

* Synopsys Ethernet QoS Driver
From: Joao Pinto @ 2016-11-21 15:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <937252db-9538-2cf6-c8fa-82b558531c51@st.com>

On 21-11-2016 14:36, Giuseppe CAVALLARO wrote:
> Hello Joao
> 
> On 11/21/2016 2:48 PM, Joao Pinto wrote:
>> Synopsys QoS IP is a separated hardware component, so it should be reusable by
>> all implementations using it and so have its own "core driver" and platform +
>> pci glue drivers. This is necessary for example in hardware validation, where
>> you prototype an IP and instantiate its drivers and test it.
>>
>> Was there a strong reason to integrate QoS features directly in stmmac and not
>> in synopsys/dwc_eth_qos.*?
> 
> We decided to enhance the stmmac on supporting the QoS for several
> reasons; for example the common APIs that the driver already exposed and
> actually suitable for other SYNP chips. Then, PTP, EEE,
> S/RGMII, MMC could be shared among different chips with a minimal
> effort.  This meant a lot of code already ready.
> 
> For sure, the net-core, Ethtool, mdio parts were reused. Same for the
> glue logic files.
> For the latter, this helped to easily bring-up new platforms also
> because the stmmac uses the HW cap register to auto-configure many
> parts of the MAC core, DMA and modules. This helped many users, AFAIK.
> 
> For validation purpose, this is my experience, the stmmac helped
> a lot because people used the same code to validate different HW
> and it was easy to switch to a platform to another one in order to
> verify / check if the support was ok or if a regression was introduced.
> This is important for complex supports like PTP or EEE.
> 
> Hoping this can help.
> 
> Do not hesitate to contact me for further details

Thanks for the highly detailed info.
My target application is to prototype the Ethernet QoS IP in a FPGA, with a PHY
attached and make hardware validation.

In your opinion a refactored stmmac with the missing QoS features would be
suitable for it?

Thanks.

> 
> peppe

^ permalink raw reply

* [BUG] hdlcd gets confused about base address
From: Russell King - ARM Linux @ 2016-11-21 14:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121143053.GH1041@n2100.armlinux.org.uk>

On Mon, Nov 21, 2016 at 02:30:53PM +0000, Russell King - ARM Linux wrote:
> On Mon, Nov 21, 2016 at 01:24:19PM +0000, Russell King - ARM Linux wrote:
> > On Mon, Nov 21, 2016 at 12:56:53PM +0000, Liviu Dudau wrote:
> > > That is mostly due to the check in hdlcd_crtc_disable() which I should
> > > remove, I've added it because I was getting a ->disable() hook call
> > > before any ->enable() was called at startup time. I need to revisit
> > > this as I remember Daniel was commenting that this was not needed.
> > 
> > Removing that test results in:
> > 
> > [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:24:crtc-0] flip_done timed out
> > 
> > and the kernel hanging, seemingly in an IRQs-off region.
> 
> Annoyingly, enabling DRM debug prevents the kernel hanging...

I've been trying to trace through what's happening with this flip_done
stuff, but I'm finding it _extremely_ difficult to follow the atomic
code.

(Sorry, I'm going to go over my usual 72 column limit for this due to
the damn long DRM function names.)

I can see that drm_atomic_helper_commit() calls drm_atomic_helper_setup_commit()
which sets up commit->flip_done for each CRTC, and sets up an event for
each.

drm_atomic_helper_commit() continues on to eventually call drm_atomic_helper_swap_state()
which then swaps the state for the CRTCs, but then ends up dropping
the event reference:

	state->crtcs[i].commit->event = NULL;

What I can't see is why this isn't a leaked pointer - I don't see
anything inbetween taking charge of that structure.  The _commit_
hasn't been swapped from what I can see, it's just state->crtcs[i].state
that have been swapped.

So I can't see who's responsible for generating this event, or how the
backend DRM drivers get to know about this event, and that they should
complete the flip.

What I also don't get is why DRM is wanting to wait for a flip event
when we're disabling the CRTC.  None of this makes sense to me, like
much of the atomic modeset code...

(I'm probably never going to convert Armada DRM to atomic modeset, I
just don't seem to be capable of understanding atomic modeset.  Maybe
I'm too old?)

[drm:drm_ioctl] pid=2178, dev=0xe200, auth=1, DRM_IOCTL_MODE_RMFB
[drm:drm_mode_object_unreference] OBJ ID: 38 (4)
[drm:drm_atomic_state_init] Allocated atomic state ffffffc974c7c300
[drm:drm_mode_object_reference] OBJ ID: 43 (1)
[drm:drm_atomic_get_crtc_state] Added [CRTC:24:crtc-0] ffffffc975e59400 state to ffffffc974c7c300
[drm:drm_mode_object_reference] OBJ ID: 38 (3)
[drm:drm_atomic_get_plane_state] Added [PLANE:23:plane-0] ffffffc974c7c100 state to ffffffc974c7c300
[drm:drm_mode_object_unreference] OBJ ID: 43 (2)
[drm:drm_atomic_set_mode_for_crtc] Set [NOMODE] for CRTC state ffffffc975e59400
[drm:drm_atomic_set_crtc_for_plane] Link plane state ffffffc974c7c100 to [NOCRTC]
[drm:drm_mode_object_unreference] OBJ ID: 38 (4)
[drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane state ffffffc974c7c100
[drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:24:crtc-0] to ffffffc974c7c300
[drm:drm_mode_object_reference] OBJ ID: 26 (4)
[drm:drm_mode_object_reference] OBJ ID: 26 (5)
[drm:drm_atomic_get_connector_state] Added [CONNECTOR:26] ffffffc974c7c000 state to ffffffc974c7c300
[drm:drm_mode_object_unreference] OBJ ID: 26 (6)
[drm:drm_atomic_set_crtc_for_connector] Link connector state ffffffc974c7c000 to [NOCRTC]
[drm:drm_atomic_check_only] checking ffffffc974c7c300
[drm:drm_atomic_helper_check_modeset] [CRTC:24:crtc-0] mode changed
[drm:drm_atomic_helper_check_modeset] [CRTC:24:crtc-0] enable changed
[drm:drm_atomic_helper_check_modeset] Updating routing for [CONNECTOR:26:HDMI-A-1]
[drm:drm_atomic_helper_check_modeset] Disabling [CONNECTOR:26:HDMI-A-1]
[drm:drm_atomic_helper_check_modeset] [CRTC:24:crtc-0] active changed
[drm:drm_atomic_helper_check_modeset] [CRTC:24:crtc-0] needs all connectors, enable: n, active: n
[drm:drm_atomic_add_affected_connectors] Adding all current connectors for [CRTC:24:crtc-0] to ffffffc974c7c300
[drm:drm_atomic_commit] commiting ffffffc974c7c300
[drm:drm_atomic_helper_commit_modeset_disables] disabling [ENCODER:25:TMDS-25]
[drm:drm_atomic_helper_commit_modeset_disables] disabling [CRTC:24:crtc-0]
hdlcd_crtc_disable: active 0
[drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:24:crtc-0] flip_done timed out
[drm:drm_atomic_state_default_clear] Clearing atomic state ffffffc974c7c300
[drm:drm_mode_object_unreference] OBJ ID: 26 (5)
[drm:drm_mode_object_unreference] OBJ ID: 26 (4)
[drm:drm_mode_object_unreference] OBJ ID: 43 (1)
[drm:drm_mode_object_unreference] OBJ ID: 38 (3)
[drm:drm_atomic_state_free] Freeing atomic state ffffffc974c7c300
[drm:drm_mode_object_unreference] OBJ ID: 38 (2)
[drm:drm_mode_object_unreference] OBJ ID: 38 (1)


-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH v6 0/3] Add Mediatek JPEG Decoder
From: Hans Verkuil @ 2016-11-21 14:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479353915-5043-1-git-send-email-rick.chang@mediatek.com>

I'm missing a MAINTAINERS patch for this new driver.

Can you post a patch for that?

It's the only thing preventing this from being merged.

Regards,

	Hans

On 17/11/16 04:38, Rick Chang wrote:
> This series of patches provide a v4l2 driver to control Mediatek JPEG decoder
> for decoding JPEG image and Motion JPEG bitstream.
>
> changes since v5:
> - remove redundant name from struct mtk_jpeg_fmt
> - Set state of all buffers to VB2_BUF_STATE_QUEUED if fail in start streaming
> - Remove VB2_USERPTR
> - Add check for buffer index
>
> changes since v4:
> - Change file name of binding documentation
> - Revise DT binding documentation
> - Revise compatible string
>
> changes since v3:
> - Revise DT binding documentation
> - Revise compatible string
>
> changes since v2:
> - Revise DT binding documentation
>
> changes since v1:
> - Rebase for v4.9-rc1.
> - Update Compliance test version and result
> - Remove redundant path in Makefile
> - Fix potential build error without CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP
> - Fix warnings from patch check and smatch check
>
> * Dependency
> The patch "arm: dts: mt2701: Add node for JPEG decoder" depends on:
>   CCF "Add clock support for Mediatek MT2701"[1]
>   iommu and smi "Add the dtsi node of iommu and smi for mt2701"[2]
>
> [1] http://lists.infradead.org/pipermail/linux-mediatek/2016-October/007271.html
> [2] https://patchwork.kernel.org/patch/9164013/
>
> * Compliance test
> v4l2-compliance SHA   : 4ad7174b908a36c4f315e3fe2efa7e2f8a6f375a
>
> Driver Info:
>         Driver name   : mtk-jpeg decode
>         Card type     : mtk-jpeg decoder
>         Bus info      : platform:15004000.jpegdec
>         Driver version: 4.9.0
>         Capabilities  : 0x84204000
>                 Video Memory-to-Memory Multiplanar
>                 Streaming
>                 Extended Pix Format
>                 Device Capabilities
>         Device Caps   : 0x04204000
>                 Video Memory-to-Memory Multiplanar
>                 Streaming
>                 Extended Pix Format
>
> Compliance test for device /dev/video3 (not using libv4l2):
>
> Required ioctls:
>         test VIDIOC_QUERYCAP: OK
>
> Allow for multiple opens:
>         test second video open: OK
>         test VIDIOC_QUERYCAP: OK
>         test VIDIOC_G/S_PRIORITY: OK
>         test for unlimited opens: OK
>
> Debug ioctls:
>         test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
>         test VIDIOC_LOG_STATUS: OK (Not Supported)
>
> Input ioctls:
>         test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>         test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>         test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>         test VIDIOC_ENUMAUDIO: OK (Not Supported)
>         test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
>         test VIDIOC_G/S_AUDIO: OK (Not Supported)
>         Inputs: 0 Audio Inputs: 0 Tuners: 0
>
> Output ioctls:
>         test VIDIOC_G/S_MODULATOR: OK (Not Supported)
>         test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>         test VIDIOC_ENUMAUDOUT: OK (Not Supported)
>         test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
>         test VIDIOC_G/S_AUDOUT: OK (Not Supported)
>         Outputs: 0 Audio Outputs: 0 Modulators: 0
>
> Input/Output configuration ioctls:
>         test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
>         test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
>         test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
>         test VIDIOC_G/S_EDID: OK (Not Supported)
>
>         Control ioctls:
>                 test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
>                 test VIDIOC_QUERYCTRL: OK (Not Supported)
>                 test VIDIOC_G/S_CTRL: OK (Not Supported)
>                 test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
>                 test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
>                 test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
>                 Standard Controls: 0 Private Controls: 0
>
>         Format ioctls:
>                 test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>                 test VIDIOC_G/S_PARM: OK (Not Supported)
>                 test VIDIOC_G_FBUF: OK (Not Supported)
>                 test VIDIOC_G_FMT: OK
>                 test VIDIOC_TRY_FMT: OK
>                 test VIDIOC_S_FMT: OK
>                 test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
>                 test Cropping: OK (Not Supported)
>                 test Composing: OK
>                 test Scaling: OK
>
>         Codec ioctls:
>                 test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
>                 test VIDIOC_G_ENC_INDEX: OK (Not Supported)
>                 test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>
>         Buffer ioctls:
>                 test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>                 test VIDIOC_EXPBUF: OK
>
> Test input 0:
>
>
> Total: 43, Succeeded: 43, Failed: 0, Warnings: 0
>
> Rick Chang (3):
>   dt-bindings: mediatek: Add a binding for Mediatek JPEG Decoder
>   vcodec: mediatek: Add Mediatek JPEG Decoder Driver
>   arm: dts: mt2701: Add node for Mediatek JPEG Decoder
>
>  .../bindings/media/mediatek-jpeg-decoder.txt       |   37 +
>  arch/arm/boot/dts/mt2701.dtsi                      |   14 +
>  drivers/media/platform/Kconfig                     |   15 +
>  drivers/media/platform/Makefile                    |    2 +
>  drivers/media/platform/mtk-jpeg/Makefile           |    2 +
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c    | 1303 ++++++++++++++++++++
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h    |  139 +++
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c      |  417 +++++++
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.h      |   91 ++
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.c   |  160 +++
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.h   |   25 +
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h     |   58 +
>  12 files changed, 2263 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.txt
>  create mode 100644 drivers/media/platform/mtk-jpeg/Makefile
>  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
>  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
>  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c
>  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.h
>  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.c
>  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.h
>  create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h
>

^ permalink raw reply

* [PATCH v6 3/3] arm: dts: mt2701: Add node for Mediatek JPEG Decoder
From: Hans Verkuil @ 2016-11-21 14:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479353915-5043-4-git-send-email-rick.chang@mediatek.com>

On 17/11/16 04:38, Rick Chang wrote:
> Signed-off-by: Rick Chang <rick.chang@mediatek.com>
> Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> ---
> This patch depends on:
>   CCF "Add clock support for Mediatek MT2701"[1]
>   iommu and smi "Add the dtsi node of iommu and smi for mt2701"[2]
>
> [1] http://lists.infradead.org/pipermail/linux-mediatek/2016-October/007271.html
> [2] https://patchwork.kernel.org/patch/9164013/

I assume that 1 & 2 will appear in 4.10? So this patch needs to go in 
after the
other two are merged in 4.10?

Regards,

	Hans

> ---
>  arch/arm/boot/dts/mt2701.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
> index 8f13c70..4dd5048 100644
> --- a/arch/arm/boot/dts/mt2701.dtsi
> +++ b/arch/arm/boot/dts/mt2701.dtsi
> @@ -298,6 +298,20 @@
>  		power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>;
>  	};
>
> +	jpegdec: jpegdec at 15004000 {
> +		compatible = "mediatek,mt2701-jpgdec";
> +		reg = <0 0x15004000 0 0x1000>;
> +		interrupts = <GIC_SPI 143 IRQ_TYPE_LEVEL_LOW>;
> +		clocks =  <&imgsys CLK_IMG_JPGDEC_SMI>,
> +			  <&imgsys CLK_IMG_JPGDEC>;
> +		clock-names = "jpgdec-smi",
> +			      "jpgdec";
> +		power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>;
> +		mediatek,larb = <&larb2>;
> +		iommus = <&iommu MT2701_M4U_PORT_JPGDEC_WDMA>,
> +			 <&iommu MT2701_M4U_PORT_JPGDEC_BSDMA>;
> +	};
> +
>  	vdecsys: syscon at 16000000 {
>  		compatible = "mediatek,mt2701-vdecsys", "syscon";
>  		reg = <0 0x16000000 0 0x1000>;
>

^ permalink raw reply

* [PATCH v16 08/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_needs_probing, and call it only if acpi disabled.
From: Fu Wei @ 2016-11-21 14:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118195657.GM1197@leverpostej>

Hi Mark,

On 19 November 2016 at 03:56, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Nov 16, 2016 at 09:49:01PM +0800, fu.wei at linaro.org wrote:
>> From: Fu Wei <fu.wei@linaro.org>
>>
>> The patch refactor original arch_timer_needs_probing function:
>>     (1) Separate out arch_timer_needs_probing from arch_timer_common_init,
>>     and call it only if acpi disabled.
>>     (2) Rename arch_timer_needs_probing to arch_timer_needs_of_probing.
>
> Please write a real commit message.

OK, how about this:

clocksource/drivers/arm_arch_timer: Refactor
arch_timer_needs_probing, only use it in the DT init code

Currently, the arch_timer_common_init uses arch_timer_needs_probing call to
wait for both timers been probed which is only for booting from DT.

For booting from ACPI, we don't need these in arch_timer_common_init, these code
should be only in the DT init code.

This patch seperates arch_timer_needs_probing related code out into a
new function
named arch_timer_needs_of_probing, and then reworks arch_timer_of_init
to use it,
so that we can keep this only in the DT init code.


>
>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 34 +++++++++++++++++++---------------
>>  1 file changed, 19 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
>> index fe4e812..9ddc091 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -792,15 +792,28 @@ static const struct of_device_id arch_timer_mem_of_match[] __initconst = {
>>       {},
>>  };
>>
>> -static bool __init
>> -arch_timer_needs_probing(int type, const struct of_device_id *matches)
>> +static bool __init arch_timer_needs_of_probing(void)
>>  {
>>       struct device_node *dn;
>>       bool needs_probing = false;
>> +     unsigned int mask = ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM;
>> +
>> +     /* We have two timers, and both device-tree nodes are probed. */
>> +     if ((arch_timers_present & mask) == mask)
>> +             return false;
>> +
>> +     /*
>> +      * Only one type of timer is probed,
>> +      * check if we have another type of timer node in device-tree.
>> +      */
>> +     if (arch_timers_present & ARCH_TIMER_TYPE_CP15)
>> +             dn = of_find_matching_node(NULL, arch_timer_mem_of_match);
>> +     else
>> +             dn = of_find_matching_node(NULL, arch_timer_of_match);
>>
>> -     dn = of_find_matching_node(NULL, matches);
>> -     if (dn && of_device_is_available(dn) && !(arch_timers_present & type))
>> +     if (dn && of_device_is_available(dn))
>>               needs_probing = true;
>> +
>>       of_node_put(dn);
>>
>>       return needs_probing;
>> @@ -808,17 +821,8 @@ arch_timer_needs_probing(int type, const struct of_device_id *matches)
>>
>>  static int __init arch_timer_common_init(void)
>>  {
>> -     unsigned int mask = ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM;
>> -
>> -     /* Wait until both nodes are probed if we have two timers */
>> -     if ((arch_timers_present & mask) != mask) {
>> -             if (arch_timer_needs_probing(ARCH_TIMER_TYPE_MEM,
>> -                                          arch_timer_mem_of_match))
>> -                     return 0;
>> -             if (arch_timer_needs_probing(ARCH_TIMER_TYPE_CP15,
>> -                                          arch_timer_of_match))
>> -                     return 0;
>> -     }
>
> Why can't we just move this into the DT-specific caller of
> arch_timer_common_init()?

yes, I have thought about this, but that means we have to split arch_timer_init,
and use arch_timer_register and arch_timer_common_init directly in
arch_timer_of_init.

In another word, I also need to modify arch_timer_of_init, not only
arch_timer_common_init,
but let me have a try in v17, see if you like it.

>
> Thanks
> Mark.
>
>> +     if (acpi_disabled && arch_timer_needs_of_probing())
>> +             return 0;
>>
>>       arch_timer_banner(arch_timers_present);
>>       arch_counter_register(arch_timers_present);
>> --
>> 2.7.4
>>



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat

^ permalink raw reply

* Synopsys Ethernet QoS Driver
From: Giuseppe CAVALLARO @ 2016-11-21 14:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <afc18eb0-4c11-9668-498d-982624c6de78@synopsys.com>

Hello Joao

On 11/21/2016 2:48 PM, Joao Pinto wrote:
> Synopsys QoS IP is a separated hardware component, so it should be reusable by
> all implementations using it and so have its own "core driver" and platform +
> pci glue drivers. This is necessary for example in hardware validation, where
> you prototype an IP and instantiate its drivers and test it.
>
> Was there a strong reason to integrate QoS features directly in stmmac and not
> in synopsys/dwc_eth_qos.*?

We decided to enhance the stmmac on supporting the QoS for several
reasons; for example the common APIs that the driver already exposed and
actually suitable for other SYNP chips. Then, PTP, EEE,
S/RGMII, MMC could be shared among different chips with a minimal
effort.  This meant a lot of code already ready.

For sure, the net-core, Ethtool, mdio parts were reused. Same for the
glue logic files.
For the latter, this helped to easily bring-up new platforms also
because the stmmac uses the HW cap register to auto-configure many
parts of the MAC core, DMA and modules. This helped many users, AFAIK.

For validation purpose, this is my experience, the stmmac helped
a lot because people used the same code to validate different HW
and it was easy to switch to a platform to another one in order to
verify / check if the support was ok or if a regression was introduced.
This is important for complex supports like PTP or EEE.

Hoping this can help.

Do not hesitate to contact me for further details

peppe

^ permalink raw reply

* [BUG] hdlcd gets confused about base address
From: Russell King - ARM Linux @ 2016-11-21 14:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121132419.GF1041@n2100.armlinux.org.uk>

On Mon, Nov 21, 2016 at 01:24:19PM +0000, Russell King - ARM Linux wrote:
> On Mon, Nov 21, 2016 at 12:56:53PM +0000, Liviu Dudau wrote:
> > That is mostly due to the check in hdlcd_crtc_disable() which I should
> > remove, I've added it because I was getting a ->disable() hook call
> > before any ->enable() was called at startup time. I need to revisit
> > this as I remember Daniel was commenting that this was not needed.
> 
> Removing that test results in:
> 
> [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:24:crtc-0] flip_done timed out
> 
> and the kernel hanging, seemingly in an IRQs-off region.

Annoyingly, enabling DRM debug prevents the kernel hanging...

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH v9 02/10] drm/mediatek: add *driver_data for different hardware settings
From: YT Shen @ 2016-11-21 14:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAGS+omAa_SLg8WKfXXvZ-UT6-Mdep28FQVJuuwfskNqP2x50eg@mail.gmail.com>

Hi Daniel,

On Fri, 2016-11-18 at 12:56 +0800, Daniel Kurtz wrote:
> Hi YT,
> 
> I don't see a reason to handle device_data in such a generic way at
> the generic mtk_ddp_comp layer.
> The device data is very component specific, so just define different
> structs for different comp types, ie:
> 
> struct mtk_disp_ovl_driver_data {
>     unsigned int reg_ovl_addr;
>     unsigned int fmt_rgb565;
>     unsigned int fmt_rgb888;
> };
> 
> struct mtk_disp_rdma_driver_data {
>     unsigned int fifo_pseudo_size;
> };
> 
> struct mtk_disp_color_driver_data {
>     unsigned int color_offset;
> };
> 
> Then add typed pointers to the local structs that use them, for example:
> 
> struct mtk_disp_ovl {
>         struct mtk_ddp_comp             ddp_comp;
>         struct drm_crtc                 *crtc;
>         const struct mtk_disp_ovl_driver_data *data;
> };
> 
> And fetch the device specific driver data directly in .probe, as you
> are already doing:
> 
> static int mtk_disp_ovl_probe(struct platform_device *pdev) {
>   ...
>   priv->data = of_device_get_match_data(dev);
>   ...
> }
These suggestions make code more readable.  We will change ovl and rdma
part, and keep mtk_disp_color_driver_data in its original place.
Because ovl and rdma have its files, other modules share
mtk_drm_ddp_comp.c.

> 
> More comments in-line...
> 
> On Fri, Nov 11, 2016 at 7:55 PM, YT Shen <yt.shen@mediatek.com> wrote:
> > There are some hardware settings changed, between MT8173 & MT2701:
> > DISP_OVL address offset changed, color format definition changed.
> > DISP_RDMA fifo size changed.
> > DISP_COLOR offset changed.
> > MIPI_TX pll setting changed.
> > And add prefix for mtk_ddp_main & mtk_ddp_ext & mutex_mod.
> 
> Nit: I think it would make sense to combine this patch with
> drm/mediatek: rename macros, add chip prefix
Will do.

> 
> >
> > Signed-off-by: YT Shen <yt.shen@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_ovl.c     | 27 ++++++++++++++++-----------
> >  drivers/gpu/drm/mediatek/mtk_disp_rdma.c    | 11 +++++++++--
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp.c      | 11 +++++++----
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 27 +++++++++++++++++++++------
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 13 +++++++++++++
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c      | 25 ++++++++++++++++++-------
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.h      |  8 ++++++++
> >  drivers/gpu/drm/mediatek/mtk_mipi_tx.c      | 24 +++++++++++++++++++++++-
> >  8 files changed, 115 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index 019b7ca..1139834 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -35,13 +35,10 @@
> >  #define DISP_REG_OVL_PITCH(n)                  (0x0044 + 0x20 * (n))
> >  #define DISP_REG_OVL_RDMA_CTRL(n)              (0x00c0 + 0x20 * (n))
> >  #define DISP_REG_OVL_RDMA_GMC(n)               (0x00c8 + 0x20 * (n))
> > -#define DISP_REG_OVL_ADDR(n)                   (0x0f40 + 0x20 * (n))
> 
> Also, I would still use the "#define macros", for example
> "DISP_REG_OVL_ADDR offsets, and use the named constant in the
> driver_data:
> 
> #define DISP_REG_OVL_ADDR_MT8173  0x0f40
> 
> (and in a later patch:
> #define DISP_REG_OVL_ADDR_MT2701  0x0040
> )
> 
> Also, I would still use the macro rather than open coding the "0x20 *
> (n)", and just pass 'ovl' to the overlay macros that depend on
> hardware type.
> Something like the following:
> 
> #define DISP_REG_OVL_ADDR(ovl, n)  ((ovl)->data->ovl_addr + 0x20 * (n))
Will use the "#define macros" here.

> 
> >
> >  #define        OVL_RDMA_MEM_GMC        0x40402020
> >
> >  #define OVL_CON_BYTE_SWAP      BIT(24)
> > -#define OVL_CON_CLRFMT_RGB565  (0 << 12)
> > -#define OVL_CON_CLRFMT_RGB888  (1 << 12)
> 
> This seems like a really random and unnecessary hardware change.
> Why chip designers, why!!?!?
There are many reasons for software bugs.  Unnecessary hardware change
should be one of them...

> 
> For this one, it seems the polarity is either one way or the other, so
> we can just use a bool to distinguish:
> 
>   bool fmt_rgb565_is_0;
> 
> > +static const struct mtk_ddp_comp_driver_data mt8173_ovl_driver_data = {
> > +       .ovl = { DISP_REG_OVL_ADDR_MT8173, .fmt_rgb565_is_0 = true }
> > +};
> 
> For use at runtime, the defines could become:
> 
> #define OVL_CON_CLRFMT_RGB565(ovl)  ((ovl)->data->fmt_rgb565_is_0 ? 0
> : OVL_CON_CLRFMT_RGB888)
> #define OVL_CON_CLRFMT_RGB888(ovl)  ((ovl)->data->fmt_rgb565_is_0 ?
> OVL_CON_CLRFMT_RGB888 : 0)
OK, will do.

> 
> >  #define OVL_CON_CLRFMT_RGBA8888        (2 << 12)
> >  #define OVL_CON_CLRFMT_ARGB8888        (3 << 12)
> >  #define        OVL_CON_AEN             BIT(8)
> > @@ -137,18 +134,18 @@ static void mtk_ovl_layer_off(struct mtk_ddp_comp *comp, unsigned int idx)
> >         writel(0x0, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx));
> >  }
> >
> > -static unsigned int ovl_fmt_convert(unsigned int fmt)
> > +static unsigned int ovl_fmt_convert(struct mtk_ddp_comp *comp, unsigned int fmt)
> >  {
> >         switch (fmt) {
> >         default:
> >         case DRM_FORMAT_RGB565:
> > -               return OVL_CON_CLRFMT_RGB565;
> > +               return comp->data->ovl.fmt_rgb565;
> 
> It will be nice to define a helper function for converting from the
> generic 'mtk_ddp_comp' to the specific 'mtk_disp_ovl':
> 
> static inline struct mtk_disp_ovl *comp_to_ovl(struct mtk_ddp_comp *comp) {
>   return container_of(comp, struct mtk_disp_ovl, ddp_comp);
> }
> 
> Then these could become:
>    return OVL_CON_CLRFMT_RGB565(comp_to_ovl(comp));
> 
> Or maybe cleaner, do the conversion once at the top of the function:
>     struct mtk_disp_ovl *ovl = comp_to_ovl(comp);
> 
> And then just:
>    return OVL_CON_CLRFMT_RGB565(ovl);
> 
> 
Will use a helper function for the converting.

> >         case DRM_FORMAT_BGR565:
> > -               return OVL_CON_CLRFMT_RGB565 | OVL_CON_BYTE_SWAP;
> > +               return comp->data->ovl.fmt_rgb565 | OVL_CON_BYTE_SWAP;
> >         case DRM_FORMAT_RGB888:
> > -               return OVL_CON_CLRFMT_RGB888;
> > +               return comp->data->ovl.fmt_rgb888;
> >         case DRM_FORMAT_BGR888:
> > -               return OVL_CON_CLRFMT_RGB888 | OVL_CON_BYTE_SWAP;
> > +               return comp->data->ovl.fmt_rgb888 | OVL_CON_BYTE_SWAP;
> >         case DRM_FORMAT_RGBX8888:
> >         case DRM_FORMAT_RGBA8888:
> >                 return OVL_CON_CLRFMT_ARGB8888;
> 
> [snip]
> 
> 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > index 1c366f8..935a8ef 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/delay.h>
> >  #include <linux/io.h>
> >  #include <linux/module.h>
> > +#include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/phy/phy.h>
> >
> > @@ -87,6 +88,9 @@
> >
> >  #define MIPITX_DSI_PLL_CON2    0x58
> >
> > +#define MIPITX_DSI_PLL_TOP     0x64
> > +#define RG_DSI_MPPLL_PRESERVE          (0xff << 8)
> > +
> >  #define MIPITX_DSI_PLL_PWR     0x68
> >  #define RG_DSI_MPPLL_SDM_PWR_ON                BIT(0)
> >  #define RG_DSI_MPPLL_SDM_ISO_EN                BIT(1)
> > @@ -123,10 +127,15 @@
> >  #define SW_LNT2_HSTX_PRE_OE            BIT(24)
> >  #define SW_LNT2_HSTX_OE                        BIT(25)
> >
> > +struct mtk_mipitx_data {
> > +       const u32 data;
> 
> Use a better name, like "mppll_preserve".
OK.

Regards,
yt.shen

> 
> Ok, that's it for now.
> Actually, the patch set in general looks pretty good.
> 
> -Dan

^ permalink raw reply

* Synopsys Ethernet QoS Driver
From: Giuseppe CAVALLARO @ 2016-11-21 14:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <BFA799B0-BB22-4515-AC14-3544A3167B0A@axis.com>

On 11/21/2016 2:28 PM, Lars Persson wrote:
>
>
>> 21 nov. 2016 kl. 13:53 skrev Giuseppe CAVALLARO <peppe.cavallaro@st.com>:
>>
>> Hello Joao
>>
>>> On 11/21/2016 1:32 PM, Joao Pinto wrote:
>>> Hello,
>>>
>>>> On 21-11-2016 05:29, Rayagond Kokatanur wrote:
>>>>> On Sat, Nov 19, 2016 at 7:26 PM, Rabin Vincent <rabin@rab.in> wrote:
>>>>>> On Fri, Nov 18, 2016 at 02:20:27PM +0000, Joao Pinto wrote:
>>>>>> For now we are interesting in improving the synopsys QoS driver under
>>>>>> /nect/ethernet/synopsys. For now the driver structure consists of a single file
>>>>>> called dwc_eth_qos.c, containing synopsys ethernet qos common ops and platform
>>>>>> related stuff.
>>>>>>
>>>>>> Our strategy would be:
>>>>>>
>>>>>> a) Implement a platform glue driver (dwc_eth_qos_pltfm.c)
>>>>>> b) Implement a pci glue driver (dwc_eth_qos_pci.c)
>>>>>> c) Implement a "core driver" (dwc_eth_qos.c) that would only have Ethernet QoS
>>>>>> related stuff to be reused by the platform / pci drivers
>>>>>> d) Add a set of features to the "core driver" that we have available internally
>>>>>
>>>>> Note that there are actually two drivers in mainline for this hardware:
>>>>>
>>>>> drivers/net/ethernet/synopsis/
>>>>> drivers/net/ethernet/stmicro/stmmac/
>>>>
>>>> Yes the later driver (drivers/net/ethernet/stmicro/stmmac/) supports
>>>> both 3.x and 4.x. It has glue layer for pci, platform, core etc,
>>>> please refer this driver once before you start.
>>>>
>>>> You can start adding missing feature of 4.x in stmmac driver.
>>>
>>> Thanks you all for all the info.
>>> Well, I think we are in a good position to organize the ethernet drivers
>>> concerning Synopsys IPs.
>>>
>>> First of all, in my opinion, it does not make sense to have a ethernet/synopsis
>>> (typo :)) when ethernet/stmicro is also for a synopsys IP. If we have another
>>> vendor using the same IP it should be able to reuse the commonn operations. But
>>> I would put that discussion for later :)
>>>
>>> For now I suggest that for we create ethernet/qos and create there a folder
>>> called dwc (designware controller) where all the synopsys qos IP specific code
>>> in order to be reused for example by ethernet/stmicro/stmmac/. We just have to
>>> figure out a clean interface for "client drivers" like stmmac to interact with
>>> the new qos driver.
>>>
>>> What do you think about this approach?
>>
>> The stmmac drivers run since many years on several platforms
>> (sh4, stm32, arm, x86, mips ...) and it supports an huge of amount of
>> configurations starting from 3.1x to 3.7x databooks.
>>
>> It also supports QoS hardware; for example, 4.00a, 4.10a and 4.20a
>> are fully working.
>>
>> Also the stmmac has platform, device-tree and pcie supports and
>> a lot of maintained glue-logic files.
>>
>> It is fully documented inside the kernel tree.
>>
>> I am happy to have new enhancements from other developers.
>> So, on my side, if you want to spend your time on improving it on your
>> platforms please feel free to do it!
>>
>> Concerning the stmicro/stmmac naming, these come from a really old
>> story and have no issue to adopt new folder/file names.
>>
>> I am also open to merge fixes and changes from ethernet/synopsis.
>> I want to point you on some benchmarks made by Alex some months ago
>> (IIRC) that showed an stmmac winner (due to the several optimizations
>> analyzed and reviewed in this mailing list).
>>
>> Peppe
>>
>
> Hello Joao and others,
>
> As the maintainer of dwc_eth_qos.c I prefer also that we put efforts on the most mature driver, the stmmac.
>
> I hope that the code can migrate into an ethernet/synopsys folder to keep the convention of naming the folder after the vendor. This makes it easy for others to find the driver.
>
> The dwc_eth_qos.c will eventually be removed and its DT binding interface can then be implemented in the stmmac driver.

Thanks Lars, I will be happy to support all you on this transition
and I agree on renaming all.

peppe


> - Lars
>
>>>
>>>
>>>>
>>>>>
>>>>> (See http://lists.openwall.net/netdev/2016/02/29/127)
>>>>>
>>>>> The former only supports 4.x of the hardware.
>>>>>
>>>>> The later supports 4.x and 3.x and already has a platform glue driver
>>>>> with support for several platforms, a PCI glue driver, and a core driver
>>>>> with several features not present in the former (for example: TX/RX
>>>>> interrupt coalescing, EEE, PTP).
>>>>>
>>>>> Have you evaluated both drivers?  Why have you decided to work on the
>>>>> former rather than the latter?
>>>>
>>>>
>>>
>>> Thanks.
>>>
>>>
>>>
>>>
>>
>

^ permalink raw reply

* [PATCH v16 07/15] clocksource/drivers/arm_arch_timer: Refactor arch_timer_detect_rate to keep dt code in *_of_init
From: Fu Wei @ 2016-11-21 14:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118195244.GL1197@leverpostej>

Hi Mark

On 19 November 2016 at 03:52, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Nov 16, 2016 at 09:49:00PM +0800, fu.wei at linaro.org wrote:
>> From: Fu Wei <fu.wei@linaro.org>
>>
>> The patch refactor original arch_timer_detect_rate function:
>>     (1) Separate out device-tree code, keep them in device-tree init
>>     function:
>>         arch_timer_of_init,
>>         arch_timer_mem_init;
>
> Please write a real commit message.

Sorry, will do

Since this patch will be separated into two patches.
the first patch will be separating out device-tree code, so commit
message can be:
-----------------
clocksource/drivers/arm_arch_timer: Separate out device-tree code from
arch_timer_detect_rate

Currently, the arch_timer_detect_rate can get system counter frequency
from device-tree, sysreg timers and MMIO timers. This is unnecessary and
confusing. For ACPI, we don't need a function included device-tree code.

This patch factors the device-tree related code out into device-tree
init function:
arch_timer_of_init and arch_timer_mem_init.
-----------------

the second patch will be split arch_timer_detect_rate in two, one is
for the MMIO timer,
another is for the CP15 timers, so commit message can be:
-----------------
clocksource/drivers/arm_arch_timer:split arch_timer_detect_rate for
the MMIO and CP15 timers

The arch_timer_detect_rate can get system counter frequency sysreg timers and
MMIO timers. This is unnecessary. For initializing sysreg timers, we
shouldn't try to
access MMIO timers.

This patch split arch_timer_detect_rate into two function:
arch_timer_detect_rate and arch_timer_mem_detect_rate.
-----------------

Please correct me if these commit message are inappropriate.
Great thanks

>
>>     (2) Improve original mechanism, if getting from memory-mapped timer
>>     fail, try arch_timer_get_cntfrq() again.
>
> This is *not* a refactoring. It's completely unrelated to the supposed
> refactoring from point (1), and if necessary, should be a separate
> patch.
>
> *Why* are you maknig this change? Does some ACPI platform have an MMIO
> timer with an ill-configured CNTFRQ register? If so, report that to the
> vendor. Don't add yet another needless bodge.
>
> I'd really like to split the MMIO and CP15 timers, and this is yet
> another hack that'll make it harder to do so.

you are right, I should split this founction for the MMIO and CP15 timers

>
>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++-------------
>>  1 file changed, 29 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
>> index af22953..fe4e812 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -487,27 +487,31 @@ static int arch_timer_starting_cpu(unsigned int cpu)
>>       return 0;
>>  }
>>
>> -static void
>> -arch_timer_detect_rate(void __iomem *cntbase, struct device_node *np)
>> +static void arch_timer_detect_rate(void __iomem *cntbase)
>>  {
>>       /* Who has more than one independent system counter? */
>>       if (arch_timer_rate)
>>               return;
>> -
>>       /*
>> -      * Try to determine the frequency from the device tree or CNTFRQ,
>> -      * if ACPI is enabled, get the frequency from CNTFRQ ONLY.
>> +      * If we got memory-mapped timer(cntbase != NULL),
>> +      * try to determine the frequency from CNTFRQ in memory-mapped timer.
>>        */
>
> *WHY* ?
>
> If we're sharing arch_timer_rate across MMIO and sysreg timers, the
> sysreg value is alreayd sufficient.

yes, we are sharing arch_timer_rate across MMIO and sysreg timers,
But for booting with device-tree, we can't make sure which timer will
be initialized first,
so we may need :
       if (arch_timer_rate)
               return;

>
> If we're not, they should be completely independent.
>
>> -     if (!acpi_disabled ||
>> -         of_property_read_u32(np, "clock-frequency", &arch_timer_rate)) {
>> -             if (cntbase)
>> -                     arch_timer_rate = readl_relaxed(cntbase + CNTFRQ);
>> -             else
>> -                     arch_timer_rate = arch_timer_get_cntfrq();
>> -     }
>> +     if (cntbase)
>> +             arch_timer_rate = readl_relaxed(cntbase + CNTFRQ);
>> +     /*
>> +      * Because in a system that implements both Secure and
>> +      * Non-secure states, CNTFRQ is only accessible in Secure state.
>
> That's true for the CNTCTLBase frame, but that doesn't matter.
>
> The CNTBase<n> frames should have a readable CNTFRQ.

Sorry, maybe I misunderstand the ARM doc, but in I3.5.7
CNTFRQ, Counter-timer Frequency, it says:
-----------------
For the CNTBaseN and CNTEL0BaseN frames:
? A RO copy of CNTFRQ is implemented in the CNTBaseN frame when both:
  ? CNTACR<n>.RFRQ is 1.
  ? Bit[0] of CNTTIDR.Frame<n> is 1.
  Otherwise the encoding in CNTBaseN is RES 0.
? When CNTFRQ is RO in the CNTBaseN frame, it is also RO in the
CNTEL0BaseN frame
  if bit[2] of CNTTIDR.Frame<n> is 1 and either:
  ? The value of CNTEL0ACR.EL0VCTEN is 1.
  ? The value of CNTEL0ACR.EL0PCTEN is 1.
  Otherwise the CNTFRQ encoding in CNTEL0BaseN frame is RES 0.
In a system that implements both Secure and Non-secure states, this
register is only accessible by
Secure accesses.
-----------------

So I think this is for both  CNTBaseN and CNTEL0BaseN frames?
Please correct me.

When I tested my patchset on Foundation model, I got "0" from
CNTBaseN's CNTFRQ, so mybe is not implemented?

>
>> +      * So the operation above may fail, even if (cntbase != NULL),
>> +      * especially on ARM64.
>> +      * In this case, we can try cntfrq_el0(system coprocessor register).
>> +      */
>> +     if (!arch_timer_rate)
>> +             arch_timer_rate = arch_timer_get_cntfrq();
>> +     else
>> +             return;
>
> Urrgh.
>
> Please have separate paths to determine the MMIO frequency and the
> sysreg frequency, and use the appropriate one for the counter you want
> to know the frequency of.

OK, will do

>
> Thanks,
> Mark.



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat

^ permalink raw reply

* [BUG] hdlcd gets confused about base address
From: Russell King - ARM Linux @ 2016-11-21 14:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161121135031.GK1005@e106497-lin.cambridge.arm.com>

On Mon, Nov 21, 2016 at 01:50:31PM +0000, Liviu Dudau wrote:
> On Mon, Nov 21, 2016 at 01:24:19PM +0000, Russell King - ARM Linux wrote:
> > On Mon, Nov 21, 2016 at 12:56:53PM +0000, Liviu Dudau wrote:
> > > That is mostly due to the check in hdlcd_crtc_disable() which I should
> > > remove, I've added it because I was getting a ->disable() hook call
> > > before any ->enable() was called at startup time. I need to revisit
> > > this as I remember Daniel was commenting that this was not needed.
> > 
> > Removing that test results in:
> > 
> > [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:24:crtc-0] flip_done timed out
> > 
> > and the kernel hanging, seemingly in an IRQs-off region.
> 
> Right, I need to sort this one out. Are you doing these tests out of
> some tagged branch that I can get in sync with?

No, not yet, and some of the changes I have are rather hacky.

I do always build my full tree of patches (which is currently running at
around 320 patches at the moment) but I never share that entire patch
set.  However, none of those touch i2c (apart from the ones I've recently
posted) and the only patches touching hdlcd are those I've posted so far.

Most of the problems I'm finding are through trying basic stuff - I'm not
doing anything special or unusual to find them, at the moment quite
literally just starting Xorg up and shutting it down.  For example, the
above was caused by logging in on serial, running:

	Xorg -terminate -verbose

and then hitting ^C.  (I have lxdm disabled so systemd boots to VT login
prompts on both the "framebuffer" and serial - I don't want Xorg coming
up when the machine is booting for its nightly KVM boot tests.)

I'm afraid that when I try someone elses code, I have a tendency to find
loads of seemingly trivial bugs when I try putting it through some basic
tests.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH v3] ARM: at91/dt: add dts file for sama5d36ek CMP board
From: Alexandre Belloni @ 2016-11-21 14:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479705283-3052-1-git-send-email-wenyou.yang@atmel.com>

Hi,

I fixed it up this time but please use the proper subject prefix next
time (ARM: dts: at91:).

On 21/11/2016 at 13:14:42 +0800, Wenyou Yang wrote :
> The sama5d36ek CMP board is the variant of sama5d3xek board.
> It is equipped with the low-power DDR2 SDRAM, PMIC ACT8865 and
> some power rail. Its main purpose is used to measure the power
> consumption.
> The difference of the sama5d36ek CMP dts from sama5d36ek dts is
> listed as below.
>  1. The USB host nodes are removed, that is, the USB host is disabled.
>  2. The gpio_keys node is added to wake up from the sleep.
>  3. The LCD isn't supported due to the pins for LCD are conflicted
>     with gpio_keys.
>  4. The adc0 node support the pinctrl sleep state to fix the over
>     consumption on VDDANA.
> 
> As said in errata, "When the USB host ports are used in high speed
> mode (EHCI), it is not possible to suspend the ports if no device is
> attached on each port. This leads to increased power consumption even
> if the system is in a low power mode." That is why the the USB host
> is disabled.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
> 
> Changes in v3:
>  - Use a dual license scheme for DT files.
>  - Use the proper model name and the compatible string to reflect
>    the nature of this new "CMP" board.
>  - Change name of wakeup property to "wakeup-source".
>  - Remove unnecessary comments.
>  - Remove bootargs.
> 
> Changes in v2:
>  - Add the pinctrl sleep state for adc0 node to fix the over
>    consumption on VDDANA.
>  - Improve the commit log.
> 
>  arch/arm/boot/dts/sama5d36ek_cmp.dts  |  87 ++++++++++
>  arch/arm/boot/dts/sama5d3xcm_cmp.dtsi | 201 +++++++++++++++++++++++
>  arch/arm/boot/dts/sama5d3xmb_cmp.dtsi | 301 ++++++++++++++++++++++++++++++++++
>  3 files changed, 589 insertions(+)
>  create mode 100644 arch/arm/boot/dts/sama5d36ek_cmp.dts
>  create mode 100644 arch/arm/boot/dts/sama5d3xcm_cmp.dtsi
>  create mode 100644 arch/arm/boot/dts/sama5d3xmb_cmp.dtsi
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH] ARM: dts: at91: sama5d3_uart: fix reg sizes to match documentation
From: Alexandre Belloni @ 2016-11-21 14:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478764000-32233-1-git-send-email-peda@axentia.se>

On 10/11/2016 at 08:46:40 +0100, Peter Rosin wrote :
> The new size (0x100) also matches the size given in sama5d3.dtsi
> 
> Documentation reference: section 43.6 "Universal Asynchronous
> Receiver Transmitter (UART) User Interface", table 43-4 "Register
> Mapping" in [1].
> 
> [1] Atmel-11121F-ATARM-SAMA5D3-Series-Datasheet_02-Feb-16
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  arch/arm/boot/dts/sama5d3_uart.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH v9 09/10] drm/mediatek: update DSI sub driver flow for sending commands to panel
From: YT Shen @ 2016-11-21 13:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAGS+omBCN5BpYZFozkjpnGPHUS2DUZw+B3Vo9LVZYyHLGD+iKQ@mail.gmail.com>

Hi Daniel,

Thanks for the review.

On Fri, 2016-11-18 at 11:21 +0800, Daniel Kurtz wrote:
> Hi YT,
> 
> Sorry for the very late review.
> 
> My biggest problem with this patch is it describes itself as adding
> support for a new use case "DSI -> panel", but makes many changes to
> the existing working flow "DSI -> bridge -> panel".
> If these changes are really needed, or improve the existing flow, I'd
> expect to see those changes added first in a preparatory patch,
> followed by a second smaller, simpler
> patch that adds any additional functionality required to enable the new flow.
We will split this patch into several smaller preparatory patches
necessary in the next version.

> 
> See detailed comments inline.
> 
> 
> On Fri, Nov 11, 2016 at 7:55 PM, YT Shen <yt.shen@mediatek.com> wrote:
> >
> > This patch update enable/disable flow of DSI module and MIPI TX module.
> > Original flow works on there is a bridge chip: DSI -> bridge -> panel.
> > In this case: DSI -> panel, the DSI sub driver flow should be updated.
> > We need to initialize DSI first so that we can send commands to panel.
> >
> > Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
> > Signed-off-by: YT Shen <yt.shen@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dsi.c     | 110 ++++++++++++++++++++++++++-------
> >  drivers/gpu/drm/mediatek/mtk_mipi_tx.c |  32 +++++-----
> >  2 files changed, 103 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index 860b84f..12a1206 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -94,6 +94,8 @@
> >  #define DSI_RACK               0x84
> >  #define RACK                           BIT(0)
> >
> > +#define DSI_MEM_CONTI          0x90
> > +
> >  #define DSI_PHY_LCCON          0x104
> >  #define LC_HS_TX_EN                    BIT(0)
> >  #define LC_ULPM_EN                     BIT(1)
> > @@ -126,6 +128,10 @@
> >  #define CLK_HS_POST                    (0xff << 8)
> >  #define CLK_HS_EXIT                    (0xff << 16)
> >
> > +#define DSI_VM_CMD_CON         0x130
> > +#define VM_CMD_EN                      BIT(0)
> > +#define TS_VFP_EN                      BIT(5)
> > +
> >  #define DSI_CMDQ0              0x180
> >  #define CONFIG                         (0xff << 0)
> >  #define SHORT_PACKET                   0
> > @@ -219,12 +225,12 @@ static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi)
> >         writel(timcon3, dsi->regs + DSI_PHY_TIMECON3);
> >  }
> >
> > -static void mtk_dsi_enable(struct mtk_dsi *dsi)
> > +static void mtk_dsi_engine_enable(struct mtk_dsi *dsi)
> 
> I don't think we need to change these names.
OK.

> 
> >  {
> >         mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, DSI_EN);
> >  }
> >
> > -static void mtk_dsi_disable(struct mtk_dsi *dsi)
> > +static void mtk_dsi_engine_disable(struct mtk_dsi *dsi)
> >  {
> >         mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, 0);
> >  }
> > @@ -249,7 +255,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> >          * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
> >          * we set mipi_ratio is 1.05.
> >          */
> > -       dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
> > +       dsi->data_rate = dsi->vm.pixelclock * 12 * 21;
> > +       dsi->data_rate /= (dsi->lanes * 1000 * 10);
> > +       dev_info(dev, "set mipitx's data rate: %dMHz\n", dsi->data_rate);
> 
> I don't think we want to spam the log like this.  Use dev_dbg.... or
> use the DRM_() messaging like elsewhere in this driver?
OK, we will remove logs like this in the patch series.

> 
> >
> >         ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
> >         if (ret < 0) {
> > @@ -271,7 +279,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> >                 goto err_disable_engine_clk;
> >         }
> >
> > -       mtk_dsi_enable(dsi);
> > +       mtk_dsi_engine_enable(dsi);
> >         mtk_dsi_reset_engine(dsi);
> >         mtk_dsi_phy_timconfig(dsi);
> >
> > @@ -289,7 +297,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> >  static void mtk_dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
> >  {
> >         mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
> > -       mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
> > +       mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, LC_ULPM_EN);
> 
> What does this change do?
> It looks like a pure bug fix (ie, previoulsy we were'nt actually
> enabling ULP MODE before).
> If so, can you please move it to a separate preliminary patch.
OK.

> 
> >  }
> >
> >  static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
> > @@ -302,7 +310,7 @@ static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
> >  static void mtk_dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
> >  {
> >         mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_HS_TX_EN, 0);
> > -       mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
> > +       mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, LD0_ULPM_EN);
> 
> Same here.
> 
> >  }
> >
> >  static void mtk_dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
> > @@ -338,11 +346,21 @@ static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
> >                 if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) &&
> >                     !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
> >                         vid_mode = BURST_MODE;
> > +               else
> > +                       vid_mode = SYNC_EVENT_MODE;
> 
> So, when do we use SYNC_PULSE_MODE (set just before the 'if')?
We will update this part.

> 
> >         }
> >
> >         writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
> >  }
> >
> > +static void mtk_dsi_set_vm_cmd(struct mtk_dsi *dsi)
> > +{
> > +       writel(0x3c, dsi->regs + DSI_MEM_CONTI);
> 
> Please use #defined constants, especially if this register is a bit field.
> Also, this looks like new behavior which doesn't seem related to
> changing the enable order.
> If this is a general fix, please use a separate patch.
We will remove this part.  This change is not necessary.

> 
> > +
> > +       mtk_dsi_mask(dsi, DSI_VM_CMD_CON, VM_CMD_EN, VM_CMD_EN);
> > +       mtk_dsi_mask(dsi, DSI_VM_CMD_CON, TS_VFP_EN, TS_VFP_EN);
> > +}
> > +
> >  static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
> >  {
> >         struct videomode *vm = &dsi->vm;
> > @@ -399,6 +417,9 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
> >                 break;
> >         }
> >
> > +       tmp_reg |= (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) << 6;
> > +       tmp_reg |= (dsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET) >> 3;
> > +
> 
> ditto
> 
> >         writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
> >  }
> >
> > @@ -477,6 +498,16 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
> >         writel(1, dsi->regs + DSI_START);
> >  }
> >
> > +static void mtk_dsi_stop(struct mtk_dsi *dsi)
> > +{
> > +       writel(0, dsi->regs + DSI_START);
> > +}
> > +
> > +static void mtk_dsi_set_cmd_mode(struct mtk_dsi *dsi)
> > +{
> > +       writel(CMD_MODE, dsi->regs + DSI_MODE_CTRL);
> > +}
> > +
> >  static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> >  {
> >         u32 inten = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
> > @@ -506,7 +537,7 @@ static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
> >         if (ret == 0) {
> >                 dev_info(dsi->dev, "Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
> >
> > -               mtk_dsi_enable(dsi);
> > +               mtk_dsi_engine_enable(dsi);
> >                 mtk_dsi_reset_engine(dsi);
> >         }
> >
> > @@ -535,6 +566,17 @@ static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> >         return IRQ_HANDLED;
> >  }
> >
> > +static s32 mtk_dsi_switch_to_cmd_mode(struct mtk_dsi *dsi, u8 irq_flag, u32 t)
> > +{
> > +       mtk_dsi_irq_data_clear(dsi, irq_flag);
> > +       mtk_dsi_set_cmd_mode(dsi);
> > +
> > +       if (!mtk_dsi_wait_for_irq_done(dsi, irq_flag, t))
> > +               return -1;
> 
> No, use a real linux errno, and return an int, and print an error
> message if this is unexpected.
Will use a real errno: ETIME.

> 
> > +       else
> > +               return 0;
> > +}
> > +
> >  static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
> >  {
> >         if (WARN_ON(dsi->refcount == 0))
> > @@ -543,11 +585,6 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
> >         if (--dsi->refcount != 0)
> >                 return;
> >
> > -       mtk_dsi_lane0_ulp_mode_enter(dsi);
> > -       mtk_dsi_clk_ulp_mode_enter(dsi);
> > -
> > -       mtk_dsi_disable(dsi);
> > -
> >         clk_disable_unprepare(dsi->engine_clk);
> >         clk_disable_unprepare(dsi->digital_clk);
> >
> > @@ -561,35 +598,45 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
> >         if (dsi->enabled)
> >                 return;
> >
> > -       if (dsi->panel) {
> > -               if (drm_panel_prepare(dsi->panel)) {
> > -                       DRM_ERROR("failed to setup the panel\n");
> > -                       return;
> > -               }
> > -       }
> > -
> >         ret = mtk_dsi_poweron(dsi);
> >         if (ret < 0) {
> >                 DRM_ERROR("failed to power on dsi\n");
> >                 return;
> >         }
> >
> > +       usleep_range(20000, 21000);
> > +
> 
> Why are you adding a 20 ms delay where there was none before?
After checking, we will remove redundant codes and the delay.

> 
> >         mtk_dsi_rxtx_control(dsi);
> > +       mtk_dsi_phy_timconfig(dsi);
> > +       mtk_dsi_ps_control_vact(dsi);
> > +       mtk_dsi_set_vm_cmd(dsi);
> > +       mtk_dsi_config_vdo_timing(dsi);
> > +       mtk_dsi_set_interrupt_enable(dsi);
> >
> > +       mtk_dsi_engine_enable(dsi);
> >         mtk_dsi_clk_ulp_mode_leave(dsi);
> >         mtk_dsi_lane0_ulp_mode_leave(dsi);
> >         mtk_dsi_clk_hs_mode(dsi, 0);
> > -       mtk_dsi_set_mode(dsi);
> >
> > -       mtk_dsi_ps_control_vact(dsi);
> > -       mtk_dsi_config_vdo_timing(dsi);
> > -       mtk_dsi_set_interrupt_enable(dsi);
> > +       if (dsi->panel) {
> > +               if (drm_panel_prepare(dsi->panel)) {
> > +                       DRM_ERROR("failed to prepare the panel\n");
> > +                       return;
> > +               }
> > +       }
> >
> >         mtk_dsi_set_mode(dsi);
> >         mtk_dsi_clk_hs_mode(dsi, 1);
> >
> >         mtk_dsi_start(dsi);
> >
> > +       if (dsi->panel) {
> > +               if (drm_panel_enable(dsi->panel)) {
> > +                       DRM_ERROR("failed to enable the panel\n");
> 
> In case of error, you must undo everything done to this point.  At least:
>  (1) unprepare the panel
>  (2) stop dsi
>  (3) poweroff dsi
OK.

> 
> > +                       return;
> > +               }
> > +       }
> > +
> >         dsi->enabled = true;
> >  }
> >
> > @@ -605,6 +652,21 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
> >                 }
> >         }
> >
> > +       mtk_dsi_stop(dsi);
> > +       mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500);
> 
> This function can return an error, so please check it.  Although,
> there probably isn't much you can do here about it.
OK.

> 
> > +
> > +       if (dsi->panel) {
> > +               if (drm_panel_unprepare(dsi->panel)) {
> > +                       DRM_ERROR("failed to unprepare the panel\n");
> > +                       return;
> 
> I think you should probably just ignore this error and continue
> disabling dsi, since it isn't really recoverable and you can't roll
> back and re-enable dsi.
OK.

> 
> 
> > +               }
> > +       }
> > +
> > +       mtk_dsi_reset_engine(dsi);
> > +       mtk_dsi_lane0_ulp_mode_enter(dsi);
> > +       mtk_dsi_clk_ulp_mode_enter(dsi);
> > +       mtk_dsi_engine_disable(dsi);
> > +
> >         mtk_dsi_poweroff(dsi);
> >
> >         dsi->enabled = false;
> > @@ -845,7 +907,7 @@ static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi)
> >         if (timeout_ms == 0) {
> >                 dev_info(dsi->dev, "polling dsi wait not busy timeout!\n");
> >
> > -               mtk_dsi_enable(dsi);
> > +               mtk_dsi_engine_enable(dsi);
> >                 mtk_dsi_reset_engine(dsi);
> >         }
> >  }
> > diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > index 108d31a..34e95c6 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > @@ -177,7 +177,9 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
> >
> >         dev_dbg(mipi_tx->dev, "prepare: %u Hz\n", mipi_tx->data_rate);
> >
> > -       if (mipi_tx->data_rate >= 500000000) {
> > +       if (mipi_tx->data_rate > 1250000000) {
> > +               return -EINVAL;
> > +       } else if (mipi_tx->data_rate >= 500000000) {
> 
> Capping the max data rate looks like an unrelated fix.
Will prepare additional patch for max data rate.

> 
> >                 txdiv = 1;
> >                 txdiv0 = 0;
> >                 txdiv1 = 0;
> > @@ -201,6 +203,10 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
> >                 return -EINVAL;
> >         }
> >
> > +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
> > +                               RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
> > +                               (8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
> > +
> >         mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_BG_CON,
> >                                 RG_DSI_VOUT_MSK |
> >                                 RG_DSI_BG_CKEN | RG_DSI_BG_CORE_EN,
> > @@ -210,24 +216,18 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
> >
> >         usleep_range(30, 100);
> >
> > -       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
> > -                               RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
> > -                               (8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
> > -
> > -       mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_CON,
> > -                            RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);
> > +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_CON,
> > +                               RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN,
> > +                               RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);
> 
> Changing from set_bits to update_bits does not do anything.  Please
> leave this alone.
OK.

> 
> >
> >         mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_PWR,
> >                                 RG_DSI_MPPLL_SDM_PWR_ON |
> >                                 RG_DSI_MPPLL_SDM_ISO_EN,
> >                                 RG_DSI_MPPLL_SDM_PWR_ON);
> >
> > -       mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> > -                              RG_DSI_MPPLL_PLL_EN);
> > -
> 
> Why don't you need to disable the PLL first now?
Yes, we need.  Will fix this.

> 
> >         mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> > -                               RG_DSI_MPPLL_TXDIV0 | RG_DSI_MPPLL_TXDIV1 |
> > -                               RG_DSI_MPPLL_PREDIV,
> > +                               RG_DSI_MPPLL_PREDIV | RG_DSI_MPPLL_TXDIV0 |
> > +                               RG_DSI_MPPLL_TXDIV1 | RG_DSI_MPPLL_POSDIV,
> >                                 (txdiv0 << 3) | (txdiv1 << 5));
> 
> If I read this right, the only thing you are changing is clearing
> "RG_DSI_MPPLL_POSDIV".
> This would be more clear if you kept the field order: TXDIV0, TXDIV1, PREDIV.
> 
> And why are you making this change in this patch?
Hmm, we will provide another patch for this part if necessary.
Sometimes settings are changed not in kernel stage (maybe display from
bootloader)  This change just make sure kernel have the right
configuration.

> 
> 
> >
> >         /*
> > @@ -242,10 +242,12 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
> >                       26000000);
> >         writel(pcw, mipi_tx->regs + MIPITX_DSI_PLL_CON2);
> >
> > -       mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
> > -                            RG_DSI_MPPLL_SDM_FRA_EN);
> > +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
> > +                               RG_DSI_MPPLL_SDM_FRA_EN,
> > +                               RG_DSI_MPPLL_SDM_FRA_EN);
> 
> AFAICT, this change does not do anything but make the code more confusing.
OK.

> 
> >
> > -       mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON0, RG_DSI_MPPLL_PLL_EN);
> > +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> > +                               RG_DSI_MPPLL_PLL_EN, RG_DSI_MPPLL_PLL_EN);
> 
> AFAICT, this change does not do anything but make the code more confusing.
OK.

> 
> >
> >         usleep_range(20, 100);
> >
> > --
> > 1.9.1
> >

^ permalink raw reply

* [PATCH] PCI: Add information about describing PCI in ACPI
From: Bjorn Helgaas @ 2016-11-21 13:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAJZ5v0h3ax4+UCm57VYqV3SVzOZ3A5Ajykks10J49rj1O4VkvQ@mail.gmail.com>

On Sat, Nov 19, 2016 at 12:02:24AM +0100, Rafael J. Wysocki wrote:
> On Thu, Nov 17, 2016 at 6:59 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > Add a writeup about how PCI host bridges should be described in ACPI
> > using PNP0A03/PNP0A08 devices, PNP0C02 devices, and the MCFG table.
> >
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> Looks great overall, but I have a few comments (below).

Thanks a lot for taking a look, Rafael!  I applied all your suggestions.

Bjorn

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox