| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 6 Feb 2014 13:31:25 +0530
From: Kishon Vijay Abraham I <kishon@...com>
To: Pratyush Anand <pratyush.anand@...com>
CC: "arnd@...db.de" <arnd@...db.de>,
Viresh Kumar <viresh.linux@...il.com>,
Tejun Heo <tj@...nel.org>,
spear-devel <spear-devel@...t.st.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-ide@...r.kernel.org" <linux-ide@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V4 5/8] SPEAr13xx: Fixup: Move SPEAr1340 SATA platform
code to phy driver
Hi,
On Thursday 06 February 2014 12:30 PM, Pratyush Anand wrote:
> On Thu, Feb 06, 2014 at 02:32:42PM +0800, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 06 February 2014 10:14 AM, Pratyush Anand wrote:
>>> ahci driver needs some platform specific functions which are called at
>>> init, exit, suspend and resume conditions. Till now these functions were
>>> present in a platform driver with a fixme notes.
>>>
>>> Similar functions modifying same set of registers will also be needed in
>>> case of PCIe phy init/exit.
>>>
>>> So move all these SATA platform code to phy-miphy40lp driver.
>>>
>>> Signed-off-by: Pratyush Anand <pratyush.anand@...com>
>>> Tested-by: Mohit Kumar <mohit.kumar@...com>
>>> Cc: Viresh Kumar <viresh.linux@...il.com>
>>> Cc: Tejun Heo <tj@...nel.org>
>>> Cc: Arnd Bergmann <arnd@...db.de>
>>> Cc: Kishon Vijay Abraham I <kishon@...com>
>>> Cc: spear-devel@...t.st.com
>>> Cc: linux-arm-kernel@...ts.infradead.org
>>> Cc: devicetree@...r.kernel.org
>>> Cc: linux-ide@...r.kernel.org
>>> Cc: linux-kernel@...r.kernel.org
>>> ---
>>> .../devicetree/bindings/arm/spear-misc.txt | 4 +
>>> arch/arm/boot/dts/spear1310-evb.dts | 4 +
>>> arch/arm/boot/dts/spear1310.dtsi | 39 +++-
>>> arch/arm/boot/dts/spear1340-evb.dts | 4 +
>>> arch/arm/boot/dts/spear1340.dtsi | 13 +-
>>> arch/arm/boot/dts/spear13xx.dtsi | 5 +
>>> arch/arm/mach-spear/Kconfig | 2 +
>>> arch/arm/mach-spear/spear1340.c | 127 +------------
>>> drivers/phy/phy-miphy40lp.c | 204 ++++++++++++++++++++-
>>
>> It would be better if you can split this patch. Keep arch/ in separate patch
>> and drivers/ in separate patch.
>
> Code is actually moving from arch to driver. Therefore I kept it in
> same patch.
>
>>> 9 files changed, 266 insertions(+), 136 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/arm/spear-misc.txt
>>>
>> .
>> .
>> <snip>
>> .
>> .
>>> static const char * const spear1340_dt_board_compat[] = {
>>> diff --git a/drivers/phy/phy-miphy40lp.c b/drivers/phy/phy-miphy40lp.c
>>> index d478c14..cc7f45d 100644
>>> --- a/drivers/phy/phy-miphy40lp.c
>>> +++ b/drivers/phy/phy-miphy40lp.c
>>> @@ -8,6 +8,7 @@
>>> * it under the terms of the GNU General Public License version 2 as
>>> * published by the Free Software Foundation.
>>> *
>>> + * 04/02/2014: Adding support of SATA mode for SPEAr1340.
>>> */
>>>
>>> #include <linux/delay.h>
>>> @@ -19,6 +20,60 @@
>>> #include <linux/phy/phy.h>
>>> #include <linux/regmap.h>
>>>
>>> +/* SPEAr1340 Registers */
>>> +/* Power Management Registers */
>>> +#define SPEAR1340_PCM_CFG 0x100
>>> + #define SPEAR1340_PCM_CFG_SATA_POWER_EN 0x800
>>> +#define SPEAR1340_PCM_WKUP_CFG 0x104
>>> +#define SPEAR1340_SWITCH_CTR 0x108
>>> +
>>> +#define SPEAR1340_PERIP1_SW_RST 0x318
>>> + #define SPEAR1340_PERIP1_SW_RST_SATA 0x1000
>>> +#define SPEAR1340_PERIP2_SW_RST 0x31C
>>> +#define SPEAR1340_PERIP3_SW_RST 0x320
>>> +
>>> +/* PCIE - SATA configuration registers */
>>> +#define SPEAR1340_PCIE_SATA_CFG 0x424
>>> + /* PCIE CFG MASks */
>>> + #define SPEAR1340_PCIE_CFG_DEVICE_PRESENT (1 << 11)
>>
>> use BIT() wherever possible.
>
> OK.
>
>>> + #define SPEAR1340_PCIE_CFG_POWERUP_RESET (1 << 10)
>>> + #define SPEAR1340_PCIE_CFG_CORE_CLK_EN (1 << 9)
>>> + #define SPEAR1340_PCIE_CFG_AUX_CLK_EN (1 << 8)
>>> + #define SPEAR1340_SATA_CFG_TX_CLK_EN (1 << 4)
>>> + #define SPEAR1340_SATA_CFG_RX_CLK_EN (1 << 3)
>>> + #define SPEAR1340_SATA_CFG_POWERUP_RESET (1 << 2)
>>> + #define SPEAR1340_SATA_CFG_PM_CLK_EN (1 << 1)
>>> + #define SPEAR1340_PCIE_SATA_SEL_PCIE (0)
>>> + #define SPEAR1340_PCIE_SATA_SEL_SATA (1)
>>> + #define SPEAR1340_PCIE_SATA_CFG_MASK 0xF1F
>>> + #define SPEAR1340_PCIE_CFG_VAL (SPEAR1340_PCIE_SATA_SEL_PCIE | \
>>> + SPEAR1340_PCIE_CFG_AUX_CLK_EN | \
>>> + SPEAR1340_PCIE_CFG_CORE_CLK_EN | \
>>> + SPEAR1340_PCIE_CFG_POWERUP_RESET | \
>>> + SPEAR1340_PCIE_CFG_DEVICE_PRESENT)
>>> + #define SPEAR1340_SATA_CFG_VAL (SPEAR1340_PCIE_SATA_SEL_SATA | \
>>> + SPEAR1340_SATA_CFG_PM_CLK_EN | \
>>> + SPEAR1340_SATA_CFG_POWERUP_RESET | \
>>> + SPEAR1340_SATA_CFG_RX_CLK_EN | \
>>> + SPEAR1340_SATA_CFG_TX_CLK_EN)
>>> +
>>> +#define SPEAR1340_PCIE_MIPHY_CFG 0x428
>>> + #define SPEAR1340_MIPHY_OSC_BYPASS_EXT (1 << 31)
>>> + #define SPEAR1340_MIPHY_CLK_REF_DIV2 (1 << 27)
>>> + #define SPEAR1340_MIPHY_CLK_REF_DIV4 (2 << 27)
>>> + #define SPEAR1340_MIPHY_CLK_REF_DIV8 (3 << 27)
>>> + #define SPEAR1340_MIPHY_PLL_RATIO_TOP(x) (x << 0)
>>> + #define SPEAR1340_PCIE_MIPHY_CFG_MASK 0xF80000FF
>>> + #define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA \
>>> + (SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
>>> + SPEAR1340_MIPHY_CLK_REF_DIV2 | \
>>> + SPEAR1340_MIPHY_PLL_RATIO_TOP(60))
>>> + #define SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK \
>>> + (SPEAR1340_MIPHY_PLL_RATIO_TOP(120))
>>> + #define SPEAR1340_PCIE_SATA_MIPHY_CFG_PCIE \
>>> + (SPEAR1340_MIPHY_OSC_BYPASS_EXT | \
>>> + SPEAR1340_MIPHY_PLL_RATIO_TOP(25))
>>> +
>>> enum phy_mode {
>>> SATA,
>>> PCIE,
>>> @@ -38,28 +93,145 @@ struct st_miphy40lp_priv {
>>> u32 id;
>>> };
>>>
>>> +static int spear1340_sata_miphy_init(struct st_miphy40lp_priv *phypriv)
>>
>> The function name format here differs from what you have already added. It will
>> be good to have consistent name in the file.
>
> You mean to pass "struct phy *phy" in all the internal functions too?
No. I meant let all the function names begin with miphy40lp_.
>
>>> +{
>>> + regmap_update_bits(phypriv->misc, SPEAR1340_PCIE_SATA_CFG,
>>> + SPEAR1340_PCIE_SATA_CFG_MASK, SPEAR1340_SATA_CFG_VAL);
>>> + regmap_update_bits(phypriv->misc, SPEAR1340_PCIE_MIPHY_CFG,
>>> + SPEAR1340_PCIE_MIPHY_CFG_MASK,
>>> + SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK);
>>> + /* Switch on sata power domain */
>>> + regmap_update_bits(phypriv->misc, SPEAR1340_PCM_CFG,
>>> + SPEAR1340_PCM_CFG_SATA_POWER_EN,
>>> + SPEAR1340_PCM_CFG_SATA_POWER_EN);
>>> + msleep(20);
>>> + /* Disable PCIE SATA Controller reset */
>>> + regmap_update_bits(phypriv->misc, SPEAR1340_PERIP1_SW_RST,
>>> + SPEAR1340_PERIP1_SW_RST_SATA, 0);
>>> + msleep(20);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int spear1340_sata_miphy_exit(struct st_miphy40lp_priv *phypriv)
>>> +{
>>> + regmap_update_bits(phypriv->misc, SPEAR1340_PCIE_SATA_CFG,
>>> + SPEAR1340_PCIE_SATA_CFG_MASK, 0);
>>> + regmap_update_bits(phypriv->misc, SPEAR1340_PCIE_MIPHY_CFG,
>>> + SPEAR1340_PCIE_MIPHY_CFG_MASK, 0);
>>> +
>>> + /* Enable PCIE SATA Controller reset */
>>> + regmap_update_bits(phypriv->misc, SPEAR1340_PERIP1_SW_RST,
>>> + SPEAR1340_PERIP1_SW_RST_SATA,
>>> + SPEAR1340_PERIP1_SW_RST_SATA);
>>> + msleep(20);
>>> + /* Switch off sata power domain */
>>> + regmap_update_bits(phypriv->misc, SPEAR1340_PCM_CFG,
>>> + SPEAR1340_PCM_CFG_SATA_POWER_EN, 0);
>>> + msleep(20);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sata_miphy_init(struct st_miphy40lp_priv *phypriv)
>>> +{
>>> + if (of_device_is_compatible(phypriv->np, "st,spear1340-miphy"))
>>
>> This compatible value is a bit confusing since it doesn't have 'sata' in it.
>> spear1340 can have usb phy or pcie phy too no? How do we differentiate it then?
>
> same spear1340 miphy is used for sata as well as for pcie. sata or
> pcie mode is selected using mode args passed in phys.
Alright. Got it while reading the next patch ;-)
Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists