Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/2] media: helene: add I2C device probe function
From: Katsuhiro Suzuki @ 2018-05-23  0:27 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds I2C probe function to use dvb_module_probe()
with this driver.

Signed-off-by: Katsuhiro Suzuki <suzuki.katsuhiro@socionext.com>

---

Changes since v2:
  - Nothing

Changes since v1:
  - Add documents for dvb_frontend member of helene_config
---
 drivers/media/dvb-frontends/helene.c | 88 ++++++++++++++++++++++++++--
 drivers/media/dvb-frontends/helene.h |  3 +
 2 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c
index a0d0b53c91d7..04033f0c278b 100644
--- a/drivers/media/dvb-frontends/helene.c
+++ b/drivers/media/dvb-frontends/helene.c
@@ -666,7 +666,7 @@ static int helene_set_params_s(struct dvb_frontend *fe)
 	return 0;
 }
 
-static int helene_set_params(struct dvb_frontend *fe)
+static int helene_set_params_t(struct dvb_frontend *fe)
 {
 	u8 data[MAX_WRITE_REGSIZE];
 	u32 frequency;
@@ -835,6 +835,19 @@ static int helene_set_params(struct dvb_frontend *fe)
 	return 0;
 }
 
+static int helene_set_params(struct dvb_frontend *fe)
+{
+	struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+
+	if (p->delivery_system == SYS_DVBT ||
+	    p->delivery_system == SYS_DVBT2 ||
+	    p->delivery_system == SYS_ISDBT ||
+	    p->delivery_system == SYS_DVBC_ANNEX_A)
+		return helene_set_params_t(fe);
+
+	return helene_set_params_s(fe);
+}
+
 static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
 {
 	struct helene_priv *priv = fe->tuner_priv;
@@ -843,7 +856,7 @@ static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency)
 	return 0;
 }
 
-static const struct dvb_tuner_ops helene_tuner_ops = {
+static const struct dvb_tuner_ops helene_tuner_ops_t = {
 	.info = {
 		.name = "Sony HELENE Ter tuner",
 		.frequency_min = 1000000,
@@ -853,7 +866,7 @@ static const struct dvb_tuner_ops helene_tuner_ops = {
 	.init = helene_init,
 	.release = helene_release,
 	.sleep = helene_sleep,
-	.set_params = helene_set_params,
+	.set_params = helene_set_params_t,
 	.get_frequency = helene_get_frequency,
 };
 
@@ -871,6 +884,20 @@ static const struct dvb_tuner_ops helene_tuner_ops_s = {
 	.get_frequency = helene_get_frequency,
 };
 
+static const struct dvb_tuner_ops helene_tuner_ops = {
+	.info = {
+		.name = "Sony HELENE Sat/Ter tuner",
+		.frequency_min = 500000,
+		.frequency_max = 1200000000,
+		.frequency_step = 1000,
+	},
+	.init = helene_init,
+	.release = helene_release,
+	.sleep = helene_sleep,
+	.set_params = helene_set_params,
+	.get_frequency = helene_get_frequency,
+};
+
 /* power-on tuner
  * call once after reset
  */
@@ -1032,7 +1059,7 @@ struct dvb_frontend *helene_attach(struct dvb_frontend *fe,
 	if (fe->ops.i2c_gate_ctrl)
 		fe->ops.i2c_gate_ctrl(fe, 0);
 
-	memcpy(&fe->ops.tuner_ops, &helene_tuner_ops,
+	memcpy(&fe->ops.tuner_ops, &helene_tuner_ops_t,
 			sizeof(struct dvb_tuner_ops));
 	fe->tuner_priv = priv;
 	dev_info(&priv->i2c->dev,
@@ -1042,6 +1069,59 @@ struct dvb_frontend *helene_attach(struct dvb_frontend *fe,
 }
 EXPORT_SYMBOL(helene_attach);
 
+static int helene_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct helene_config *config = client->dev.platform_data;
+	struct dvb_frontend *fe = config->fe;
+	struct device *dev = &client->dev;
+	struct helene_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->i2c_address = client->addr;
+	priv->i2c = client->adapter;
+	priv->set_tuner_data = config->set_tuner_priv;
+	priv->set_tuner = config->set_tuner_callback;
+	priv->xtal = config->xtal;
+
+	if (fe->ops.i2c_gate_ctrl)
+		fe->ops.i2c_gate_ctrl(fe, 1);
+
+	if (helene_x_pon(priv) != 0)
+		return -EINVAL;
+
+	if (fe->ops.i2c_gate_ctrl)
+		fe->ops.i2c_gate_ctrl(fe, 0);
+
+	memcpy(&fe->ops.tuner_ops, &helene_tuner_ops,
+	       sizeof(struct dvb_tuner_ops));
+	fe->tuner_priv = priv;
+	i2c_set_clientdata(client, priv);
+
+	dev_info(dev, "Sony HELENE attached on addr=%x at I2C adapter %p\n",
+		 priv->i2c_address, priv->i2c);
+
+	return 0;
+}
+
+static const struct i2c_device_id helene_id[] = {
+	{ "helene", },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, helene_id);
+
+static struct i2c_driver helene_driver = {
+	.driver = {
+		.name = "helene",
+	},
+	.probe    = helene_probe,
+	.id_table = helene_id,
+};
+module_i2c_driver(helene_driver);
+
 MODULE_DESCRIPTION("Sony HELENE Sat/Ter tuner driver");
 MODULE_AUTHOR("Abylay Ospan <aospan@netup.ru>");
 MODULE_LICENSE("GPL");
diff --git a/drivers/media/dvb-frontends/helene.h b/drivers/media/dvb-frontends/helene.h
index c9fc81c7e4e7..8562d01bc93e 100644
--- a/drivers/media/dvb-frontends/helene.h
+++ b/drivers/media/dvb-frontends/helene.h
@@ -39,6 +39,7 @@ enum helene_xtal {
  * @set_tuner_callback:	Callback function that notifies the parent driver
  *			which tuner is active now
  * @xtal: Cristal frequency as described by &enum helene_xtal
+ * @fe: Frontend for which connects this tuner
  */
 struct helene_config {
 	u8	i2c_address;
@@ -46,6 +47,8 @@ struct helene_config {
 	void	*set_tuner_priv;
 	int	(*set_tuner_callback)(void *, int);
 	enum helene_xtal xtal;
+
+	struct dvb_frontend *fe;
 };
 
 #if IS_REACHABLE(CONFIG_DVB_HELENE)
-- 
2.17.0

^ permalink raw reply related

* [PATCH net] net: phy: broadcom: Fix bcm_write_exp()
From: Andrew Lunn @ 2018-05-23  0:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180523000450.9384-1-f.fainelli@gmail.com>

On Tue, May 22, 2018 at 05:04:49PM -0700, Florian Fainelli wrote:
> On newer PHYs, we need to select the expansion register to write with
> setting bits [11:8] to 0xf. This was done correctly by bcm7xxx.c prior
> to being migrated to generic code under bcm-phy-lib.c which
> unfortunately used the older implementation from the BCM54xx days.

Hi Florian

Does selecting the expansion register affect access to the standard
registers? Does this need locking like the Marvell PHY has when
changing pages?

Thanks
	 Andrew

^ permalink raw reply

* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Doug Anderson @ 2018-05-23  0:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <a106c337-0713-e5d6-cb40-13e05f4d361d@codeaurora.org>

Hi,

On Tue, May 22, 2018 at 3:46 PM, David Collins <collinsd@codeaurora.org> wrote:
> On 05/22/2018 09:43 AM, Doug Anderson wrote:
>> On Mon, May 21, 2018 at 5:00 PM, David Collins <collinsd@codeaurora.org> wrote:
> ...
>>> Returning the cached (but not sent) initial voltage equal to the min
>>> constraint voltage in get_voltage() calls should not cause any problems.
>>> This represents the highest voltage that is guaranteed to be output by the
>>> regulator.  Consumer's should call regulator_set_voltage() to specify
>>> their voltage needs.  If they simply call regulator_enable(), then the
>>> cached voltage will be sent along with the enable request.
>>
>> I'm still not seeing the argument for initial-voltage here.  If we
>> added a feature like you describe where we don't send the voltage
>> until the first enable couldn't we use the minimum voltage here?  If a
>> consumer calls regulator_enable() without setting a voltage (which
>> seems like a terrible idea for something where the voltage could vary)
>> then it would end up at the minimum voltage.
>
> I wasn't proposing the voltage caching feature to be used in the upstream
> qcom-rpmh-regulator.  I was explaining exactly how the downstream
> rpmh-regulator driver works.
>
> However, if the voltage caching feature is acceptable for upstream usage,
> then I could add it.  With that in place, I see less of a need for the
> qcom,regulator-initial-microvolt property and would be ok with removing it
> for now.

I think it's the right thing to do an Mark didn't seem to yell, so I'd
say go for it.


>> OK, so how's this for a proposal then:
>>
>> 1. For RPMh-regulator whenever we see a "set voltage" but Linux hasn't
>> specified that the regulator is enabled then we don't send the
>> voltage, we just cache it.
>>
>> 2. When we see the first enable then we first send the cached voltage
>> and then do the enable.
>>
>> 3. We don't need an "initial voltage" because any rails that are
>> expected to be variable voltage the client should be choosing a
>> voltage.
>>
>>
>> ...taking the SD card case as an example: if the regulator framework
>> says "set to the minimum: 1.8V" we'll cache this but not apply it yet
>> because the rail is off as far as Linux is concerned.  Then when the
>> SD card framework starts up it will set the voltage to 3.3V which will
>> overwrite the cache.  Then the SD card framework will enable the
>> regulator and RPMh will set the voltage to 3.3V and enable.
>
> I am ok with implementing this feature.
>
> However, should the voltage be cached instead of sent immediately any time
> that Linux has not explicitly requested the regulator to be enabled, or
> only before the first time that an enable request is sent?
>
> 1. regulator_set_voltage(reg, 2960000, 2960000)
>    --> cache voltage=2960 mV
> 2. regulator_enable(reg)
>    --> Send voltage=2960 then enable=1
> 3. regulator_disable(reg)
>    --> Send enable=0
> 4. regulator_set_voltage(reg, 1800000, 2960000)
>    --> A. Send voltage=1800 OR B. cache voltage=1800?
>
> Option A is used on the downstream rpmh-regulator driver in order to avoid
> cases where AP votes to disable a regulator that is kept enabled by
> another subsystem but then does not lower the voltage requested thanks to
> regulator_set_voltage() being called after regulator_disable().  I plan to
> go with option A for qcom-rpmh-regulator unless there are objections.

So one client's vote for a voltage continues to be in effect even if
that client votes to have the regulator disabled?  That seems
fundamentally broken in RPMh.  I guess my take would be to work around
this RPMh misfeature by saying that whenever Linux votes to disable a
regulator it also votes for a voltage of 0.  Then another client of
RPMh won't be affected.

That seems like it would be beneficial in any case.  If the AP always
asks for 1.3 V and the modem always asks for 1.1 V for the same rail
then the rail should go down to 1.1 V when the AP says to disable the
rail.


>> This whole thing makes me worry about another problem, though.  You
>> say that the bootloader left the SD card rail on, right?  ...but as
>> far as Linux is concerned the SD card rail is off (argh, it can't read
>> the state that the bootloader left the rail in!)
>>
>> ...now imagine any of the following:
>>
>> * The user boots up a kernel where the SD card driver is disabled.
>>
>> * The user ejects the SD card right after the bootloader used it but
>> before the kernel driver started.
>>
>> When the kernel comes up it will believe that the SD card rail is
>> disabled so it won't try to disable it.  So the voltage will be left
>> on.
>
> We have not encountered issues with regulators getting left on
> indefinitely because Linux devices failed to take control of them during
> boot.  I don't think that this hypothetical issue needs to be addressed in
> the first qcom-rpmh-regulator driver patch if at all.

I don't think it hypothetical at all.  Linux takes control of a
regulator and then at bootup it disables all regulators that aren't
currently used/enabled.  In your case you will report that regulators
are already disabled and thus you'll neuter the kernel's attempt to
disable regulators that nobody is using but that might have been left
on by the bootloader.  It seemed like Mark agreed here.


>> You can even come up with a less contrived use case here.  One could
>> argue that the SD card framework really ought to be ensuring VMMC and
>> VQMMC are off before it starts tweaking with them just in case the
>> bootloader left them on.  Thus, it should do:
>>
>> A) Turn off VMMC and VQMMC
>> B) Program VMMC and VQMMC to defaults
>> C) Turn on VMMC and VQMMC
>>
>> ...right now we bootup and pretend to Linux that VMMC and VQMMC start
>> off, so step A) will be no-op.  Sigh.
>
> Step A) would not work because the regulator's use_count would be 0 and
> regulator_disable() can only be called successfully if use_count > 0.  The
> call would have no impact and it would return an error.

Are you sure regulator_force_disable() won't do the trick on most
boards (which will report the regulator being enabled at bootup)?  I
haven't tried it, but it seems like it might.

...I think you're right that this is a theoretical case at the moment
because I don't think the MMC framework attempts to get this right,
but I don't have time to dig right now.  I think it just sets the
voltage to 3.3V and turns the rail on and if the card thinks it should
be at 1.8V because the bootloader left the card in that state then:
whoops.  I'd have to walk through the regulator framework more closely
to confirm that though.  Certainly on all other boards besides ones
using RPMh the bootloader can leave regulators on and the kernel can
query that state.  It seems sane that there would be some way to turn
a regulator in this state off.  I know for a fact that if you just
leave the regulator alone (don't claim it or try to enable it) that
Linux will turn it off.


> I don't think that this is an avenue that we want to pursue.  Consumers
> should not be expected to call regulator_disable() before regulator_enable().
>
>
>> Do we need to have ".is_enabled" return -ENOTRECOVERABLE to help the
>> regulator framework understand that we don't know the state?  I think
>> that might require a pile of patches to the regulator framework, but
>> it can't be helped unless we can somehow get RPMh to give us back the
>> status of how the bootloader left us (if we had that, we could return
>> 0 for anything the bootloader didn't touch and that would be correct
>> from the point of view of the AP).
>
> I'm not following what the regulator framework would do as a result of
> is_enabled() returning -ENOTRECOVERABLE.  If it saw this while processing
> a regulator_enable() call then it would continue to enable the regulator.

The important use case here is at bootup when we try to disable all
unused regulators.  We need the framework to know that these
regulators might not be disabled so it should go ahead and try to
disable them.


> This value could not be seen while handling a regulator_disable() call
> since the is_enabled() callback is not invoked in the disable call-path.
> This also seems like an optimization for a problem that we are not
> encountering now (or likely to ever encounter).  Therefore, I would
> suggest that we not try to work this into the initial qcom-rpmh-regulator
> patch.

I think you haven't thought through the bootup case of disabling all
unused regulators.  When you look at that path I think you'll find
it's important to make sure that the regulator framework knows that
you don't know if you're disabled or enabled yet.  I think you want to
look at regulator_late_cleanup().  Note that right now
regulator_late_cleanup() doesn't handle error codes from is_enabled
(actually, several places in the regulator core don't), but actually
since the error case and the "enabled" case are probably the same this
might be OK.


-Doug

^ permalink raw reply

* [PATCH net] net: phy: broadcom: Fix bcm_write_exp()
From: Florian Fainelli @ 2018-05-23  0:04 UTC (permalink / raw)
  To: linux-arm-kernel

On newer PHYs, we need to select the expansion register to write with
setting bits [11:8] to 0xf. This was done correctly by bcm7xxx.c prior
to being migrated to generic code under bcm-phy-lib.c which
unfortunately used the older implementation from the BCM54xx days.

Fix this by creating an inline stub: bcm_write_exp_sel() which adds the
correct value (MII_BCM54XX_EXP_SEL_ER) and update both the Cygnus PHY
and BCM7xxx PHY drivers which require setting these bits.

broadcom.c is unchanged because some PHYs even use a different selector
method, so let them specify it directly (e.g: SerDes secondary selector).

Fixes: a1cba5613edf ("net: phy: Add Broadcom phy library for common interfaces")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
David, please also queue this one up for -stable, thanks!

 drivers/net/phy/bcm-cygnus.c  | 6 +++---
 drivers/net/phy/bcm-phy-lib.h | 7 +++++++
 drivers/net/phy/bcm7xxx.c     | 4 ++--
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/bcm-cygnus.c b/drivers/net/phy/bcm-cygnus.c
index 6838129839ca..e757b09f1889 100644
--- a/drivers/net/phy/bcm-cygnus.c
+++ b/drivers/net/phy/bcm-cygnus.c
@@ -61,17 +61,17 @@ static int bcm_cygnus_afe_config(struct phy_device *phydev)
 		return rc;
 
 	/* make rcal=100, since rdb default is 000 */
-	rc = bcm_phy_write_exp(phydev, MII_BRCM_CORE_EXPB1, 0x10);
+	rc = bcm_phy_write_exp_sel(phydev, MII_BRCM_CORE_EXPB1, 0x10);
 	if (rc < 0)
 		return rc;
 
 	/* CORE_EXPB0, Reset R_CAL/RC_CAL Engine */
-	rc = bcm_phy_write_exp(phydev, MII_BRCM_CORE_EXPB0, 0x10);
+	rc = bcm_phy_write_exp_sel(phydev, MII_BRCM_CORE_EXPB0, 0x10);
 	if (rc < 0)
 		return rc;
 
 	/* CORE_EXPB0, Disable Reset R_CAL/RC_CAL Engine */
-	rc = bcm_phy_write_exp(phydev, MII_BRCM_CORE_EXPB0, 0x00);
+	rc = bcm_phy_write_exp_sel(phydev, MII_BRCM_CORE_EXPB0, 0x00);
 
 	return 0;
 }
diff --git a/drivers/net/phy/bcm-phy-lib.h b/drivers/net/phy/bcm-phy-lib.h
index 7c73808cbbde..81cceaa412fe 100644
--- a/drivers/net/phy/bcm-phy-lib.h
+++ b/drivers/net/phy/bcm-phy-lib.h
@@ -14,11 +14,18 @@
 #ifndef _LINUX_BCM_PHY_LIB_H
 #define _LINUX_BCM_PHY_LIB_H
 
+#include <linux/brcmphy.h>
 #include <linux/phy.h>
 
 int bcm_phy_write_exp(struct phy_device *phydev, u16 reg, u16 val);
 int bcm_phy_read_exp(struct phy_device *phydev, u16 reg);
 
+static inline int bcm_phy_write_exp_sel(struct phy_device *phydev,
+					u16 reg, u16 val)
+{
+	return bcm_phy_write_exp(phydev, reg | MII_BCM54XX_EXP_SEL_ER, val);
+}
+
 int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val);
 int bcm54xx_auxctl_read(struct phy_device *phydev, u16 regnum);
 
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index 29b1c88b55cc..01d2ff2f6241 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -65,10 +65,10 @@ struct bcm7xxx_phy_priv {
 static void r_rc_cal_reset(struct phy_device *phydev)
 {
 	/* Reset R_CAL/RC_CAL Engine */
-	bcm_phy_write_exp(phydev, 0x00b0, 0x0010);
+	bcm_phy_write_exp_sel(phydev, 0x00b0, 0x0010);
 
 	/* Disable Reset R_AL/RC_CAL Engine */
-	bcm_phy_write_exp(phydev, 0x00b0, 0x0000);
+	bcm_phy_write_exp_sel(phydev, 0x00b0, 0x0000);
 }
 
 static int bcm7xxx_28nm_b0_afe_config_init(struct phy_device *phydev)
-- 
2.14.1

^ permalink raw reply related

* [PATCH] arm64: Make sure permission updates happen for pmd/pud
From: Laura Abbott @ 2018-05-22 23:50 UTC (permalink / raw)
  To: linux-arm-kernel

Commit 15122ee2c515 ("arm64: Enforce BBM for huge IO/VMAP mappings")
disallowed block mappings for ioremap since that code does not honor
break-before-make. The same APIs are also used for permission updating
though and the extra checks prevent the permission updates from happening,
even though this should be permitted. This results in read-only permissions
not being fully applied. Visibly, this can occasionaly be seen as a failure
on the built in rodata test when the test data ends up in a section or
as an odd RW gap on the page table dump. Fix this by keeping the check
for the top level p*d_set_huge APIs but using separate functions for the
update APIs.

Reported-by: Peter Robinson <pbrobinson@gmail.com>
Fixes: 15122ee2c515 ("arm64: Enforce BBM for huge IO/VMAP mappings")
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 arch/arm64/mm/mmu.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 2dbb2c9f1ec1..57517ad86910 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -66,6 +66,9 @@ static pte_t bm_pte[PTRS_PER_PTE] __page_aligned_bss;
 static pmd_t bm_pmd[PTRS_PER_PMD] __page_aligned_bss __maybe_unused;
 static pud_t bm_pud[PTRS_PER_PUD] __page_aligned_bss __maybe_unused;
 
+static void __pmd_set_huge(pmd_t *pmdp, phys_addr_t phys, pgprot_t prot);
+static void __pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot);
+
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 			      unsigned long size, pgprot_t vma_prot)
 {
@@ -200,7 +203,7 @@ static void init_pmd(pud_t *pudp, unsigned long addr, unsigned long end,
 		/* try section mapping first */
 		if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
 		    (flags & NO_BLOCK_MAPPINGS) == 0) {
-			pmd_set_huge(pmdp, phys, prot);
+			__pmd_set_huge(pmdp, phys, prot);
 
 			/*
 			 * After the PMD entry has been populated once, we
@@ -299,7 +302,7 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end,
 		 */
 		if (use_1G_block(addr, next, phys) &&
 		    (flags & NO_BLOCK_MAPPINGS) == 0) {
-			pud_set_huge(pudp, phys, prot);
+			__pud_set_huge(pudp, phys, prot);
 
 			/*
 			 * After the PUD entry has been populated once, we
@@ -929,31 +932,40 @@ int __init arch_ioremap_pmd_supported(void)
 	return 1;
 }
 
-int pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
+void __pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
 {
 	pgprot_t sect_prot = __pgprot(PUD_TYPE_SECT |
 					pgprot_val(mk_sect_prot(prot)));
 
+	BUG_ON(phys & ~PUD_MASK);
+	set_pud(pudp, pfn_pud(__phys_to_pfn(phys), sect_prot));
+}
+
+int pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
+{
 	/* ioremap_page_range doesn't honour BBM */
 	if (pud_present(READ_ONCE(*pudp)))
 		return 0;
 
-	BUG_ON(phys & ~PUD_MASK);
-	set_pud(pudp, pfn_pud(__phys_to_pfn(phys), sect_prot));
+	__pud_set_huge(pudp, phys, prot);
 	return 1;
 }
 
-int pmd_set_huge(pmd_t *pmdp, phys_addr_t phys, pgprot_t prot)
+static void __pmd_set_huge(pmd_t *pmdp, phys_addr_t phys, pgprot_t prot)
 {
 	pgprot_t sect_prot = __pgprot(PMD_TYPE_SECT |
 					pgprot_val(mk_sect_prot(prot)));
+	BUG_ON(phys & ~PMD_MASK);
+	set_pmd(pmdp, pfn_pmd(__phys_to_pfn(phys), sect_prot));
+}
 
+int pmd_set_huge(pmd_t *pmdp, phys_addr_t phys, pgprot_t prot)
+{
 	/* ioremap_page_range doesn't honour BBM */
 	if (pmd_present(READ_ONCE(*pmdp)))
 		return 0;
 
-	BUG_ON(phys & ~PMD_MASK);
-	set_pmd(pmdp, pfn_pmd(__phys_to_pfn(phys), sect_prot));
+	__pmd_set_huge(pmdp, phys, prot);
 	return 1;
 }
 
-- 
2.17.0

^ permalink raw reply related

* [PATCH v2 07/40] iommu: Add a page fault handler
From: Jacob Pan @ 2018-05-22 23:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8a640794-a6f3-fa01-82a9-06479a6f779a@arm.com>

On Mon, 21 May 2018 15:49:40 +0100
Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:

> On 18/05/18 19:04, Jacob Pan wrote:
> >> +struct iopf_context {
> >> +	struct device			*dev;
> >> +	struct iommu_fault_event	evt;
> >> +	struct list_head		head;
> >> +};
> >> +
> >> +struct iopf_group {
> >> +	struct iopf_context		last_fault;
> >> +	struct list_head		faults;
> >> +	struct work_struct		work;
> >> +};
> >> +  
> > All the page requests in the group should belong to the same device,
> > perhaps struct device tracking should be in iopf_group instead of
> > iopf_context?  
> 
> Right, this is a leftover from when we kept a global list of partial
> faults. Since the list is now per-device, I can move the dev pointer
> (I think I should also rename iopf_context to iopf_fault, if that's
> all right)
> 
> >> +static int iopf_complete(struct device *dev, struct
> >> iommu_fault_event *evt,
> >> +			 enum page_response_code status)
> >> +{
> >> +	struct page_response_msg resp = {
> >> +		.addr			= evt->addr,
> >> +		.pasid			= evt->pasid,
> >> +		.pasid_present		= evt->pasid_valid,
> >> +		.page_req_group_id	=
> >> evt->page_req_group_id,
> >> +		.private_data		= evt->iommu_private,
> >> +		.resp_code		= status,
> >> +	};
> >> +
> >> +	return iommu_page_response(dev, &resp);
> >> +}
> >> +
> >> +static enum page_response_code
> >> +iopf_handle_single(struct iopf_context *fault)
> >> +{
> >> +	/* TODO */
> >> +	return -ENODEV;
> >> +}
> >> +
> >> +static void iopf_handle_group(struct work_struct *work)
> >> +{
> >> +	struct iopf_group *group;
> >> +	struct iopf_context *fault, *next;
> >> +	enum page_response_code status = IOMMU_PAGE_RESP_SUCCESS;
> >> +
> >> +	group = container_of(work, struct iopf_group, work);
> >> +
> >> +	list_for_each_entry_safe(fault, next, &group->faults,
> >> head) {
> >> +		struct iommu_fault_event *evt = &fault->evt;
> >> +		/*
> >> +		 * Errors are sticky: don't handle subsequent
> >> faults in the
> >> +		 * group if there is an error.
> >> +		 */  
> > There might be performance benefit for certain device to continue in
> > spite of error in that the ATS retry may have less fault. Perhaps a
> > policy decision for later enhancement.  
> 
> Yes I think we should leave it to future work. My current reasoning is
> that subsequent requests are more likely to fail as well and reporting
> the error is more urgent, but we need real workloads to confirm this.
> 
> >> +		if (status == IOMMU_PAGE_RESP_SUCCESS)
> >> +			status = iopf_handle_single(fault);
> >> +
> >> +		if (!evt->last_req)
> >> +			kfree(fault);
> >> +	}
> >> +
> >> +	iopf_complete(group->last_fault.dev,
> >> &group->last_fault.evt, status);
> >> +	kfree(group);
> >> +}
> >> +
> >> +/**
> >> + * iommu_queue_iopf - IO Page Fault handler
> >> + * @evt: fault event
> >> + * @cookie: struct device, passed to
> >> iommu_register_device_fault_handler.
> >> + *
> >> + * Add a fault to the device workqueue, to be handled by mm.
> >> + */
> >> +int iommu_queue_iopf(struct iommu_fault_event *evt, void *cookie)
> >> +{
> >> +	struct iopf_group *group;
> >> +	struct iopf_context *fault, *next;
> >> +	struct iopf_device_param *iopf_param;
> >> +
> >> +	struct device *dev = cookie;
> >> +	struct iommu_param *param = dev->iommu_param;
> >> +
> >> +	if (WARN_ON(!mutex_is_locked(&param->lock)))
> >> +		return -EINVAL;
> >> +
> >> +	if (evt->type != IOMMU_FAULT_PAGE_REQ)
> >> +		/* Not a recoverable page fault */
> >> +		return IOMMU_PAGE_RESP_CONTINUE;
> >> +
> >> +	/*
> >> +	 * As long as we're holding param->lock, the queue can't
> >> be unlinked
> >> +	 * from the device and therefore cannot disappear.
> >> +	 */
> >> +	iopf_param = param->iopf_param;
> >> +	if (!iopf_param)
> >> +		return -ENODEV;
> >> +
> >> +	if (!evt->last_req) {
> >> +		fault = kzalloc(sizeof(*fault), GFP_KERNEL);
> >> +		if (!fault)
> >> +			return -ENOMEM;
> >> +
> >> +		fault->evt = *evt;
> >> +		fault->dev = dev;
> >> +
> >> +		/* Non-last request of a group. Postpone until the
> >> last one */
> >> +		list_add(&fault->head, &iopf_param->partial);
> >> +
> >> +		return IOMMU_PAGE_RESP_HANDLED;
> >> +	}
> >> +
> >> +	group = kzalloc(sizeof(*group), GFP_KERNEL);
> >> +	if (!group)
> >> +		return -ENOMEM;
> >> +
> >> +	group->last_fault.evt = *evt;
> >> +	group->last_fault.dev = dev;
> >> +	INIT_LIST_HEAD(&group->faults);
> >> +	list_add(&group->last_fault.head, &group->faults);
> >> +	INIT_WORK(&group->work, iopf_handle_group);
> >> +
> >> +	/* See if we have partial faults for this group */
> >> +	list_for_each_entry_safe(fault, next,
> >> &iopf_param->partial, head) {
> >> +		if (fault->evt.page_req_group_id ==
> >> evt->page_req_group_id)  
> > If all 9 bit group index are used, there can be lots of PRGs. For
> > future enhancement, maybe we can have per group partial list ready
> > to go when LPIG comes in? I will be less searching.  
> 
> Yes, allocating the PRG from the start could be more efficient. I
> think it's slightly more complicated code so I'd rather see
> performance numbers before implementing it
> 
> >> +			/* Insert *before* the last fault */
> >> +			list_move(&fault->head, &group->faults);
> >> +	}
> >> +  
> > If you already sorted the group list with last fault at the end,
> > why do you need a separate entry to track the last fault?  
> 
> Not sure I understand your question, sorry. Do you mean why the
> iopf_group.last_fault? Just to avoid one more kzalloc.
> 
kind of :) what i thought was that why can't the last_fault naturally
be the last entry in your group fault list? then there is no need for
special treatment in terms of allocation of the last fault. just my
preference.
> >> +
> >> +	queue->flush(queue->flush_arg, dev);
> >> +
> >> +	/*
> >> +	 * No need to clear the partial list. All PRGs containing
> >> the PASID that
> >> +	 * needs to be decommissioned are whole (the device driver
> >> made sure of
> >> +	 * it before this function was called). They have been
> >> submitted to the
> >> +	 * queue by the above flush().
> >> +	 */  
> > So you are saying device driver need to make sure LPIG PRQ is
> > submitted in the flush call above such that partial list is
> > cleared?  
> 
> Not exactly, it's the IOMMU driver that makes sure all LPIG in its
> queues are submitted by the above flush call. In more details the
> flow is:
> 
> * Either device driver calls unbind()/sva_device_shutdown(), or the
> process exits.
> * If the device driver called, then it already told the device to stop
> using the PASID. Otherwise we use the mm_exit() callback to tell the
> device driver to stop using the PASID.
> * In either case, when receiving a stop request from the driver, the
> device sends the LPIGs to the IOMMU queue.
> * Then, the flush call above ensures that the IOMMU reports the LPIG
> with iommu_report_device_fault.
> * While submitting all LPIGs for this PASID to the work queue,
> ipof_queue_fault also picked up all partial faults, so the partial
> list is clean.
> 
> Maybe I should improve this comment?
> 
thanks for explaining. LPIG submission is done by device asynchronously
w.r.t. driver stopping/decommission PASID. so if we were to use this
flow on vt-d, which does not stall page request queue, then we should
use the iommu model specific flush() callback to ensure LPIG is
received? There could be queue full condition and retry. I am just
trying to understand how and where we can make sure LPIG is
received and all groups are whole.

> > what if
> > there are device failures where device needs to reset (either whole
> > function level or PASID level), should there still be a need to
> > clear the partial list for all associated PASID/group?  
> 
> I guess the simplest way out, when resetting the device, is for the
> device driver to call iommu_sva_device_shutdown(). Both the IOMMU
> driver's sva_device_shutdown() and remove_device() ops should call
> iopf_queue_remove_device(), which clears the partial list.
> 
yes. I think that should work for functional reset.
> Thanks,
> Jean

[Jacob Pan]

^ permalink raw reply

* [PATCH 08/14] ARM: spectre-v2: harden user aborts in kernel space
From: Russell King - ARM Linux @ 2018-05-22 23:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e52b3541-7106-a6dc-5851-01b091bec38f@arm.com>

On Tue, May 22, 2018 at 06:15:02PM +0100, Marc Zyngier wrote:
> On 21/05/18 12:45, Russell King wrote:
> > +	switch (read_cpuid_part()) {
> > +	case ARM_CPU_PART_CORTEX_A8:
> > +	case ARM_CPU_PART_CORTEX_A9:
> > +	case ARM_CPU_PART_CORTEX_A12:
> > +	case ARM_CPU_PART_CORTEX_A17:
> > +	case ARM_CPU_PART_CORTEX_A73:
> > +	case ARM_CPU_PART_CORTEX_A75:
> > +		harden_branch_predictor = harden_branch_predictor_bpiall;
> > +		spectre_v2_method = "BPIALL";
> > +		break;
> 
> You don't seem to take into account the PFR0.CSV2 field which indicates
> that the CPU has a branch predictor that is immune to Spectre-v2.

That information is not covered in the description of the vulnerability
- the published information on the security-updates site states that
BPIALL is required without stating any conditions.

> See for example the Cortex-A75 r3p0 TRM[1].

So which cores should such a test be applied to?  As I mention,
the support site doesn't give this detail.  That brings up the
obvious question: what else does the web page miss out on?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply

* [PATCH 3/5] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Ray Jui @ 2018-05-22 23:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522205457.GA16363@roeck-us.net>

Hi Guenter,

On 5/22/2018 1:54 PM, Guenter Roeck wrote:
> On Tue, May 22, 2018 at 11:47:18AM -0700, Ray Jui wrote:
>> If the watchdog hardware is already enabled during the boot process,
>> when the Linux watchdog driver loads, it should reset the watchdog and
>> tell the watchdog framework. As a result, ping can be generated from
>> the watchdog framework, until the userspace watchdog daemon takes over
>> control
>>
>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> ---
>>   drivers/watchdog/sp805_wdt.c | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> index 1484609..408ffbe 100644
>> --- a/drivers/watchdog/sp805_wdt.c
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -42,6 +42,7 @@
>>   	/* control register masks */
>>   	#define	INT_ENABLE	(1 << 0)
>>   	#define	RESET_ENABLE	(1 << 1)
>> +	#define	ENABLE_MASK	(INT_ENABLE | RESET_ENABLE)
>>   #define WDTINTCLR		0x00C
>>   #define WDTRIS			0x010
>>   #define WDTMIS			0x014
>> @@ -74,6 +75,18 @@ module_param(nowayout, bool, 0);
>>   MODULE_PARM_DESC(nowayout,
>>   		"Set to 1 to keep watchdog running after device release");
>>   
>> +/* returns true if wdt is running; otherwise returns false */
>> +static bool wdt_is_running(struct watchdog_device *wdd)
>> +{
>> +	struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> +
>> +	if ((readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK) ==
>> +	    ENABLE_MASK)
>> +		return true;
>> +	else
>> +		return false;
> 
> 	return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
> 

Note ENABLE_MASK contains two bits (INT_ENABLE and RESET_ENABLE); 
therefore, a simple !!(expression) would not work? That is, the masked 
result needs to be compared against the mask again to ensure both bits 
are set, right?

Thanks,

Ray

^ permalink raw reply

* [RFC PATCH 09/10] arm64: dts: rk3399: Add dfi and dmc nodes.
From: Rob Herring @ 2018-05-22 22:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180514211610.26618-10-enric.balletbo@collabora.com>

On Mon, May 14, 2018 at 11:16:09PM +0200, Enric Balletbo i Serra wrote:
> From: Lin Huang <hl@rock-chips.com>
> 
> These are required to support DDR DVFS on rk3399 platform. The patch also
> introduces two new files (rk3399-dram.h and rk3399-dram-default-timing)
> with default DRAM settings.
> 
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>  .../rockchip/rk3399-dram-default-timing.dtsi  | 38 ++++++++++
>  arch/arm64/boot/dts/rockchip/rk3399-dram.h    | 73 +++++++++++++++++++
>  .../boot/dts/rockchip/rk3399-op1-opp.dtsi     | 29 ++++++++
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi      | 20 +++++
>  4 files changed, 160 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-dram-default-timing.dtsi
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-dram.h
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-dram-default-timing.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-dram-default-timing.dtsi
> new file mode 100644
> index 000000000000..4dfe3e1d8bdf
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-dram-default-timing.dtsi
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR X11)
> +/*
> + * Copyright (c) 2016-2018, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * Author: Lin Huang <hl@rock-chips.com>
> + */
> +
> +#include "rk3399-dram.h"
> +
> +rockchip,ddr3_speed_bin = <21>;
> +rockchip,pd_idle = <0x40>;
> +rockchip,sr_idle = <0x2>;

Don't do includes this way please. These should go under a node.

> +rockchip,sr_mc_gate_idle = <0x3>;
> +rockchip,srpd_lite_idle	= <0x4>;
> +rockchip,standby_idle = <0x2000>;
> +rockchip,dram_dll_dis_freq = <300000000>;
> +rockchip,phy_dll_dis_freq = <125000000>;
> +rockchip,auto_pd_dis_freq = <666000000>;
> +rockchip,ddr3_odt_dis_freq = <333000000>;
> +rockchip,ddr3_drv = <DDR3_DS_40ohm>;
> +rockchip,ddr3_odt = <DDR3_ODT_120ohm>;
> +rockchip,phy_ddr3_ca_drv = <PHY_DRV_ODT_40>;
> +rockchip,phy_ddr3_dq_drv = <PHY_DRV_ODT_40>;
> +rockchip,phy_ddr3_odt = <PHY_DRV_ODT_240>;
> +rockchip,lpddr3_odt_dis_freq = <333000000>;
> +rockchip,lpddr3_drv = <LP3_DS_34ohm>;
> +rockchip,lpddr3_odt = <LP3_ODT_240ohm>;
> +rockchip,phy_lpddr3_ca_drv = <PHY_DRV_ODT_40>;
> +rockchip,phy_lpddr3_dq_drv = <PHY_DRV_ODT_40>;
> +rockchip,phy_lpddr3_odt = <PHY_DRV_ODT_240>;
> +rockchip,lpddr4_odt_dis_freq = <333000000>;
> +rockchip,lpddr4_drv = <LP4_PDDS_60ohm>;
> +rockchip,lpddr4_dq_odt = <LP4_DQ_ODT_40ohm>;
> +rockchip,lpddr4_ca_odt = <LP4_CA_ODT_40ohm>;
> +rockchip,phy_lpddr4_ca_drv = <PHY_DRV_ODT_40>;
> +rockchip,phy_lpddr4_ck_cs_drv = <PHY_DRV_ODT_80>;
> +rockchip,phy_lpddr4_dq_drv = <PHY_DRV_ODT_80>;
> +rockchip,phy_lpddr4_odt = <PHY_DRV_ODT_60>;

^ permalink raw reply

* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: David Collins @ 2018-05-22 22:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAD=FV=XARtQNo5Cyg78XuT26JUFgn=7BjWRnXPjP566=a-sF1w@mail.gmail.com>

On 05/22/2018 09:43 AM, Doug Anderson wrote:
> On Mon, May 21, 2018 at 5:00 PM, David Collins <collinsd@codeaurora.org> wrote:
...
>> Returning the cached (but not sent) initial voltage equal to the min
>> constraint voltage in get_voltage() calls should not cause any problems.
>> This represents the highest voltage that is guaranteed to be output by the
>> regulator.  Consumer's should call regulator_set_voltage() to specify
>> their voltage needs.  If they simply call regulator_enable(), then the
>> cached voltage will be sent along with the enable request.
> 
> I'm still not seeing the argument for initial-voltage here.  If we
> added a feature like you describe where we don't send the voltage
> until the first enable couldn't we use the minimum voltage here?  If a
> consumer calls regulator_enable() without setting a voltage (which
> seems like a terrible idea for something where the voltage could vary)
> then it would end up at the minimum voltage.

I wasn't proposing the voltage caching feature to be used in the upstream
qcom-rpmh-regulator.  I was explaining exactly how the downstream
rpmh-regulator driver works.

However, if the voltage caching feature is acceptable for upstream usage,
then I could add it.  With that in place, I see less of a need for the
qcom,regulator-initial-microvolt property and would be ok with removing it
for now.


>>> BTW: have I already said how terrible of a design it is that you can't
>>> read back the voltages that the BIOS set?  If we could just read back
>>> what the BIOS set then everything would work great and we could stop
>>> talking about this.
>>
>> Even if such reading were feasible, it would not help the situation
>> substantially.  Very few requests are made via the AP RSC before Linux
>> kernel boot, so 0 V values would still be read back for most regulators.
> 
> Sure, but all the regulators we're talking about are ones where this
> would help.  Said another way: are there any rails that are not
> touched by the bootloader where it's bad to set the minimum voltage?

I'm not sure about this.


> OK, so how's this for a proposal then:
> 
> 1. For RPMh-regulator whenever we see a "set voltage" but Linux hasn't
> specified that the regulator is enabled then we don't send the
> voltage, we just cache it.
> 
> 2. When we see the first enable then we first send the cached voltage
> and then do the enable.
> 
> 3. We don't need an "initial voltage" because any rails that are
> expected to be variable voltage the client should be choosing a
> voltage.
> 
> 
> ...taking the SD card case as an example: if the regulator framework
> says "set to the minimum: 1.8V" we'll cache this but not apply it yet
> because the rail is off as far as Linux is concerned.  Then when the
> SD card framework starts up it will set the voltage to 3.3V which will
> overwrite the cache.  Then the SD card framework will enable the
> regulator and RPMh will set the voltage to 3.3V and enable.

I am ok with implementing this feature.

However, should the voltage be cached instead of sent immediately any time
that Linux has not explicitly requested the regulator to be enabled, or
only before the first time that an enable request is sent?

1. regulator_set_voltage(reg, 2960000, 2960000)
   --> cache voltage=2960 mV
2. regulator_enable(reg)
   --> Send voltage=2960 then enable=1
3. regulator_disable(reg)
   --> Send enable=0
4. regulator_set_voltage(reg, 1800000, 2960000)
   --> A. Send voltage=1800 OR B. cache voltage=1800?

Option A is used on the downstream rpmh-regulator driver in order to avoid
cases where AP votes to disable a regulator that is kept enabled by
another subsystem but then does not lower the voltage requested thanks to
regulator_set_voltage() being called after regulator_disable().  I plan to
go with option A for qcom-rpmh-regulator unless there are objections.


> This whole thing makes me worry about another problem, though.  You
> say that the bootloader left the SD card rail on, right?  ...but as
> far as Linux is concerned the SD card rail is off (argh, it can't read
> the state that the bootloader left the rail in!)
> 
> ...now imagine any of the following:
> 
> * The user boots up a kernel where the SD card driver is disabled.
> 
> * The user ejects the SD card right after the bootloader used it but
> before the kernel driver started.
> 
> When the kernel comes up it will believe that the SD card rail is
> disabled so it won't try to disable it.  So the voltage will be left
> on.

We have not encountered issues with regulators getting left on
indefinitely because Linux devices failed to take control of them during
boot.  I don't think that this hypothetical issue needs to be addressed in
the first qcom-rpmh-regulator driver patch if at all.


> You can even come up with a less contrived use case here.  One could
> argue that the SD card framework really ought to be ensuring VMMC and
> VQMMC are off before it starts tweaking with them just in case the
> bootloader left them on.  Thus, it should do:
> 
> A) Turn off VMMC and VQMMC
> B) Program VMMC and VQMMC to defaults
> C) Turn on VMMC and VQMMC
> 
> ...right now we bootup and pretend to Linux that VMMC and VQMMC start
> off, so step A) will be no-op.  Sigh.

Step A) would not work because the regulator's use_count would be 0 and
regulator_disable() can only be called successfully if use_count > 0.  The
call would have no impact and it would return an error.

I don't think that this is an avenue that we want to pursue.  Consumers
should not be expected to call regulator_disable() before regulator_enable().


> Do we need to have ".is_enabled" return -ENOTRECOVERABLE to help the
> regulator framework understand that we don't know the state?  I think
> that might require a pile of patches to the regulator framework, but
> it can't be helped unless we can somehow get RPMh to give us back the
> status of how the bootloader left us (if we had that, we could return
> 0 for anything the bootloader didn't touch and that would be correct
> from the point of view of the AP).

I'm not following what the regulator framework would do as a result of
is_enabled() returning -ENOTRECOVERABLE.  If it saw this while processing
a regulator_enable() call then it would continue to enable the regulator.
This value could not be seen while handling a regulator_disable() call
since the is_enabled() callback is not invoked in the disable call-path.
This also seems like an optimization for a problem that we are not
encountering now (or likely to ever encounter).  Therefore, I would
suggest that we not try to work this into the initial qcom-rpmh-regulator
patch.

Thanks,
David

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* [RFC PATCH 02/10] dt-bindings: devfreq: rk3399_dmc: Add rockchip,pmu phandle.
From: Rob Herring @ 2018-05-22 22:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180514211610.26618-3-enric.balletbo@collabora.com>

On Mon, May 14, 2018 at 11:16:02PM +0200, Enric Balletbo i Serra wrote:
> The Rockchip DMC (Dynamic Memory Interface) needs to access to the PMU
> general register files to know the DRAM type, so add a phandle to the
> syscon that manages these registers.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>  Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH 1/2] ARM: dts: imx51-babbage: Fix USB PHY duplicate unit-address
From: Rob Herring @ 2018-05-22 22:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180515080624.GR26863@dragon>

On Tue, May 15, 2018 at 04:06:26PM +0800, Shawn Guo wrote:
> On Mon, May 14, 2018 at 03:29:35PM -0300, Fabio Estevam wrote:
> > From: Fabio Estevam <fabio.estevam@nxp.com>
> > 
> > Currently the following DTC warning is seen with W=1:
> > 
> > arch/arm/boot/dts/imx51-babbage.dtb: Warning (unique_unit_address): /usbphy/usbphy at 0: duplicate unit-address (also used in node /usbphy/usbh1phy at 0)
> > 
> > Fix it by moving the USB PHY node outside of simple-bus and drop the
> > unneeded unit-address.
> > 
> > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> > ---
> >  arch/arm/boot/dts/imx51-babbage.dts | 21 +++++++--------------
> >  1 file changed, 7 insertions(+), 14 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
> > index b8ca73d..de46906 100644
> > --- a/arch/arm/boot/dts/imx51-babbage.dts
> > +++ b/arch/arm/boot/dts/imx51-babbage.dts
> > @@ -170,20 +170,13 @@
> >  		mux-ext-port = <3>;
> >  	};
> >  
> > -	usbphy {
> > -		#address-cells = <1>;
> > -		#size-cells = <0>;
> > -		compatible = "simple-bus";
> > -
> > -		usbh1phy: usbh1phy at 0 {
> > -			compatible = "usb-nop-xceiv";
> > -			reg = <0>;
> > -			clocks = <&clk_usb>;
> > -			clock-names = "main_clk";
> > -			reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
> > -			vcc-supply = <&vusb_reg>;
> > -			#phy-cells = <0>;
> > -		};
> > +	usbh1phy: usbphy1 {
> > +		compatible = "usb-nop-xceiv";
> > +		clocks = <&clk_usb>;
> > +		clock-names = "main_clk";
> > +		reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
> > +		vcc-supply = <&vusb_reg>;
> > +		#phy-cells = <0>;
> 
> This should be considered as a whole together with usbphy in imx51.dtsi.
> Also, I would like to get some input from DT folks on how we should name
> the node uniquely.  @Rob.

'usbphy1' is fine. I don't have a better suggestion...

Rob

^ permalink raw reply

* [PATCH 2/2] soc: bcm: brcmstb: Add missing DDR MEMC compatible strings
From: Rob Herring @ 2018-05-22 22:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180511220242.837-3-f.fainelli@gmail.com>

On Fri, May 11, 2018 at 03:02:42PM -0700, Florian Fainelli wrote:
> We would not be matching the following chip/compatible strings
> combinations, which would lead to not setting the warm boot flag
> correctly, fix that:
> 
>     7260A0/B0: brcm,brcmstb-memc-ddr-rev-b.2.1
>     7255A0: brcm,brcmstb-memc-ddr-rev-b.2.3
>     7278Bx: brcm,brcmstb-memc-ddr-rev-b.3.1
> 
> The B2.1 core (which is in 7260 A0 and B0) doesn't have the
> SHIMPHY_ADDR_CNTL_0_DDR_PAD_CNTRL setup in the memsys init code, nor
> does it have the warm boot flag re-definition on entry. Those changes
> were for B2.2 and later MEMSYS cores. Fall back to the previous S2/S3
> entry method for these specific chips.
> 
> Fixes: 0b741b8234c8 ("soc: bcm: brcmstb: Add support for S2/S3/S5 suspend states (ARM)")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt |  3 +++
>  drivers/soc/bcm/brcmstb/pm/pm-arm.c                        | 12 ++++++++++++
>  2 files changed, 15 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH 1/2] soc: bcm: brcmstb: pm: Add support for newer rev B3.0 controllers
From: Rob Herring @ 2018-05-22 22:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180511220242.837-2-f.fainelli@gmail.com>

On Fri, May 11, 2018 at 03:02:41PM -0700, Florian Fainelli wrote:
> From: Doug Berger <opendmb@gmail.com>
> 
> Update the Device Tree binding document and add a matching entry for the
> MEMC DDR controller revision B3.0 which is found on chips like 7278A0
> and newer.
> 
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> [florian: tweak commit message, make it apply to upstream kernel]
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt | 1 +
>  drivers/soc/bcm/brcmstb/pm/pm-arm.c                        | 4 ++++
>  2 files changed, 5 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Side note: this should really move out of bindings/arm/ to 
bindings/memory-controllers/ or at least to its own file.

Rob

^ permalink raw reply

* [PATCH 6/9] ARM: dts: wheat: Drop MTD partitioning from DT
From: Marek Vasut @ 2018-05-22 22:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdXRnoyhNz=3v_=ZCV=FWX4TwKqx7MpH0g3c7GwtXKP1tg@mail.gmail.com>

On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote:
> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> Drop the MTD partitioning from DT, since it does not describe HW
>> and to give way to a more flexible kernel command line partition
>> passing.
>>
>> To retain the original partitioning, assure you have enabled
>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
>> following to your kernel command line:
>>
>>   mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)
> 
> I think the "@0" can be dropped, as it's optional?
> 4m?

My take on this is that the loader is actually at offset 0x0 of the MTD
device and we explicitly state that in the mtdparts to anchor the first
partition within the MTD device and all the other partitions are at
offset +(sum of the sizes of all partitions listed before the current
one) relative to that first partition.

Removing the @0 feels fragile at best and it seems to depend on the
current behavior of the code.

> (Gaining more knowledge during reviewing ;-)
> 
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply

* [PATCH] arm64: vdso32: Use full path to Clang instead of relying on PATH
From: Nathan Chancellor @ 2018-05-22 21:59 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, in order to build the compat VDSO with Clang, this format
has to be used:

  PATH=${BIN_FOLDER}:${PATH} make CC=clang

Prior to the addition of this file, this format would also be
acceptable:

  make CC=${BIN_FOLDER}/clang

This is because the vdso32 Makefile uses cc-name instead of CC. After
this path, CC will still evaluate to clang for the first case as
expected but now the second case will use the specified Clang, rather
than the host's copy, which may not be compatible as shown below.

/usr/bin/as: unrecognized option '-mfloat-abi=soft'
clang-6.0: error: assembler command failed with exit code 1

Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

Hi everyone,

I noticed this issue when building the Pixel 2's kernel. Since this
is a supplement to https://patchwork.kernel.org/patch/10186671/, I
was instructed by Mark to push it here. I could not find a public git
tree for this patch, I am not sure if it has been applied or not so I
couldn't add a fixes tag. If there are any other issues, please let me
know!

Nathan

 arch/arm64/kernel/vdso32/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 6d44d972e89d..837e877a326b 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -5,7 +5,7 @@
 # A mix between the arm64 and arm vDSO Makefiles.
 
 ifeq ($(cc-name),clang)
-  CC_ARM32 := $(cc-name) $(CLANG_TARGET_ARM32) -no-integrated-as
+  CC_ARM32 := $(CC) $(CLANG_TARGET_ARM32) -no-integrated-as
 else
   CC_ARM32 := $(CROSS_COMPILE_ARM32)$(cc-name)
 endif
-- 
2.17.0

^ permalink raw reply related

* v4.17-rc1: regressions on N900, N950
From: Aaro Koskinen @ 2018-05-22 21:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522205824.GA24927@amd>

Hi,

On Tue, May 22, 2018 at 10:58:26PM +0200, Pavel Machek wrote:
> On Tue 2018-05-22 22:41:39, Aaro Koskinen wrote:
> > My device worked with v4.17-rc1 (haven't found time to test newer kernels),
> > but if you say the probe order is random then we must find some proper way
> > to express the dependency.
> 
> I started bisect, but.. that will probably not be useful.
> 
> If your device works ok in v4.17-rc1, it probably works in newer -rcs,
> too.

Actually, my statement may be bogus... Now I tried again with -rc1
(and also -rc6) and it fails... But v4.16 works.

> Thanks for the ordering hint, I'll try to figure out what is going on
> there.

My bisection pointed to 6fa7324ac5489ad43c4b6351355b869bc5458bef which
doesn't seem to make any sense...?! So maybe there really is something
random stuff going on? :-(

A.

^ permalink raw reply

* [PATCH 2/9] ARM: dts: lager: Drop MTD partitioning from DT
From: Marek Vasut @ 2018-05-22 21:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdV1TFH_QwTv4fsOGnUOCNftC9369_1AQmZDK+NBLDmw-Q@mail.gmail.com>

On 05/22/2018 04:32 PM, Geert Uytterhoeven wrote:
> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> Drop the MTD partitioning from DT, since it does not describe HW
>> and to give way to a more flexible kernel command line partition
>> passing.
>>
>> To retain the original partitioning, assure you have enabled
>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
>> following to your kernel command line:
>>
>>   mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)
> 
> I guess s/4096k/4m/ works, too?

Yes, but all the other boards use nnn k, so let's stick with k . I don't
want one value to stick out like a sore thumb.

>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply

* [PATCH 6/6] coresight: allow to build as modules
From: Mathieu Poirier @ 2018-05-22 21:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180518012024.22645-6-kim.phillips@arm.com>

On Thu, May 17, 2018 at 08:20:24PM -0500, Kim Phillips wrote:
> Allow to build coresight as modules.  This greatly enhances developer
> efficiency by allowing the development to take place exclusively on the
> target, and without needing to reboot in between changes.
> 
> - Kconfig bools become tristates, to allow =m
> 
> - use -objs to denote merge object directives in Makefile, adds a
>   coresight-core nomenclature for the base module.
> 
> - Export core functions so as to be able to be used by
>   non-core modules.
> 
> - add a coresight_exit() that unregisters the coresight bus, add
>   remove fns for most others.
> 
> - fix up modules with ID tables for autoloading on boot
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
> ---
>  drivers/hwtracing/coresight/Kconfig           | 48 +++++++++++++++----
>  drivers/hwtracing/coresight/Makefile          | 28 +++++++----
>  .../hwtracing/coresight/coresight-cpu-debug.c |  2 +
>  .../coresight/coresight-dynamic-replicator.c  | 26 ++++++++--
>  drivers/hwtracing/coresight/coresight-etb10.c | 27 +++++++++--
>  .../hwtracing/coresight/coresight-etm-perf.c  |  9 +++-
>  .../coresight/coresight-etm3x-sysfs.c         |  1 +
>  drivers/hwtracing/coresight/coresight-etm3x.c | 32 +++++++++++--
>  .../coresight/coresight-etm4x-sysfs.c         |  1 +
>  drivers/hwtracing/coresight/coresight-etm4x.c | 33 +++++++++++--
>  .../hwtracing/coresight/coresight-funnel.c    | 26 ++++++++--
>  drivers/hwtracing/coresight/coresight-priv.h  |  1 -
>  .../coresight/coresight-replicator.c          | 28 +++++++++--
>  drivers/hwtracing/coresight/coresight-stm.c   | 23 ++++++++-
>  drivers/hwtracing/coresight/coresight-tmc.c   | 18 ++++++-
>  drivers/hwtracing/coresight/coresight-tpiu.c  | 26 ++++++++--
>  drivers/hwtracing/coresight/coresight.c       | 14 ++++++
>  17 files changed, 299 insertions(+), 44 deletions(-)

For the next revision please split the work based on files.

> 
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index f9abdef5b0d9..4512885f7a3e 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -2,7 +2,7 @@
>  # Coresight configuration
>  #
>  menuconfig CORESIGHT
> -	bool "CoreSight Tracing Support"
> +	tristate "CoreSight Tracing Support"
>  	select ARM_AMBA
>  	select PERF_EVENTS
>  	help
> @@ -12,17 +12,23 @@ menuconfig CORESIGHT
>  	  specification and configure the right series of components when a
>  	  trace source gets enabled.
>  
> +	  To compile this code as a module, choose M here: the
> +	  module will be called coresight-core.
> +
>  if CORESIGHT
>  config CORESIGHT_LINKS_AND_SINKS
> -	bool "CoreSight Link and Sink drivers"
> +	tristate "CoreSight Link and Sink drivers"
>  	help
>  	  This enables support for CoreSight link and sink drivers that are
>  	  responsible for transporting and collecting the trace data
>  	  respectively.  Link and sinks are dynamically aggregated with a trace
>  	  entity at run time to form a complete trace path.
>  
> +	  To compile this code as modules, choose M here: the
> +	  modules will be called coresight-funnel and coresight-replicator.
> +
>  config CORESIGHT_LINK_AND_SINK_TMC
> -	bool "Coresight generic TMC driver"
> +	tristate "Coresight generic TMC driver"
>  	help
>  	  This enables support for the Trace Memory Controller driver.
>  	  Depending on its configuration the device can act as a link (embedded
> @@ -30,8 +36,11 @@ config CORESIGHT_LINK_AND_SINK_TMC
>  	  complies with the generic implementation of the component without
>  	  special enhancement or added features.
>  
> +	  To compile this code as a module, choose M here: the
> +	  module will be called coresight-tmc-core.
> +
>  config CORESIGHT_SINK_TPIU
> -	bool "Coresight generic TPIU driver"
> +	tristate "Coresight generic TPIU driver"
>  	help
>  	  This enables support for the Trace Port Interface Unit driver,
>  	  responsible for bridging the gap between the on-chip coresight
> @@ -40,15 +49,21 @@ config CORESIGHT_SINK_TPIU
>  	  connected to an external host for use case capturing more traces than
>  	  the on-board coresight memory can handle.
>  
> +	  To compile this code as a module, choose M here: the
> +	  module will be called coresight-tpiu.
> +
>  config CORESIGHT_SINK_ETBV10
> -	bool "Coresight ETBv1.0 driver"
> +	tristate "Coresight ETBv1.0 driver"
>  	help
>  	  This enables support for the Embedded Trace Buffer version 1.0 driver
>  	  that complies with the generic implementation of the component without
>  	  special enhancement or added features.
>  
> +	  To compile this code as a module, choose M here: the
> +	  module will be called coresight-etb10.
> +
>  config CORESIGHT_SOURCE_ETM3X
> -	bool "CoreSight Embedded Trace Macrocell 3.x driver"
> +	tristate "CoreSight Embedded Trace Macrocell 3.x driver"
>  	depends on !ARM64
>  	help
>  	  This driver provides support for processor ETM3.x and PTM1.x modules,
> @@ -56,8 +71,11 @@ config CORESIGHT_SOURCE_ETM3X
>  	  This is primarily useful for instruction level tracing.  Depending
>  	  the ETM version data tracing may also be available.
>  
> +	  To compile this code as a module, choose M here: the
> +	  module will be called coresight-etm3x-core.
> +
>  config CORESIGHT_SOURCE_ETM4X
> -	bool "CoreSight Embedded Trace Macrocell 4.x driver"
> +	tristate "CoreSight Embedded Trace Macrocell 4.x driver"
>  	depends on ARM64
>  	help
>  	  This driver provides support for the ETM4.x tracer module, tracing the
> @@ -65,15 +83,21 @@ config CORESIGHT_SOURCE_ETM4X
>  	  for instruction level tracing. Depending on the implemented version
>  	  data tracing may also be available.
>  
> +	  To compile this code as a module, choose M here: the
> +	  module will be called coresight-etm4x-core.
> +
>  config CORESIGHT_DYNAMIC_REPLICATOR
> -	bool "CoreSight Programmable Replicator driver"
> +	tristate "CoreSight Programmable Replicator driver"
>  	help
>  	  This enables support for dynamic CoreSight replicator link driver.
>  	  The programmable ATB replicator allows independent filtering of the
>  	  trace data based on the traceid.
>  
> +	  To compile this code as a module, choose M here: the
> +	  module will be called coresight-dynamic-replicator.
> +
>  config CORESIGHT_STM
> -	bool "CoreSight System Trace Macrocell driver"
> +	tristate "CoreSight System Trace Macrocell driver"
>  	depends on STM && ((ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64)
>  	help
>  	  This driver provides support for hardware assisted software
> @@ -81,6 +105,9 @@ config CORESIGHT_STM
>  	  logging useful software events or data coming from various entities
>  	  in the system, possibly running different OSs
>  
> +	  To compile this code as a module, choose M here: the
> +	  module will be called coresight-stm.
> +
>  config CORESIGHT_CPU_DEBUG
>  	tristate "CoreSight CPU Debug driver"
>  	depends on ARM || ARM64
> @@ -95,4 +122,7 @@ config CORESIGHT_CPU_DEBUG
>  	  properly, please refer Documentation/trace/coresight-cpu-debug.txt
>  	  for detailed description and the example for usage.
>  
> +	  To compile this code as a module, choose M here: the
> +	  module will be called coresight-cpu-debug.
> +
>  endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 61db9dd0d571..5990710289c2 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -2,19 +2,29 @@
>  #
>  # Makefile for CoreSight drivers.
>  #
> -obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o
> -obj-$(CONFIG_OF) += of_coresight.o
> -obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o \
> -					     coresight-tmc-etf.o \
> -					     coresight-tmc-etr.o
> +obj-$(CONFIG_CORESIGHT) += coresight-core.o
> +coresight-core-objs := coresight.o \
> +		       of_coresight.o
> +
> +obj-$(CONFIG_CORESIGHT) += coresight-etm-perf.o
> +
> +obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc-core.o
> +coresight-tmc-core-objs :=  coresight-tmc.o \
> +				 coresight-tmc-etf.o \
> +				 coresight-tmc-etr.o
>  obj-$(CONFIG_CORESIGHT_SINK_TPIU) += coresight-tpiu.o
>  obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o
>  obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \
>  					   coresight-replicator.o
> -obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o \
> -					coresight-etm3x-sysfs.o
> -obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
> -					coresight-etm4x-sysfs.o
> +
> +obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x-core.o
> +coresight-etm3x-core-objs := coresight-etm3x.o \
> +			     coresight-etm-cp14.o \
> +			     coresight-etm3x-sysfs.o
> +
> +obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x-core.o
> +coresight-etm4x-core-objs := coresight-etm4x.o coresight-etm4x-sysfs.o
> +
>  obj-$(CONFIG_CORESIGHT_DYNAMIC_REPLICATOR) += coresight-dynamic-replicator.o
>  obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
>  obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
> diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c
> index 45b2460f3166..1efe9626eb6c 100644
> --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c
> +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c
> @@ -671,6 +671,8 @@ static const struct amba_id debug_ids[] = {
>  	{ 0, 0 },
>  };
>  
> +MODULE_DEVICE_TABLE(amba, debug_ids);
> +
>  static struct amba_driver debug_driver = {
>  	.drv = {
>  		.name   = "coresight-cpu-debug",
> diff --git a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
> index fc742215ab05..bc42b8022556 100644
> --- a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
> @@ -37,7 +37,12 @@ struct replicator_state {
>  static int replicator_enable(struct coresight_device *csdev, int inport,
>  			      int outport)
>  {
> -	struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct replicator_state *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
> +
> +	if (!try_module_get(module))
> +		return -ENODEV;
>  
>  	CS_UNLOCK(drvdata->base);
>  
> @@ -63,7 +68,9 @@ static int replicator_enable(struct coresight_device *csdev, int inport,
>  static void replicator_disable(struct coresight_device *csdev, int inport,
>  				int outport)
>  {
> -	struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct replicator_state *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
>  
>  	CS_UNLOCK(drvdata->base);
>  
> @@ -75,6 +82,7 @@ static void replicator_disable(struct coresight_device *csdev, int inport,
>  
>  	CS_LOCK(drvdata->base);
>  
> +	module_put(module);
>  	dev_info(drvdata->dev, "REPLICATOR disabled\n");
>  }
>  
> @@ -159,6 +167,15 @@ static int replicator_probe(struct amba_device *adev, const struct amba_id *id)
>  	return PTR_ERR_OR_ZERO(drvdata->csdev);
>  }
>  
> +static int __exit replicator_remove(struct amba_device *adev)
> +{
> +	struct replicator_state *drvdata = dev_get_drvdata(&adev->dev);
> +
> +	coresight_unregister(drvdata->csdev);
> +
> +	return 0;
> +}
> +
>  #ifdef CONFIG_PM
>  static int replicator_runtime_suspend(struct device *dev)
>  {
> @@ -200,6 +217,8 @@ static const struct amba_id replicator_ids[] = {
>  	{ 0, 0 },
>  };
>  
> +MODULE_DEVICE_TABLE(amba, replicator_ids);
> +
>  static struct amba_driver replicator_driver = {
>  	.drv = {
>  		.name	= "coresight-dynamic-replicator",
> @@ -207,9 +226,10 @@ static struct amba_driver replicator_driver = {
>  		.suppress_bind_attrs = true,
>  	},
>  	.probe		= replicator_probe,
> +	.remove		= replicator_remove,
>  	.id_table	= replicator_ids,
>  };
> -builtin_amba_driver(replicator_driver);
> +module_amba_driver(replicator_driver);
>  
>  MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
>  MODULE_DESCRIPTION("ARM Coresight Dynamic Replicator Driver");
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index a3dac5a8b37c..8825a3e4e47a 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -135,7 +135,12 @@ static int etb_enable(struct coresight_device *csdev, u32 mode)
>  {
>  	u32 val;
>  	unsigned long flags;
> -	struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct etb_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
> +
> +	if (!try_module_get(module))
> +		return -ENODEV;
>  
>  	val = local_cmpxchg(&drvdata->mode,
>  			    CS_MODE_DISABLED, mode);
> @@ -256,7 +261,9 @@ static void etb_dump_hw(struct etb_drvdata *drvdata)
>  
>  static void etb_disable(struct coresight_device *csdev)
>  {
> -	struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct etb_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&drvdata->spinlock, flags);
> @@ -266,6 +273,7 @@ static void etb_disable(struct coresight_device *csdev)
>  
>  	local_set(&drvdata->mode, CS_MODE_DISABLED);
>  
> +	module_put(module);
>  	dev_info(drvdata->dev, "ETB disabled\n");
>  }
>  
> @@ -712,6 +720,16 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
>  	return ret;
>  }
>  
> +static int __exit etb_remove(struct amba_device *adev)
> +{
> +	struct etb_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> +	misc_deregister(&drvdata->miscdev);
> +	coresight_unregister(drvdata->csdev);
> +
> +	return 0;
> +}
> +
>  #ifdef CONFIG_PM
>  static int etb_runtime_suspend(struct device *dev)
>  {
> @@ -746,6 +764,8 @@ static const struct amba_id etb_ids[] = {
>  	{ 0, 0},
>  };
>  
> +MODULE_DEVICE_TABLE(amba, etb_ids);
> +
>  static struct amba_driver etb_driver = {
>  	.drv = {
>  		.name	= "coresight-etb10",
> @@ -755,9 +775,10 @@ static struct amba_driver etb_driver = {
>  
>  	},
>  	.probe		= etb_probe,
> +	.remove		= etb_remove,
>  	.id_table	= etb_ids,
>  };
> -builtin_amba_driver(etb_driver);
> +module_amba_driver(etb_driver);
>  
>  MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
>  MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index ad0ef8d27111..feb287083ba5 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -466,6 +466,7 @@ int etm_perf_symlink(struct coresight_device *csdev, bool link)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(etm_perf_symlink);
>  
>  static int __init etm_perf_init(void)
>  {
> @@ -493,7 +494,13 @@ static int __init etm_perf_init(void)
>  
>  	return ret;
>  }
> -device_initcall(etm_perf_init);
> +module_init(etm_perf_init);
> +
> +static void __exit etm_perf_exit(void)
> +{
> +	perf_pmu_unregister(&etm_pmu);
> +}
> +module_exit(etm_perf_exit);
>  
>  MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
>  MODULE_DESCRIPTION("Arm CoreSight tracer perf driver");
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> index 91a2a23143d8..84fa5e0fe07b 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> @@ -7,6 +7,7 @@
>  #include <linux/pid_namespace.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/sysfs.h>
> +#include <linux/coresight.h>

Why do we need this?

>  #include "coresight-etm.h"
>  #include "coresight-priv.h"
>  
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
> index 7ca73a15c735..a2357b26b3a2 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x.c
> @@ -514,7 +514,12 @@ static int etm_enable(struct coresight_device *csdev,
>  {
>  	int ret;
>  	u32 val;
> -	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct etm_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
> +
> +	if (!try_module_get(module))
> +		return -ENODEV;
>  
>  	val = local_cmpxchg(&drvdata->mode, CS_MODE_DISABLED, mode);
>  
> @@ -611,7 +616,9 @@ static void etm_disable(struct coresight_device *csdev,
>  			struct perf_event *event)
>  {
>  	u32 mode;
> -	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct etm_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
>  
>  	/*
>  	 * For as long as the tracer isn't disabled another entity can't
> @@ -636,6 +643,8 @@ static void etm_disable(struct coresight_device *csdev,
>  
>  	if (mode)
>  		local_set(&drvdata->mode, CS_MODE_DISABLED);
> +
> +	module_put(module);
>  }
>  
>  static const struct coresight_ops_source etm_source_ops = {
> @@ -864,6 +873,20 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
>  	return ret;
>  }
>  
> +static int __exit etm_remove(struct amba_device *adev)
> +{
> +	struct etm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> +	etm_perf_symlink(drvdata->csdev, false);
> +
> +	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
> +	cpuhp_remove_state_nocalls(hp_online);
> +
> +	coresight_unregister(drvdata->csdev);
> +
> +	return 0;
> +}
> +
>  #ifdef CONFIG_PM
>  static int etm_runtime_suspend(struct device *dev)
>  {
> @@ -924,6 +947,8 @@ static const struct amba_id etm_ids[] = {
>  	{ 0, 0},
>  };
>  
> +MODULE_DEVICE_TABLE(amba, etm_ids);
> +
>  static struct amba_driver etm_driver = {
>  	.drv = {
>  		.name	= "coresight-etm3x",
> @@ -932,9 +957,10 @@ static struct amba_driver etm_driver = {
>  		.suppress_bind_attrs = true,
>  	},
>  	.probe		= etm_probe,
> +	.remove		= etm_remove,
>  	.id_table	= etm_ids,
>  };
> -builtin_amba_driver(etm_driver);
> +module_amba_driver(etm_driver);
>  
>  MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
>  MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 577a38673444..ee0cbada45d6 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -2173,6 +2173,7 @@ const struct attribute_group *coresight_etmv4_groups[] = {
>  	&coresight_etmv4_trcidr_group,
>  	NULL,
>  };
> +EXPORT_SYMBOL_GPL(coresight_etmv4_groups);

>From where I stand this is not needed.

>  
>  MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
>  MODULE_DESCRIPTION("Arm CoreSight Program Flow Trace v4 sysfs driver");
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> index ba10f5302a55..a6ff152ab61d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> @@ -280,7 +280,12 @@ static int etm4_enable(struct coresight_device *csdev,
>  {
>  	int ret;
>  	u32 val;
> -	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct etmv4_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
> +
> +	if (!try_module_get(module))
> +		return -ENODEV;
>  
>  	val = local_cmpxchg(&drvdata->mode, CS_MODE_DISABLED, mode);
>  
> @@ -387,7 +392,9 @@ static void etm4_disable(struct coresight_device *csdev,
>  			 struct perf_event *event)
>  {
>  	u32 mode;
> -	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct etmv4_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
>  
>  	/*
>  	 * For as long as the tracer isn't disabled another entity can't
> @@ -409,6 +416,8 @@ static void etm4_disable(struct coresight_device *csdev,
>  
>  	if (mode)
>  		local_set(&drvdata->mode, CS_MODE_DISABLED);
> +
> +	module_put(module);
>  }
>  
>  static const struct coresight_ops_source etm4_source_ops = {
> @@ -1045,6 +1054,20 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
>  	return ret;
>  }
>  
> +static int __exit etm4_remove(struct amba_device *adev)
> +{
> +	struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> +	etm_perf_symlink(drvdata->csdev, false);
> +
> +	cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
> +	cpuhp_remove_state_nocalls(hp_online);
> +
> +	coresight_unregister(drvdata->csdev);
> +
> +	return 0;
> +}
> +
>  static const struct amba_id etm4_ids[] = {
>  	{       /* ETM 4.0 - Cortex-A53  */
>  		.id	= 0x000bb95d,
> @@ -1064,15 +1087,19 @@ static const struct amba_id etm4_ids[] = {
>  	{ 0, 0},
>  };
>  
> +MODULE_DEVICE_TABLE(amba, etm4_ids);
> +
>  static struct amba_driver etm4x_driver = {
>  	.drv = {
>  		.name   = "coresight-etm4x",
> +		.owner  = THIS_MODULE,
>  		.suppress_bind_attrs = true,
>  	},
>  	.probe		= etm4_probe,
> +	.remove		= etm4_remove,
>  	.id_table	= etm4_ids,
>  };
> -builtin_amba_driver(etm4x_driver);
> +module_amba_driver(etm4x_driver);
>  
>  MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
>  MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
> index 1e497a75b956..c355a66bcc51 100644
> --- a/drivers/hwtracing/coresight/coresight-funnel.c
> +++ b/drivers/hwtracing/coresight/coresight-funnel.c
> @@ -61,7 +61,12 @@ static void funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
>  static int funnel_enable(struct coresight_device *csdev, int inport,
>  			 int outport)
>  {
> -	struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct funnel_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
> +
> +	if (!try_module_get(module))
> +		return -ENODEV;
>  
>  	funnel_enable_hw(drvdata, inport);
>  
> @@ -85,10 +90,13 @@ static void funnel_disable_hw(struct funnel_drvdata *drvdata, int inport)
>  static void funnel_disable(struct coresight_device *csdev, int inport,
>  			   int outport)
>  {
> -	struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct funnel_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
>  
>  	funnel_disable_hw(drvdata, inport);
>  
> +	module_put(module);
>  	dev_info(drvdata->dev, "FUNNEL inport %d disabled\n", inport);
>  }
>  
> @@ -211,6 +219,15 @@ static int funnel_probe(struct amba_device *adev, const struct amba_id *id)
>  	return PTR_ERR_OR_ZERO(drvdata->csdev);
>  }
>  
> +static int __exit funnel_remove(struct amba_device *adev)
> +{
> +	struct funnel_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> +	coresight_unregister(drvdata->csdev);
> +
> +	return 0;
> +}
> +
>  #ifdef CONFIG_PM
>  static int funnel_runtime_suspend(struct device *dev)
>  {
> @@ -250,6 +267,8 @@ static const struct amba_id funnel_ids[] = {
>  	{ 0, 0},
>  };
>  
> +MODULE_DEVICE_TABLE(amba, funnel_ids);
> +
>  static struct amba_driver funnel_driver = {
>  	.drv = {
>  		.name	= "coresight-funnel",
> @@ -258,9 +277,10 @@ static struct amba_driver funnel_driver = {
>  		.suppress_bind_attrs = true,
>  	},
>  	.probe		= funnel_probe,
> +	.remove		= funnel_remove,
>  	.id_table	= funnel_ids,
>  };
> -builtin_amba_driver(funnel_driver);
> +module_amba_driver(funnel_driver);
>  
>  MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
>  MODULE_DESCRIPTION("ARM Coresight Funnel Driver");
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 45de8c15b687..896958c2dd44 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -65,7 +65,6 @@ static DEVICE_ATTR_RO(name)
>  static const u32 barrier_pkt[5] = {0x7fffffff, 0x7fffffff,
>  				   0x7fffffff, 0x7fffffff, 0x0};
>  
> -

No need for that.

>  enum etm_addr_type {
>  	ETM_ADDR_TYPE_NONE,
>  	ETM_ADDR_TYPE_SINGLE,
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index 9ef539893eaa..6f16dcd7e107 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -33,7 +33,12 @@ struct replicator_drvdata {
>  static int replicator_enable(struct coresight_device *csdev, int inport,
>  			     int outport)
>  {
> -	struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct replicator_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
> +
> +	if (!try_module_get(module))
> +		return -ENODEV;
>  
>  	dev_info(drvdata->dev, "REPLICATOR enabled\n");
>  	return 0;
> @@ -42,8 +47,11 @@ static int replicator_enable(struct coresight_device *csdev, int inport,
>  static void replicator_disable(struct coresight_device *csdev, int inport,
>  			       int outport)
>  {
> -	struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct replicator_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
>  
> +	module_put(module);
>  	dev_info(drvdata->dev, "REPLICATOR disabled\n");
>  }
>  
> @@ -112,6 +120,17 @@ static int replicator_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> +static int __exit replicator_remove(struct platform_device *pdev)
> +{
> +	struct replicator_drvdata *drvdata = dev_get_drvdata(&pdev->dev);
> +
> +	coresight_unregister(drvdata->csdev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return 0;
> +}
> +
>  #ifdef CONFIG_PM
>  static int replicator_runtime_suspend(struct device *dev)
>  {
> @@ -144,8 +163,11 @@ static const struct of_device_id replicator_match[] = {
>  	{}
>  };
>  
> +MODULE_DEVICE_TABLE(of, replicator_match);
> +
>  static struct platform_driver replicator_driver = {
>  	.probe          = replicator_probe,
> +	.remove         = replicator_remove,
>  	.driver         = {
>  		.name   = "coresight-replicator",
>  		.of_match_table = replicator_match,
> @@ -153,7 +175,7 @@ static struct platform_driver replicator_driver = {
>  		.suppress_bind_attrs = true,
>  	},
>  };
> -builtin_platform_driver(replicator_driver);
> +module_platform_driver(replicator_driver);
>  
>  MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
>  MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index 30eae52a8757..9997ba0dbd54 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -194,7 +194,12 @@ static int stm_enable(struct coresight_device *csdev,
>  		      struct perf_event *event, u32 mode)
>  {
>  	u32 val;
> -	struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct stm_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
> +
> +	if (!try_module_get(module))
> +		return -ENODEV;
>  
>  	if (mode != CS_MODE_SYSFS)
>  		return -EINVAL;


Function stm_disable() would likely benefit from a module_put().  

> @@ -882,6 +887,17 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id)
>  	return ret;
>  }
>  
> +static int __exit stm_remove(struct amba_device *adev)
> +{
> +	struct stm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> +	coresight_unregister(drvdata->csdev);
> +
> +	stm_unregister_device(&drvdata->stm);
> +
> +	return 0;
> +}
> +
>  #ifdef CONFIG_PM
>  static int stm_runtime_suspend(struct device *dev)
>  {
> @@ -922,6 +938,8 @@ static const struct amba_id stm_ids[] = {
>  	{ 0, 0},
>  };
>  
> +MODULE_DEVICE_TABLE(amba, stm_ids);
> +
>  static struct amba_driver stm_driver = {
>  	.drv = {
>  		.name   = "coresight-stm",
> @@ -930,10 +948,11 @@ static struct amba_driver stm_driver = {
>  		.suppress_bind_attrs = true,
>  	},
>  	.probe          = stm_probe,
> +	.remove         = stm_remove,
>  	.id_table	= stm_ids,
>  };
>  
> -builtin_amba_driver(stm_driver);
> +module_amba_driver(stm_driver);
>  
>  MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
>  MODULE_DESCRIPTION("Arm CoreSight System Trace Macrocell driver");
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index 176a5aeab20e..eb3cdb832f84 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -429,6 +429,19 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
>  	return ret;
>  }
>  
> +static int __exit tmc_remove(struct amba_device *adev)
> +{
> +	struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> +	/* free ETB/ETF or ETR memory */
> +	tmc_read_unprepare(drvdata);
> +
> +	misc_deregister(&drvdata->miscdev);
> +	coresight_unregister(drvdata->csdev);
> +
> +	return 0;
> +}
> +

Right now I can remove the module for a TMC link or sink when part of an active
session, something I pointed out during an earlier revision.

I also think we need to deal with driver removal cases when the TMC buffer
(ETR or ETF) is being read from sysFS.

>  static const struct amba_id tmc_ids[] = {
>  	{
>  		.id     = 0x000bb961,
> @@ -453,6 +466,8 @@ static const struct amba_id tmc_ids[] = {
>  	{ 0, 0},
>  };
>  
> +MODULE_DEVICE_TABLE(amba, tmc_ids);
> +
>  static struct amba_driver tmc_driver = {
>  	.drv = {
>  		.name   = "coresight-tmc",
> @@ -460,9 +475,10 @@ static struct amba_driver tmc_driver = {
>  		.suppress_bind_attrs = true,
>  	},
>  	.probe		= tmc_probe,
> +	.remove		= tmc_remove,
>  	.id_table	= tmc_ids,
>  };
> -builtin_amba_driver(tmc_driver);
> +module_amba_driver(tmc_driver);
>  
>  MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
>  MODULE_DESCRIPTION("Arm CoreSight Trace Memory Controller driver");
> diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
> index f3b154e150b3..9622f2a5a451 100644
> --- a/drivers/hwtracing/coresight/coresight-tpiu.c
> +++ b/drivers/hwtracing/coresight/coresight-tpiu.c
> @@ -69,7 +69,12 @@ static void tpiu_enable_hw(struct tpiu_drvdata *drvdata)
>  
>  static int tpiu_enable(struct coresight_device *csdev, u32 mode)
>  {
> -	struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct tpiu_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
> +
> +	if (!try_module_get(module))
> +		return -ENODEV;
>  
>  	tpiu_enable_hw(drvdata);
>  
> @@ -95,10 +100,13 @@ static void tpiu_disable_hw(struct tpiu_drvdata *drvdata)
>  
>  static void tpiu_disable(struct coresight_device *csdev)
>  {
> -	struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct device *parent_dev = csdev->dev.parent;
> +	struct tpiu_drvdata *drvdata = dev_get_drvdata(parent_dev);
> +	struct module *module = parent_dev->driver->owner;
>  
>  	tpiu_disable_hw(drvdata);
>  
> +	module_put(module);
>  	dev_info(drvdata->dev, "TPIU disabled\n");
>  }
>  
> @@ -164,6 +172,15 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id)
>  	return PTR_ERR_OR_ZERO(drvdata->csdev);
>  }
>  
> +static int __exit tpiu_remove(struct amba_device *adev)
> +{
> +	struct tpiu_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> +	coresight_unregister(drvdata->csdev);
> +
> +	return 0;
> +}
> +
>  #ifdef CONFIG_PM
>  static int tpiu_runtime_suspend(struct device *dev)
>  {
> @@ -207,6 +224,8 @@ static const struct amba_id tpiu_ids[] = {
>  	{ 0, 0},
>  };
>  
> +MODULE_DEVICE_TABLE(amba, tpiu_ids);
> +
>  static struct amba_driver tpiu_driver = {
>  	.drv = {
>  		.name	= "coresight-tpiu",
> @@ -215,9 +234,10 @@ static struct amba_driver tpiu_driver = {
>  		.suppress_bind_attrs = true,
>  	},
>  	.probe		= tpiu_probe,
> +	.remove		= tpiu_remove,
>  	.id_table	= tpiu_ids,
>  };
> -builtin_amba_driver(tpiu_driver);
> +module_amba_driver(tpiu_driver);
>  
>  MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
>  MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> index 406899f316e4..c00229b0db52 100644
> --- a/drivers/hwtracing/coresight/coresight.c
> +++ b/drivers/hwtracing/coresight/coresight.c
> @@ -302,6 +302,7 @@ void coresight_disable_path(struct list_head *path)
>  		}
>  	}
>  }
> +EXPORT_SYMBOL_GPL(coresight_disable_path);
>  
>  int coresight_enable_path(struct list_head *path, u32 mode)
>  {
> @@ -353,6 +354,7 @@ int coresight_enable_path(struct list_head *path, u32 mode)
>  	coresight_disable_path(path);
>  	goto out;
>  }
> +EXPORT_SYMBOL_GPL(coresight_enable_path);
>  
>  struct coresight_device *coresight_get_sink(struct list_head *path)
>  {
> @@ -368,6 +370,7 @@ struct coresight_device *coresight_get_sink(struct list_head *path)
>  
>  	return csdev;
>  }
> +EXPORT_SYMBOL_GPL(coresight_get_sink);
>  
>  static int coresight_enabled_sink(struct device *dev, void *data)
>  {
> @@ -392,6 +395,7 @@ static int coresight_enabled_sink(struct device *dev, void *data)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(coresight_enabled_sink);
>  
>  /**
>   * coresight_get_enabled_sink - returns the first enabled sink found on the bus
> @@ -414,6 +418,7 @@ struct coresight_device *coresight_get_enabled_sink(bool deactivate)
>  
>  	return dev ? to_coresight_device(dev) : NULL;
>  }
> +EXPORT_SYMBOL_GPL(coresight_get_enabled_sink);
>  
>  /**
>   * _coresight_build_path - recursively build a path from a @csdev to a sink.
> @@ -493,6 +498,7 @@ struct list_head *coresight_build_path(struct coresight_device *source,
>  
>  	return path;
>  }
> +EXPORT_SYMBOL_GPL(coresight_build_path);
>  
>  /**
>   * coresight_release_path - release a previously built path.
> @@ -517,6 +523,7 @@ void coresight_release_path(struct list_head *path)
>  	kfree(path);
>  	path = NULL;
>  }
> +EXPORT_SYMBOL_GPL(coresight_release_path);
>  
>  /** coresight_validate_source - make sure a source has the right credentials
>   *  @csdev:	the device structure for a source.
> @@ -933,6 +940,7 @@ int coresight_timeout(void __iomem *addr, u32 offset, int position, int value)
>  
>  	return -EAGAIN;
>  }
> +EXPORT_SYMBOL_GPL(coresight_timeout);
>  
>  struct bus_type coresight_bustype = {
>  	.name	= "coresight",
> @@ -944,6 +952,12 @@ static int __init coresight_init(void)
>  }
>  postcore_initcall(coresight_init);
>  
> +static void __exit coresight_exit(void)
> +{
> +	bus_unregister(&coresight_bustype);
> +}
> +module_exit(coresight_exit);
> +
>  struct coresight_device *coresight_register(struct coresight_desc *desc)
>  {
>  	int i;
> -- 
> 2.17.0
> 

^ permalink raw reply

* [linux-next PATCH 0/4] Enable network driver on K2G ICE and GP EVMs
From: Murali Karicheri @ 2018-05-22 21:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8e1aa492-ae7a-de2b-f5d2-8756b10ece79@oracle.com>

On 05/20/2018 11:17 PM, santosh.shilimkar at oracle.com wrote:
> On 5/11/18 12:29 PM, Murali Karicheri wrote:
>> Now that NetCP driver patches for K2G SoC is merged to linux-next master
>> this series add patches to enable network driver on K2G ICE and GP EVMs.
>>
>> Thanks
>>
>> Applied the patches on top of latest linux-next master, built kernel and
>> booted up on both EVMs. The logs are below
>>
>> K2G GP EVM: https://pastebin.ubuntu.com/p/ycZDnZXYPx/
>> K2G ICE EVM: https://pastebin.ubuntu.com/p/bdCpzgdrXr/
>>
>> Murali Karicheri (4):
>>    ARM: dts: k2g: add dt bindings to support network driver
>>    ARM: dts: keystone-k2g-evm: Enable netcp network driver
>>    ARM: dts: keystone-k2g-ice: Enable netcp network driver
>>    ARM: keystone: k2g: enable micrel and dp83867 phys
>>
>>   arch/arm/boot/dts/keystone-k2g-evm.dts    |  53 +++++++++++
>>   arch/arm/boot/dts/keystone-k2g-ice.dts    |  59 ++++++++++++
>>   arch/arm/boot/dts/keystone-k2g-netcp.dtsi | 147 ++++++++++++++++++++++++++++++
>>   arch/arm/boot/dts/keystone-k2g.dtsi       |  13 +++
>>   arch/arm/configs/keystone_defconfig       |   2 +
>>   5 files changed, 274 insertions(+)
>>   create mode 100644 arch/arm/boot/dts/keystone-k2g-netcp.dtsi
>>
> Looks good. Will add this to the queue for 4.19. Thanks !!
> 
> Regards,
> Santosh
> 
Thanks Santosh!

-- 
Murali Karicheri
Linux Kernel, Keystone

^ permalink raw reply

* v4.17-rc1: regressions on N900, N950
From: Pavel Machek @ 2018-05-22 20:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522194139.GB2299@darkstar.musicnaut.iki.fi>

On Tue 2018-05-22 22:41:39, Aaro Koskinen wrote:
> Hi,
> 
> On Tue, May 22, 2018 at 10:02:50AM +0200, Pali Roh?r wrote:
> > Hi! I remember that in time of migration from platform board code to
> > device tree structures there appeared some bug which caused that
> > sometimes display were not initialized. And somebody figured out that
> > display initialization is failing when some other SPI devices are
> > initialized before or after display... This behavior was observed only
> > on real N900 hardware, not in qemu.
> 
> Touchscreen needs to be initialized before display. This is documented
> in the DTS, see arch/arm/boot/dts/omap3-n900.dts:
> 
> 	* For some reason, touchscreen is necessary for screen to work at
> 	* all on real hw. It works well without it on emulator.
> 	*
> 	* Also... order in the device tree actually matters here.
> 
> > Real reason was never explained. In old platform board code there was
> > hardcoded order of SPI devices in which initialization happened. And in
> > device tree it is probably in (pseudo)-random order. Enabling/disabling
> > various config option can affect some timings and order in which kernel
> > starts probing and initializing devices...
> 
> The issue was also somewhat present with platform/board code, see e.g.
> commit e65f131a14726e5f1b880a528271a52428e5b3a5.
> 
> My device worked with v4.17-rc1 (haven't found time to test newer kernels),
> but if you say the probe order is random then we must find some proper way
> to express the dependency.

I started bisect, but.. that will probably not be useful.

If your device works ok in v4.17-rc1, it probably works in newer -rcs,
too.

Thanks for the ordering hint, I'll try to figure out what is going on
there.

									Pavel



# bad: [60cc43fc888428bb2f18f08997432d426a243338] Linux 4.17-rc1
# good: [0adb32858b0bddf4ada5f364a84ed60b196dbcda] Linux 4.16
git bisect start 'v4.17-rc1' 'v4.16'
# bad: [ac9053d2dcb9e8c3fa35ce458dfca8fddc141680] Merge tag 'usb-4.17-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
git bisect bad ac9053d2dcb9e8c3fa35ce458dfca8fddc141680
# bad: [bb2407a7219760926760f0448fddf00d625e5aec] Merge tag 'docs-4.17' of git://git.lwn.net/linux
git bisect bad bb2407a7219760926760f0448fddf00d625e5aec
# bad: [1c7095d2836baafd84e596dd34ba1a1293a4faa9] Merge airlied/drm-next into drm-misc-next
git bisect bad 1c7095d2836baafd84e596dd34ba1a1293a4faa9



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/1ed84105/attachment.sig>

^ permalink raw reply

* [PATCH] media: rcar-vin:  Drop unnecessary register properties from example vin port
From: Rob Herring @ 2018-05-22 20:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180509184558.14960-1-horms+renesas@verge.net.au>

On Wed, May 09, 2018 at 08:45:58PM +0200, Simon Horman wrote:
> The example vin port node does not have an address and thus does not
> need address-cells or address size-properties.
> 
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 3 ---
>  1 file changed, 3 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH 2/5] watchdog: sp805: add 'timeout-sec' DT property support
From: Guenter Roeck @ 2018-05-22 20:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527014840-21236-3-git-send-email-ray.jui@broadcom.com>

On Tue, May 22, 2018 at 11:47:17AM -0700, Ray Jui wrote:
> Add support for optional devicetree property 'timeout-sec'.
> 'timeout-sec' is used in the driver if specified in devicetree.
> Otherwise, fall back to driver default, i.e., 60 seconds
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/sp805_wdt.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
> index 03805bc..1484609 100644
> --- a/drivers/watchdog/sp805_wdt.c
> +++ b/drivers/watchdog/sp805_wdt.c
> @@ -230,7 +230,14 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
>  	spin_lock_init(&wdt->lock);
>  	watchdog_set_nowayout(&wdt->wdd, nowayout);
>  	watchdog_set_drvdata(&wdt->wdd, wdt);
> -	wdt_setload(&wdt->wdd, DEFAULT_TIMEOUT);
> +
> +	/*
> +	 * If 'timeout-sec' devicetree property is specified, use that.
> +	 * Otherwise, use DEFAULT_TIMEOUT
> +	 */
> +	wdt->wdd.timeout = DEFAULT_TIMEOUT;
> +	watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
> +	wdt_setload(&wdt->wdd, wdt->wdd.timeout);
>  
>  	ret = watchdog_register_device(&wdt->wdd);
>  	if (ret) {
> -- 
> 2.1.4
> 

^ permalink raw reply

* [PATCH 1/5] Documentation: DT: Add optional 'timeout-sec' property for sp805
From: Guenter Roeck @ 2018-05-22 20:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527014840-21236-2-git-send-email-ray.jui@broadcom.com>

On Tue, May 22, 2018 at 11:47:16AM -0700, Ray Jui wrote:
> Update the SP805 binding document to add optional 'timeout-sec'
> devicetree property
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  Documentation/devicetree/bindings/watchdog/sp805-wdt.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> index edc4f0e..f898a86 100644
> --- a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> @@ -19,6 +19,8 @@ Required properties:
>  
>  Optional properties:
>  - interrupts : Should specify WDT interrupt number.
> +- timeout-sec : Should specify default WDT timeout in seconds. If unset, the
> +                default timeout is 30 seconds
>  
>  Examples:
>  
> -- 
> 2.1.4
> 

^ permalink raw reply

* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Jon Hunter @ 2018-05-22 20:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAPDyKFr_7hbs-MC1jQn5uAgSnb1UGufXZdEmUqNTM63KemTe1g@mail.gmail.com>


On 22/05/18 15:47, Ulf Hansson wrote:
> [...]
> 
>>>
>>> +/**
>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>> + * @dev: Device to attach.
>>> + * @index: The index of the PM domain.
>>> + *
>>> + * Parse device's OF node to find a PM domain specifier at the provided @index.
>>> + * If such is found, allocates a new device and attaches it to retrieved
>>> + * pm_domain ops.
>>> + *
>>> + * Returns the allocated device if successfully attached PM domain, NULL when
>>> + * the device don't need a PM domain or have a single PM domain, else PTR_ERR()
>>> + * in case of failures. Note that if a power-domain exists for the device, but
>>> + * cannot be found or turned on, then return PTR_ERR(-EPROBE_DEFER) to ensure
>>> + * that the device is not probed and to re-try again later.
>>> + */
>>> +struct device *genpd_dev_pm_attach_by_id(struct device *dev,
>>> +                                      unsigned int index)
>>> +{
>>> +     struct device *genpd_dev;
>>> +     int num_domains;
>>> +     int ret;
>>> +
>>> +     if (!dev->of_node)
>>> +             return NULL;
>>> +
>>> +     /* Deal only with devices using multiple PM domains. */
>>> +     num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
>>> +                                              "#power-domain-cells");
>>> +     if (num_domains < 2 || index >= num_domains)
>>> +             return NULL;
>>> +
>>> +     /* Allocate and register device on the genpd bus. */
>>> +     genpd_dev = kzalloc(sizeof(*genpd_dev), GFP_KERNEL);
>>> +     if (!genpd_dev)
>>> +             return ERR_PTR(-ENOMEM);
>>> +
>>> +     dev_set_name(genpd_dev, "genpd:%u:%s", index, dev_name(dev));
>>> +     genpd_dev->bus = &genpd_bus_type;
>>> +     genpd_dev->release = genpd_release_dev;
>>> +
>>> +     ret = device_register(genpd_dev);
>>> +     if (ret) {
>>> +             kfree(genpd_dev);
>>> +             return ERR_PTR(ret);
>>> +     }
>>> +
>>> +     /* Try to attach the device to the PM domain at the specified index. */
>>> +     ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index);
>>> +     if (ret < 1) {
>>> +             device_unregister(genpd_dev);
>>> +             return ret ? ERR_PTR(ret) : NULL;
>>> +     }
>>> +
>>> +     pm_runtime_set_active(genpd_dev);
>>> +     pm_runtime_enable(genpd_dev);
>>> +
>>> +     return genpd_dev;
>>> +}
>>> +EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_by_id);
>>
>> Thanks for sending this. Believe it or not this has still been on my to-do list
>> and so we definitely need a solution for Tegra.
>>
>> Looking at the above it appears that additional power-domains exposed as devices
>> to the client device. So I assume that this means that the drivers for devices
>> with multiple power-domains will need to call RPM APIs for each of these
>> additional power-domains. Is that correct?
> 
> They can, but should not!
> 
> Instead, the driver shall use device_link_add() and device_link_del(),
> dynamically, depending on what PM domain that their original device
> needs for the current running use case.
> 
> In that way, they keep existing runtime PM deployment, operating on
> its original device.

OK, sounds good. Any reason why the linking cannot be handled by the 
above API? Is there a use-case where you would not want it linked?

Thanks
Jon

-- 
nvpublic

^ 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