* Re: [PATCH v6 4/5] phy: rockchip-typec: support variable phy config value
From: Enric Balletbo Serra @ 2018-05-21 14:13 UTC (permalink / raw)
To: Lin Huang
Cc: devicetree@vger.kernel.org, David Airlie, linux-kernel,
Brian Norris, Doug Anderson, Kishon Vijay Abraham I,
open list:ARM/Rockchip SoC..., Rob Herring, dri-devel,
Chris Zhong, daniel.vetter, Linux ARM
In-Reply-To: <1526895424-22894-4-git-send-email-hl@rock-chips.com>
Hi Lin,
2018-05-21 11:37 GMT+02:00 Lin Huang <hl@rock-chips.com>:
> the phy config values used to fix in dp firmware, but some boards
> need change these values to do training and get the better eye diagram
> result. So support that in phy driver.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> Changes in v2:
> - update patch following Enric suggest
> Changes in v3:
> - delete need_software_training variable
> - add default phy config value, if dts do not define phy config value, use these value
> Changes in v4:
> - rename variable config to tcphy_default_config
> Changes in v5:
> - None
> Changes in v6:
> - split the header file to new patch
>
> drivers/phy/rockchip/phy-rockchip-typec.c | 261 ++++++++++++++++++++++++------
> 1 file changed, 208 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index 795055f..4c4b925 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
> @@ -324,21 +324,29 @@
> * clock 0: PLL 0 div 1
> * clock 1: PLL 1 div 2
> */
> -#define CLK_PLL_CONFIG 0X30
> +#define CLK_PLL1_DIV1 0x20
> +#define CLK_PLL1_DIV2 0x30
> #define CLK_PLL_MASK 0x33
>
> #define CMN_READY BIT(0)
>
> +#define DP_PLL_CLOCK_ENABLE_ACK BIT(3)
> #define DP_PLL_CLOCK_ENABLE BIT(2)
> +#define DP_PLL_ENABLE_ACK BIT(1)
> #define DP_PLL_ENABLE BIT(0)
> #define DP_PLL_DATA_RATE_RBR ((2 << 12) | (4 << 8))
> #define DP_PLL_DATA_RATE_HBR ((2 << 12) | (4 << 8))
> #define DP_PLL_DATA_RATE_HBR2 ((1 << 12) | (2 << 8))
> +#define DP_PLL_DATA_RATE_MASK 0xff00
>
> -#define DP_MODE_A0 BIT(4)
> -#define DP_MODE_A2 BIT(6)
> -#define DP_MODE_ENTER_A0 0xc101
> -#define DP_MODE_ENTER_A2 0xc104
> +#define DP_MODE_MASK 0xf
> +#define DP_MODE_ENTER_A0 BIT(0)
> +#define DP_MODE_ENTER_A2 BIT(2)
> +#define DP_MODE_ENTER_A3 BIT(3)
> +#define DP_MODE_A0_ACK BIT(4)
> +#define DP_MODE_A2_ACK BIT(6)
> +#define DP_MODE_A3_ACK BIT(7)
> +#define DP_LINK_RESET_DEASSERTED BIT(8)
>
> #define PHY_MODE_SET_TIMEOUT 100000
>
> @@ -350,6 +358,8 @@
> #define MODE_DFP_USB BIT(1)
> #define MODE_DFP_DP BIT(2)
>
> +#define DP_DEFAULT_RATE 162000
> +
> struct phy_reg {
> u16 value;
> u32 addr;
> @@ -372,15 +382,15 @@ struct phy_reg usb3_pll_cfg[] = {
> { 0x8, CMN_DIAG_PLL0_LF_PROG },
> };
>
> -struct phy_reg dp_pll_cfg[] = {
> +struct phy_reg dp_pll_rbr_cfg[] = {
> { 0xf0, CMN_PLL1_VCOCAL_INIT },
> { 0x18, CMN_PLL1_VCOCAL_ITER },
> { 0x30b9, CMN_PLL1_VCOCAL_START },
> - { 0x21c, CMN_PLL1_INTDIV },
> + { 0x87, CMN_PLL1_INTDIV },
> { 0, CMN_PLL1_FRACDIV },
> - { 0x5, CMN_PLL1_HIGH_THR },
> - { 0x35, CMN_PLL1_SS_CTRL1 },
> - { 0x7f1e, CMN_PLL1_SS_CTRL2 },
> + { 0x22, CMN_PLL1_HIGH_THR },
> + { 0x8000, CMN_PLL1_SS_CTRL1 },
> + { 0, CMN_PLL1_SS_CTRL2 },
> { 0x20, CMN_PLL1_DSM_DIAG },
> { 0, CMN_PLLSM1_USER_DEF_CTRL },
> { 0, CMN_DIAG_PLL1_OVRD },
> @@ -391,9 +401,52 @@ struct phy_reg dp_pll_cfg[] = {
> { 0x8, CMN_DIAG_PLL1_LF_PROG },
> { 0x100, CMN_DIAG_PLL1_PTATIS_TUNE1 },
> { 0x7, CMN_DIAG_PLL1_PTATIS_TUNE2 },
> - { 0x4, CMN_DIAG_PLL1_INCLK_CTRL },
> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
> };
>
> +struct phy_reg dp_pll_hbr_cfg[] = {
> + { 0xf0, CMN_PLL1_VCOCAL_INIT },
> + { 0x18, CMN_PLL1_VCOCAL_ITER },
> + { 0x30b4, CMN_PLL1_VCOCAL_START },
> + { 0xe1, CMN_PLL1_INTDIV },
> + { 0, CMN_PLL1_FRACDIV },
> + { 0x5, CMN_PLL1_HIGH_THR },
> + { 0x8000, CMN_PLL1_SS_CTRL1 },
> + { 0, CMN_PLL1_SS_CTRL2 },
> + { 0x20, CMN_PLL1_DSM_DIAG },
> + { 0x1000, CMN_PLLSM1_USER_DEF_CTRL },
> + { 0, CMN_DIAG_PLL1_OVRD },
> + { 0, CMN_DIAG_PLL1_FBH_OVRD },
> + { 0, CMN_DIAG_PLL1_FBL_OVRD },
> + { 0x7, CMN_DIAG_PLL1_V2I_TUNE },
> + { 0x45, CMN_DIAG_PLL1_CP_TUNE },
> + { 0x8, CMN_DIAG_PLL1_LF_PROG },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE1 },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE2 },
> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
> +};
> +
> +struct phy_reg dp_pll_hbr2_cfg[] = {
> + { 0xf0, CMN_PLL1_VCOCAL_INIT },
> + { 0x18, CMN_PLL1_VCOCAL_ITER },
> + { 0x30b4, CMN_PLL1_VCOCAL_START },
> + { 0xe1, CMN_PLL1_INTDIV },
> + { 0, CMN_PLL1_FRACDIV },
> + { 0x5, CMN_PLL1_HIGH_THR },
> + { 0x8000, CMN_PLL1_SS_CTRL1 },
> + { 0, CMN_PLL1_SS_CTRL2 },
> + { 0x20, CMN_PLL1_DSM_DIAG },
> + { 0x1000, CMN_PLLSM1_USER_DEF_CTRL },
> + { 0, CMN_DIAG_PLL1_OVRD },
> + { 0, CMN_DIAG_PLL1_FBH_OVRD },
> + { 0, CMN_DIAG_PLL1_FBL_OVRD },
> + { 0x7, CMN_DIAG_PLL1_V2I_TUNE },
> + { 0x45, CMN_DIAG_PLL1_CP_TUNE },
> + { 0x8, CMN_DIAG_PLL1_LF_PROG },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE1 },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE2 },
> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
> +};
> static const struct rockchip_usb3phy_port_cfg rk3399_usb3phy_port_cfgs[] = {
> {
> .reg = 0xff7c0000,
> @@ -418,6 +471,24 @@ static const struct rockchip_usb3phy_port_cfg rk3399_usb3phy_port_cfgs[] = {
> { /* sentinel */ }
> };
>
> +/* default phy config */
> +static const struct phy_config tcphy_default_config[3][4] = {
> + {{ .swing = 0x2a, .pe = 0x00 },
> + { .swing = 0x1f, .pe = 0x15 },
> + { .swing = 0x14, .pe = 0x22 },
> + { .swing = 0x02, .pe = 0x2b } },
> +
> + {{ .swing = 0x21, .pe = 0x00 },
> + { .swing = 0x12, .pe = 0x15 },
> + { .swing = 0x02, .pe = 0x22 },
> + { .swing = 0, .pe = 0 } },
> +
> + {{ .swing = 0x15, .pe = 0x00 },
> + { .swing = 0x00, .pe = 0x15 },
> + { .swing = 0, .pe = 0 },
> + { .swing = 0, .pe = 0 } },
> +};
> +
> static void tcphy_cfg_24m(struct rockchip_typec_phy *tcphy)
> {
> u32 i, rdata;
> @@ -439,7 +510,7 @@ static void tcphy_cfg_24m(struct rockchip_typec_phy *tcphy)
>
> rdata = readl(tcphy->base + CMN_DIAG_HSCLK_SEL);
> rdata &= ~CLK_PLL_MASK;
> - rdata |= CLK_PLL_CONFIG;
> + rdata |= CLK_PLL1_DIV2;
> writel(rdata, tcphy->base + CMN_DIAG_HSCLK_SEL);
> }
>
> @@ -453,17 +524,44 @@ static void tcphy_cfg_usb3_pll(struct rockchip_typec_phy *tcphy)
> tcphy->base + usb3_pll_cfg[i].addr);
> }
>
> -static void tcphy_cfg_dp_pll(struct rockchip_typec_phy *tcphy)
> +static void tcphy_cfg_dp_pll(struct rockchip_typec_phy *tcphy, int link_rate)
> {
> - u32 i;
> + struct phy_reg *phy_cfg;
> + u32 clk_ctrl;
> + u32 i, cfg_size, hsclk_sel;
> +
> + hsclk_sel = readl(tcphy->base + CMN_DIAG_HSCLK_SEL);
> + hsclk_sel &= ~CLK_PLL_MASK;
> +
> + switch (link_rate) {
> + case 162000:
> + clk_ctrl = DP_PLL_DATA_RATE_RBR;
> + hsclk_sel |= CLK_PLL1_DIV2;
> + phy_cfg = dp_pll_rbr_cfg;
> + cfg_size = ARRAY_SIZE(dp_pll_rbr_cfg);
> + break;
> + case 270000:
> + clk_ctrl = DP_PLL_DATA_RATE_HBR;
> + hsclk_sel |= CLK_PLL1_DIV2;
> + phy_cfg = dp_pll_hbr_cfg;
> + cfg_size = ARRAY_SIZE(dp_pll_hbr_cfg);
> + break;
> + case 540000:
> + clk_ctrl = DP_PLL_DATA_RATE_HBR2;
> + hsclk_sel |= CLK_PLL1_DIV1;
> + phy_cfg = dp_pll_hbr2_cfg;
> + cfg_size = ARRAY_SIZE(dp_pll_hbr2_cfg);
> + break;
If someone calls this function with a link_rate different to the ones
in the switch statement, some variables will be uninitialized and the
kernel will crash. I'd add a default case or return an error if a
different link_rate is passed to the function.
> + }
>
> - /* set the default mode to RBR */
> - writel(DP_PLL_CLOCK_ENABLE | DP_PLL_ENABLE | DP_PLL_DATA_RATE_RBR,
> - tcphy->base + DP_CLK_CTL);
> + clk_ctrl |= DP_PLL_CLOCK_ENABLE | DP_PLL_ENABLE;
> + writel(clk_ctrl, tcphy->base + DP_CLK_CTL);
> +
> + writel(hsclk_sel, tcphy->base + CMN_DIAG_HSCLK_SEL);
>
> /* load the configuration of PLL1 */
> - for (i = 0; i < ARRAY_SIZE(dp_pll_cfg); i++)
> - writel(dp_pll_cfg[i].value, tcphy->base + dp_pll_cfg[i].addr);
> + for (i = 0; i < cfg_size; i++)
> + writel(phy_cfg[i].value, tcphy->base + phy_cfg[i].addr);
> }
>
> static void tcphy_tx_usb3_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> @@ -490,9 +588,10 @@ static void tcphy_rx_usb3_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> writel(0xfb, tcphy->base + XCVR_DIAG_BIDI_CTRL(lane));
> }
>
> -static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> +static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, int link_rate,
> + u8 swing, u8 pre_emp, u32 lane)
> {
> - u16 rdata;
> + u16 val;
>
Just use rdata in your added code, you don't need to rename the
variable. That will result in a smaller diff.
> writel(0xbefc, tcphy->base + XCVR_PSM_RCTRL(lane));
> writel(0x6799, tcphy->base + TX_PSC_A0(lane));
> @@ -500,25 +599,31 @@ static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> writel(0x98, tcphy->base + TX_PSC_A2(lane));
> writel(0x98, tcphy->base + TX_PSC_A3(lane));
>
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_000(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_001(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_010(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_011(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_100(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_101(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_110(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_111(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_10(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_01(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_00(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_11(lane));
> -
> - writel(0x128, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
> - writel(0x400, tcphy->base + TX_DIAG_TX_DRV(lane));
> -
> - rdata = readl(tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> - rdata = (rdata & 0x8fff) | 0x6000;
> - writel(rdata, tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> + writel(tcphy->config[swing][pre_emp].swing,
> + tcphy->base + TX_TXCC_MGNFS_MULT_000(lane));
> + writel(tcphy->config[swing][pre_emp].pe,
> + tcphy->base + TX_TXCC_CPOST_MULT_00(lane));
> +
> + if (swing == 2 && pre_emp == 0 && link_rate != 540000) {
> + writel(0x700, tcphy->base + TX_DIAG_TX_DRV(lane));
> + writel(0x13c, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
> + } else {
> + writel(0x128, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
> + writel(0x0400, tcphy->base + TX_DIAG_TX_DRV(lane));
> + }
> +
> + val = readl(tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> + val = val & 0x8fff;
> + switch (link_rate) {
> + case 162000:
> + case 270000:
> + val |= (6 << 12);
> + break;
> + case 540000:
> + val |= (4 << 12);
> + break;
If link_rate is another value you will write (val & 0x8fff), is ok?
Before this patch, we were writing (rdata & 0x8fff) | 0x6000 by
default, maybe we should default to this value?
> + }
> + writel(val, tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> }
>
> static inline int property_enable(struct rockchip_typec_phy *tcphy,
> @@ -709,30 +814,33 @@ static int tcphy_phy_init(struct rockchip_typec_phy *tcphy, u8 mode)
> tcphy_cfg_24m(tcphy);
>
> if (mode == MODE_DFP_DP) {
> - tcphy_cfg_dp_pll(tcphy);
> + tcphy_cfg_dp_pll(tcphy, DP_DEFAULT_RATE);
> for (i = 0; i < 4; i++)
> - tcphy_dp_cfg_lane(tcphy, i);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, i);
>
> writel(PIN_ASSIGN_C_E, tcphy->base + PMA_LANE_CFG);
> } else {
> tcphy_cfg_usb3_pll(tcphy);
> - tcphy_cfg_dp_pll(tcphy);
> + tcphy_cfg_dp_pll(tcphy, DP_DEFAULT_RATE);
> if (tcphy->flip) {
> tcphy_tx_usb3_cfg_lane(tcphy, 3);
> tcphy_rx_usb3_cfg_lane(tcphy, 2);
> - tcphy_dp_cfg_lane(tcphy, 0);
> - tcphy_dp_cfg_lane(tcphy, 1);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 0);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 1);
> } else {
> tcphy_tx_usb3_cfg_lane(tcphy, 0);
> tcphy_rx_usb3_cfg_lane(tcphy, 1);
> - tcphy_dp_cfg_lane(tcphy, 2);
> - tcphy_dp_cfg_lane(tcphy, 3);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 2);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 3);
> }
>
> writel(PIN_ASSIGN_D_F, tcphy->base + PMA_LANE_CFG);
> }
>
> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
> + val = readl(tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A2 | DP_LINK_RESET_DEASSERTED;
> + writel(val, tcphy->base + DP_MODE_CTL);
>
> reset_control_deassert(tcphy->uphy_rst);
>
> @@ -945,7 +1053,7 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
> property_enable(tcphy, &cfg->uphy_dp_sel, 1);
>
> ret = readx_poll_timeout(readl, tcphy->base + DP_MODE_CTL,
> - val, val & DP_MODE_A2, 1000,
> + val, val & DP_MODE_A2_ACK, 1000,
> PHY_MODE_SET_TIMEOUT);
> if (ret < 0) {
> dev_err(tcphy->dev, "failed to wait TCPHY enter A2\n");
> @@ -954,13 +1062,19 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
>
> tcphy_dp_aux_calibration(tcphy);
>
> - writel(DP_MODE_ENTER_A0, tcphy->base + DP_MODE_CTL);
> + /* enter A0 mode */
> + val = readl(tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A0;
> + writel(val, tcphy->base + DP_MODE_CTL);
>
> ret = readx_poll_timeout(readl, tcphy->base + DP_MODE_CTL,
> - val, val & DP_MODE_A0, 1000,
> + val, val & DP_MODE_A0_ACK, 1000,
> PHY_MODE_SET_TIMEOUT);
> if (ret < 0) {
> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A2;
> + writel(val, tcphy->base + DP_MODE_CTL);
> dev_err(tcphy->dev, "failed to wait TCPHY enter A0\n");
> goto power_on_finish;
> }
> @@ -978,6 +1092,7 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
> static int rockchip_dp_phy_power_off(struct phy *phy)
> {
> struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> + u32 val;
>
> mutex_lock(&tcphy->lock);
>
> @@ -986,7 +1101,10 @@ static int rockchip_dp_phy_power_off(struct phy *phy)
>
> tcphy->mode &= ~MODE_DFP_DP;
>
> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
> + val = readl(tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A2;
> + writel(val, tcphy->base + DP_MODE_CTL);
>
> if (tcphy->mode == MODE_DISCONNECT)
> tcphy_phy_deinit(tcphy);
> @@ -1002,9 +1120,35 @@ static const struct phy_ops rockchip_dp_phy_ops = {
> .owner = THIS_MODULE,
> };
>
> +static int typec_dp_phy_config(struct phy *phy, int link_rate,
> + int lanes, u8 swing, u8 pre_emp)
> +{
> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> + u8 i;
> +
> + tcphy_cfg_dp_pll(tcphy, link_rate);
> +
> + if (tcphy->mode == MODE_DFP_DP) {
> + for (i = 0; i < 4; i++)
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, i);
> + } else {
> + if (tcphy->flip) {
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 0);
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 1);
> + } else {
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 2);
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 3);
> + }
> + }
> +
> + return 0;
> +}
> +
> static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
> struct device *dev)
> {
> + int ret;
> +
> tcphy->grf_regs = syscon_regmap_lookup_by_phandle(dev->of_node,
> "rockchip,grf");
> if (IS_ERR(tcphy->grf_regs)) {
> @@ -1042,6 +1186,16 @@ static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
> return PTR_ERR(tcphy->tcphy_rst);
> }
>
> + /*
> + * check if phy_config pass from dts, if no,
> + * use default phy config value.
nit: Check if phy-config is passed from the dts, if no, use the
default phy configuration.
> + */
> + ret = of_property_read_u32_array(dev->of_node, "rockchip,phy-config",
> + (u32 *)tcphy->config, sizeof(tcphy->config) / sizeof(u32));
> + if (ret)
> + memcpy(tcphy->config, tcphy_default_config,
> + sizeof(tcphy->config));
> +
> return 0;
> }
>
> @@ -1126,6 +1280,7 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
> }
> }
>
> + tcphy->typec_phy_config = typec_dp_phy_config;
> pm_runtime_enable(dev);
>
> for_each_available_child_of_node(np, child_np) {
> --
> 2.7.4
>
Best regards,
Enric
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH RFC 03/24] arm64/dts: add switch-delay for meson mali
From: Neil Armstrong @ 2018-05-21 14:16 UTC (permalink / raw)
To: Qiang Yu, dri-devel, devicetree
Cc: Simon Shields, Marek Vasut, Connor Abbott, Andrei Paulau,
Vasily Khoruzhick, open list:ARM/Amlogic Meson..., Erico Nunes
In-Reply-To: <20180518092815.25280-4-yuq825@gmail.com>
Hi Yuq,
On 18/05/2018 11:27, Qiang Yu wrote:
> Meson mali GPU operate in high clock frequency, need
> this value be high to work in a stable state.
>
> Signed-off-by: Qiang Yu <yuq825@gmail.com>
> ---
> arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi
> index eb327664a4d8..8bed15267c9c 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi
> @@ -23,6 +23,7 @@
> "pp2", "ppmmu2";
> clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_MALI>;
> clock-names = "bus", "core";
> + switch-delay = <0xffff>;
>
> /*
> * Mali clocking is provided by two identical clock paths
>
Please CC : linux-amlogic@lists.infradead.org
to have these amlogic DT patches reviewed and applied.
Same for cover-letter to have context.
Thanks,
Neil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH RFC 04/24] arm64/dts: add switch-delay for meson mali
From: Neil Armstrong @ 2018-05-21 14:16 UTC (permalink / raw)
To: Qiang Yu, dri-devel, devicetree
Cc: Simon Shields, Marek Vasut, Connor Abbott, Andrei Paulau,
Vasily Khoruzhick, open list:ARM/Amlogic Meson..., Erico Nunes
In-Reply-To: <20180518092815.25280-5-yuq825@gmail.com>
Hi Yuq,
On 18/05/2018 11:27, Qiang Yu wrote:
> From: Andrei Paulau <7134956@gmail.com>
>
> Meson mali GPU operate in high clock frequency, need
> this value be high to work in a stable state.
>
> Signed-off-by: Andrei Paulau <7134956@gmail.com>
> ---
> arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> index 562c26a0ba33..00cd2fc1aa95 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> @@ -241,6 +241,7 @@
> "pp2", "ppmmu2";
> clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_MALI>;
> clock-names = "bus", "core";
> + switch-delay = <0xffff>;
>
> /*
> * Mali clocking is provided by two identical clock paths
>
Same this one, CC it to : linux-amlogic@lists.infradead.org
Thanks,
Neil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 03/14] dt-bindings: qcom_nandc: make nand-ecc-strength optional
From: Miquel Raynal @ 2018-05-21 14:32 UTC (permalink / raw)
To: Abhishek Sahu
Cc: Boris Brezillon, David Woodhouse, Brian Norris, Marek Vasut,
Richard Weinberger, linux-arm-msm, linux-kernel, linux-mtd,
Andy Gross, Archit Taneja, Rob Herring, Mark Rutland, devicetree
In-Reply-To: <1525350041-22995-4-git-send-email-absahu@codeaurora.org>
Hi Abhishek,
On Thu, 3 May 2018 17:50:30 +0530, Abhishek Sahu
<absahu@codeaurora.org> wrote:
> Now, nand-ecc-strength is optional. If specified in DT, then
> controller will use this ECC strength otherwise ECC strength will
> be calculated according to chip requirement and available OOB size.
Same comment as before: don't start the commit log with "now,".
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH net-next 1/7] net: dsa: qca8k: Add QCA8334 binding documentation
From: Andrew Lunn @ 2018-05-21 14:39 UTC (permalink / raw)
To: Michal Vokáč
Cc: netdev, michal.vokac, linux-kernel, devicetree, f.fainelli,
vivien.didelot, mark.rutland, robh+dt, davem
In-Reply-To: <1526909293-56377-2-git-send-email-michal.vokac@ysoft.com>
On Mon, May 21, 2018 at 03:28:07PM +0200, Michal Vokáč wrote:
Hi Michal
It is normal to have some commit message, even if it is the subject
said differently.
Andrew
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> ---
> Documentation/devicetree/bindings/net/dsa/qca8k.txt | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> index 9c67ee4..3d73cd0 100644
> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> @@ -2,7 +2,10 @@
>
> Required properties:
>
> -- compatible: should be "qca,qca8337"
> +- compatible: should be one of:
> + "qca,qca8334"
> + "qca,qca8337"
> +
> - #size-cells: must be 0
> - #address-cells: must be 1
>
> --
> 2.1.4
>
^ permalink raw reply
* Re: [PATCH net-next 3/7] net: dsa: qca8k: Enable RXMAC when bringing up a port
From: Andrew Lunn @ 2018-05-21 14:40 UTC (permalink / raw)
To: Michal Vokáč
Cc: netdev, michal.vokac, linux-kernel, devicetree, f.fainelli,
vivien.didelot, mark.rutland, robh+dt, davem
In-Reply-To: <1526909293-56377-4-git-send-email-michal.vokac@ysoft.com>
On Mon, May 21, 2018 at 03:28:09PM +0200, Michal Vokáč wrote:
> When a port is brought up/down do not enable/disable only the TXMAC
> but the RXMAC as well. This is essential for the CPU port to work.
>
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH] iommu/ipmmu-vmsa: Document R-Car V3H and E3 IPMMU DT bindings
From: Magnus Damm @ 2018-05-21 14:41 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ, Magnus Damm,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ
From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
Update the IPMMU DT binding documentation to include the compat strings
for the IPMMU devices included in the R-Car V3H and E3 SoCs.
Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
---
Developed on top of renesas-drivers-2018-05-15-v4.17-rc5
Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt | 2 ++
1 file changed, 2 insertions(+)
--- 0001/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
+++ work/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt 2018-05-21 23:37:16.370607110 +0900
@@ -20,6 +20,8 @@ Required Properties:
- "renesas,ipmmu-r8a7795" for the R8A7795 (R-Car H3) IPMMU.
- "renesas,ipmmu-r8a7796" for the R8A7796 (R-Car M3-W) IPMMU.
- "renesas,ipmmu-r8a77970" for the R8A77970 (R-Car V3M) IPMMU.
+ - "renesas,ipmmu-r8a77980" for the R8A77980 (R-Car V3H) IPMMU.
+ - "renesas,ipmmu-r8a77990" for the R8A77990 (R-Car E3) IPMMU.
- "renesas,ipmmu-r8a77995" for the R8A77995 (R-Car D3) IPMMU.
- "renesas,ipmmu-vmsa" for generic R-Car Gen2 or RZ/G1 VMSA-compatible
IPMMU.
^ permalink raw reply
* Re: [PATCH net-next 4/7] net: dsa: qca8k: Force CPU port to its highest bandwidth
From: Andrew Lunn @ 2018-05-21 14:42 UTC (permalink / raw)
To: Michal Vokáč
Cc: netdev, michal.vokac, linux-kernel, devicetree, f.fainelli,
vivien.didelot, mark.rutland, robh+dt, davem
In-Reply-To: <1526909293-56377-5-git-send-email-michal.vokac@ysoft.com>
On Mon, May 21, 2018 at 03:28:10PM +0200, Michal Vokáč wrote:
> By default autonegotiation is enabled to configure MAC on all ports.
> For the CPU port autonegotiation can not be used so we need to set
> some sensible defaults manually.
>
> This patch forces the default setting of the CPU port to 1000Mbps/full
> duplex which is the chip maximum capability.
>
> Also correct size of the bit field used to configure link speed.
>
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 02/40] iommu/sva: Bind process address spaces to devices
From: Jean-Philippe Brucker @ 2018-05-21 14:43 UTC (permalink / raw)
To: Jonathan Cameron
Cc: bharatku-gjFFaj9aHVfQT0dZR+AlfA, ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
kvm-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, xuzaibo-hv44wF8Li93QT0dZR+AlfA,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
devicetree-u79uwXL29TY76Z2rM5mHXA, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
rfranz-YGCgFSpz5w/QT0dZR+AlfA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
christian.koenig-5C7GfCeVMHo,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180517141058.00001c76-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On 17/05/18 14:10, Jonathan Cameron wrote:
> On Fri, 11 May 2018 20:06:03 +0100
> Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org> wrote:
>
>> Add bind() and unbind() operations to the IOMMU API. Bind() returns a
>> PASID that drivers can program in hardware, to let their devices access an
>> mm. This patch only adds skeletons for the device driver API, most of the
>> implementation is still missing.
>>
>> IOMMU groups with more than one device aren't supported for SVA at the
>> moment. There may be P2P traffic between devices within a group, which
>> cannot be seen by an IOMMU (note that supporting PASID doesn't add any
>> form of isolation with regard to P2P). Supporting groups would require
>> calling bind() for all bound processes every time a device is added to a
>> group, to perform sanity checks (e.g. ensure that new devices support
>> PASIDs at least as big as those already allocated in the group).
>
> Is it worth adding an explicit comment on this reasoning (or a minimal subset
> of it) at the check for the number of devices in the group?
> It's well laid out here, but might not be so obvious if someone is reading
> the code in the future.
Sure, I'll add something
Thanks,
Jean
^ permalink raw reply
* Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Jean-Philippe Brucker @ 2018-05-21 14:44 UTC (permalink / raw)
To: Jonathan Cameron
Cc: bharatku-gjFFaj9aHVfQT0dZR+AlfA, ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
kvm-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, xuzaibo-hv44wF8Li93QT0dZR+AlfA,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
devicetree-u79uwXL29TY76Z2rM5mHXA, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
rfranz-YGCgFSpz5w/QT0dZR+AlfA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
christian.koenig-5C7GfCeVMHo,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180517150516.000067ca-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On 17/05/18 15:25, Jonathan Cameron wrote:
>> +static struct io_mm *
>> +io_mm_alloc(struct iommu_domain *domain, struct device *dev,
>> + struct mm_struct *mm, unsigned long flags)
>> +{
>> + int ret;
>> + int pasid;
>> + struct io_mm *io_mm;
>> + struct iommu_sva_param *param = dev->iommu_param->sva_param;
>> +
>> + if (!domain->ops->mm_alloc || !domain->ops->mm_free)
>> + return ERR_PTR(-ENODEV);
>> +
>> + io_mm = domain->ops->mm_alloc(domain, mm, flags);
>> + if (IS_ERR(io_mm))
>> + return io_mm;
>> + if (!io_mm)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + /*
>> + * The mm must not be freed until after the driver frees the io_mm
>> + * (which may involve unpinning the CPU ASID for instance, requiring a
>> + * valid mm struct.)
>> + */
>> + mmgrab(mm);
>> +
>> + io_mm->flags = flags;
>> + io_mm->mm = mm;
>> + io_mm->release = domain->ops->mm_free;
>> + INIT_LIST_HEAD(&io_mm->devices);
>> +
>> + idr_preload(GFP_KERNEL);
>> + spin_lock(&iommu_sva_lock);
>> + pasid = idr_alloc(&iommu_pasid_idr, io_mm, param->min_pasid,
>> + param->max_pasid + 1, GFP_ATOMIC);
>
> I'd expect the IDR cleanup to be in io_mm_free as that would 'match'
> against io_mm_alloc but it's in io_mm_release just before the io_mm_free
> call, perhaps move it or am I missing something?
>
> Hmm. This is reworked in patch 5 to use call rcu to do the free. I suppose
> we may be burning an idr entry if we take a while to get round to the
> free.. If so a comment to explain this would be great.
Ok, I'll see if I can come up with some comments for both patch 3 and 5.
>> + io_mm->pasid = pasid;
>> + spin_unlock(&iommu_sva_lock);
>> + idr_preload_end();
>> +
>> + if (pasid < 0) {
>> + ret = pasid;
>> + goto err_free_mm;
>> + }
>> +
>> + /* TODO: keep track of mm. For the moment, abort. */
>
> From later patches, I can now see why we didn't init the kref
> here, but perhaps a comment would make that clear rather than
> people checking it is correctly used throughout? Actually just grab
> the comment from patch 5 and put it in this one and that will
> do the job nicely.
Ok
>> + ret = -ENOSYS;
>> + spin_lock(&iommu_sva_lock);
>> + idr_remove(&iommu_pasid_idr, io_mm->pasid);
>> + spin_unlock(&iommu_sva_lock);
>> +
>> +err_free_mm:
>> + domain->ops->mm_free(io_mm);
>
> Really minor, but you now have io_mm->release set so to keep
> this obviously the same as the io_mm_free path, perhaps call
> that rather than mm_free directly.
Yes, makes sense
>> +static void io_mm_detach_locked(struct iommu_bond *bond)
>> +{
>> + struct iommu_bond *tmp;
>> + bool detach_domain = true;
>> + struct iommu_domain *domain = bond->domain;
>> +
>> + list_for_each_entry(tmp, &domain->mm_list, domain_head) {
>> + if (tmp->io_mm == bond->io_mm && tmp->dev != bond->dev) {
>> + detach_domain = false;
>> + break;
>> + }
>> + }
>> +
>> + domain->ops->mm_detach(domain, bond->dev, bond->io_mm, detach_domain);
>> +
>
> I can't see an immediate reason to have a different order in her to the reverse of
> the attach above. So I think you should be detaching after the list_del calls.
> If there is a reason, can we have a comment so I don't ask on v10.
I don't see a reason either right now, I'll see if it can be moved
>
>> + list_del(&bond->mm_head);
>> + list_del(&bond->domain_head);
>> + list_del(&bond->dev_head);
>> + io_mm_put_locked(bond->io_mm);
>> + /* If an io_mm already exists, use it */
>> + spin_lock(&iommu_sva_lock);
>> + idr_for_each_entry(&iommu_pasid_idr, io_mm, i) {
>> + if (io_mm->mm == mm && io_mm_get_locked(io_mm)) {
>> + /* ... Unless it's already bound to this device */
>> + list_for_each_entry(tmp, &io_mm->devices, mm_head) {
>> + if (tmp->dev == dev) {
>> + bond = tmp;
>
> Using bond for this is clear in a sense, but can we not just use ret
> so it is obvious here that we are going to return -EEXIST?
> At first glance I thought you were going to carry on with this bond
> and couldn't work out why it would ever make sense to have two bonds
> between a device an an io_mm (which it doesn't!)
Yes, using ret is nicer
Thanks,
Jean
^ permalink raw reply
* Re: [PATCH v2 05/40] iommu/sva: Track mm changes with an MMU notifier
From: Jean-Philippe Brucker @ 2018-05-21 14:44 UTC (permalink / raw)
To: Jonathan Cameron
Cc: bharatku-gjFFaj9aHVfQT0dZR+AlfA, ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
kvm-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, xuzaibo-hv44wF8Li93QT0dZR+AlfA,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
devicetree-u79uwXL29TY76Z2rM5mHXA, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
rfranz-YGCgFSpz5w/QT0dZR+AlfA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
christian.koenig-5C7GfCeVMHo,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180517152514.00004247-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On 17/05/18 15:25, Jonathan Cameron wrote:
>> + * already have been removed from the list. Check is someone is
>
> Check if someone...
Ok
Thanks,
Jean
^ permalink raw reply
* [PATCH v4 0/3] arm64: dts: Draak: Enable video inputs and VIN4
From: Jacopo Mondi @ 2018-05-21 14:45 UTC (permalink / raw)
To: niklas.soderlund, laurent.pinchart, horms, geert, magnus.damm,
robh+dt
Cc: Jacopo Mondi, linux-renesas-soc, devicetree, linux-arm-kernel,
linux-kernel
Hello,
this series enables HDMI, CVBS and VIN4 for R8A77995 Draak board.
Compared to v3, the analog video decoder ADV7180 is now described in bindings
with the compatible string "adi,adv7180cp" and its port nodes definition
has been res-structured according to the chip bindings as reported by
Laurent on v3.
Bindings have been copi^W^Winspired from Gen2 Gose board, the only Gen2 board
using the "adv7180cp" compatible string.
Example of how to switch to HDMI input is still available here:
git://jmondi.org/linux d3/media-master/salvator-x-dts_csi2/d3-hdmi
And the series still depends on
[PATCH v3 0/9] rcar-vin: Add support for parallel input on Gen3
Thanks
j
Jacopo Mondi (3):
dt-bindings: media: rcar-vin: Add R8A77995 support
arm64: dts: renesas: draak: Describe CVBS input
arm64: dts: renesas: draak: Describe HDMI input
v3 -> v4:
- Use 'adi,adv7180cp' in [3/3] as detailed in the commit message
- Add Laurent's R-b tag to [2/3]
v2 -> v3:
- Add comment to CVBS and HDMI inputs
- Add missing Signed-off-by to [2/3]
- Add Niklas' tag
.../devicetree/bindings/media/rcar_vin.txt | 1 +
arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 117 +++++++++++++++++++++
2 files changed, 118 insertions(+)
--
2.7.4
^ permalink raw reply
* [PATCH v4 1/3] dt-bindings: media: rcar-vin: Add R8A77995 support
From: Jacopo Mondi @ 2018-05-21 14:45 UTC (permalink / raw)
To: niklas.soderlund, laurent.pinchart, horms, geert, magnus.damm,
robh+dt
Cc: Jacopo Mondi, linux-renesas-soc, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <1526913942-15426-1-git-send-email-jacopo+renesas@jmondi.org>
Add compatible string for R-Car D3 R8A7795 to list of SoCs supported by
rcar-vin driver.
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
---
Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index a19517e1..5c6f2a7 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -22,6 +22,7 @@ on Gen3 platforms to a CSI-2 receiver.
- "renesas,vin-r8a7795" for the R8A7795 device
- "renesas,vin-r8a7796" for the R8A7796 device
- "renesas,vin-r8a77970" for the R8A77970 device
+ - "renesas,vin-r8a77995" for the R8A77995 device
- "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
device.
--
2.7.4
^ permalink raw reply related
* [PATCH v4 2/3] arm64: dts: renesas: draak: Describe CVBS input
From: Jacopo Mondi @ 2018-05-21 14:45 UTC (permalink / raw)
To: niklas.soderlund, laurent.pinchart, horms, geert, magnus.damm,
robh+dt
Cc: Jacopo Mondi, linux-renesas-soc, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <1526913942-15426-1-git-send-email-jacopo+renesas@jmondi.org>
Describe CVBS video input through analog video decoder ADV7180
connected to video input interface VIN4.
The video input signal path is shared with HDMI video input, and
selected by on-board switches SW-53 and SW-54 with CVBS input selected
by the default switches configuration.
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
v3 -> v4:
- Use 'adi,adv7180cp' compatible string in place of just 'adi,adv7180'
as suggested by Laurent.
- Re-structure CVBS input ports definition according to adv7180cp
bindings: add port@0 for composite input, add port@3 for parallel
video output.
- Change node label to 'composite-in' as in Gose board bindings.
v2 -> v3:
- Add comment to describe the shared input video path.
- Add my SoB and Niklas' R-b tags.
---
arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 ++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index 9d73de8..ad59032 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -59,6 +59,16 @@
};
};
+ composite-in {
+ compatible = "composite-video-connector";
+
+ port {
+ composite_con_in: endpoint {
+ remote-endpoint = <&adv7180_in>;
+ };
+ };
+ };
+
memory@48000000 {
device_type = "memory";
/* first 128MB is reserved for secure area. */
@@ -142,6 +152,11 @@
groups = "usb0";
function = "usb0";
};
+
+ vin4_pins_cvbs: vin4 {
+ groups = "vin4_data8", "vin4_sync", "vin4_clk";
+ function = "vin4";
+ };
};
&i2c0 {
@@ -154,6 +169,39 @@
reg = <0x50>;
pagesize = <8>;
};
+
+ composite-in@20 {
+ compatible = "adi,adv7180cp";
+ reg = <0x20>;
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ adv7180_in: endpoint {
+ remote-endpoint = <&composite_con_in>;
+ };
+ };
+
+ port@3 {
+ reg = <3>;
+
+ /*
+ * The VIN4 video input path is shared between
+ * CVBS and HDMI inputs through SW[49-53]
+ * switches.
+ *
+ * CVBS is the default selection, link it to
+ * VIN4 here.
+ */
+ adv7180_out: endpoint {
+ remote-endpoint = <&vin4_in>;
+ };
+ };
+ };
+ };
};
&i2c1 {
@@ -246,3 +294,23 @@
timeout-sec = <60>;
status = "okay";
};
+
+&vin4 {
+ pinctrl-0 = <&vin4_pins_cvbs>;
+ pinctrl-names = "default";
+
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ vin4_in: endpoint {
+ remote-endpoint = <&adv7180_out>;
+ };
+ };
+ };
+};
--
2.7.4
^ permalink raw reply related
* [PATCH v4 3/3] arm64: dts: renesas: draak: Describe HDMI input
From: Jacopo Mondi @ 2018-05-21 14:45 UTC (permalink / raw)
To: niklas.soderlund, laurent.pinchart, horms, geert, magnus.damm,
robh+dt
Cc: Jacopo Mondi, linux-renesas-soc, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <1526913942-15426-1-git-send-email-jacopo+renesas@jmondi.org>
Describe HDMI input connector and ADV7612 HDMI decoder installed on
R-Car Gen3 Draak board.
The video signal routing to the HDMI decoder to the video input interface
VIN4 is multiplexed with CVBS input path, and enabled/disabled through
on-board switches SW-49, SW-50, SW-51 and SW-52.
As the default board switches configuration connects CVBS input to VIN4,
leave the HDMI decoder unconnected in DTS.
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
v3 -> v4:
- Add Laurent's R-b tag.
v2 -> v3:
- Add comment on HDMI output port about the shared CVBS/HDMI video path.
- Add Niklas' R-b tag.
---
arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 49 ++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index ad59032..1a474f94 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -69,6 +69,17 @@
};
};
+ hdmi-in {
+ compatible = "hdmi-connector";
+ type = "a";
+
+ port {
+ hdmi_con_in: endpoint {
+ remote-endpoint = <&adv7612_in>;
+ };
+ };
+ };
+
memory@48000000 {
device_type = "memory";
/* first 128MB is reserved for secure area. */
@@ -201,6 +212,44 @@
};
};
};
+
+ };
+
+ hdmi-decoder@4c {
+ compatible = "adi,adv7612";
+ reg = <0x4c>;
+ default-input = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ adv7612_in: endpoint {
+ remote-endpoint = <&hdmi_con_in>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+
+ /*
+ * The VIN4 video input path is shared between
+ * CVBS and HDMI inputs through SW[49-53]
+ * switches.
+ *
+ * CVBS is the default selection, leave HDMI
+ * not connected here.
+ */
+ adv7612_out: endpoint {
+ pclk-sample = <0>;
+ hsync-active = <0>;
+ vsync-active = <0>;
+ };
+ };
+ };
};
};
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 5/7] net: dsa: qca8k: Allow overwriting CPU port setting
From: Andrew Lunn @ 2018-05-21 14:46 UTC (permalink / raw)
To: Michal Vokáč
Cc: netdev, michal.vokac, linux-kernel, devicetree, f.fainelli,
vivien.didelot, mark.rutland, robh+dt, davem
In-Reply-To: <1526909293-56377-6-git-send-email-michal.vokac@ysoft.com>
On Mon, May 21, 2018 at 03:28:11PM +0200, Michal Vokáč wrote:
> Implement adjust_link function that allows to overwrite default CPU port
> setting using fixed-link device tree subnode.
>
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 1/7] net: dsa: qca8k: Add QCA8334 binding documentation
From: Andrew Lunn @ 2018-05-21 14:47 UTC (permalink / raw)
To: Michal Vokáč
Cc: netdev, michal.vokac, linux-kernel, devicetree, f.fainelli,
vivien.didelot, mark.rutland, robh+dt, davem
In-Reply-To: <1526909293-56377-2-git-send-email-michal.vokac@ysoft.com>
On Mon, May 21, 2018 at 03:28:07PM +0200, Michal Vokáč wrote:
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
Hi Michal
It would be good to document that fixed-link can be used.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 6/7] net: dsa: qca8k: Replace GPL boilerplate by SPDX
From: Andrew Lunn @ 2018-05-21 14:47 UTC (permalink / raw)
To: Michal Vokáč
Cc: netdev, michal.vokac, linux-kernel, devicetree, f.fainelli,
vivien.didelot, mark.rutland, robh+dt, davem
In-Reply-To: <1526909293-56377-7-git-send-email-michal.vokac@ysoft.com>
On Mon, May 21, 2018 at 03:28:12PM +0200, Michal Vokáč wrote:
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 07/40] iommu: Add a page fault handler
From: Jean-Philippe Brucker @ 2018-05-21 14:48 UTC (permalink / raw)
To: Jonathan Cameron
Cc: bharatku-gjFFaj9aHVfQT0dZR+AlfA, ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
kvm-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, xuzaibo-hv44wF8Li93QT0dZR+AlfA,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
devicetree-u79uwXL29TY76Z2rM5mHXA, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
rfranz-YGCgFSpz5w/QT0dZR+AlfA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
christian.koenig-5C7GfCeVMHo,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180517162555.00002bd3-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On 17/05/18 16:25, Jonathan Cameron wrote:
> On Fri, 11 May 2018 20:06:08 +0100
> Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org> wrote:
>
>> Some systems allow devices to handle I/O Page Faults in the core mm. For
>> example systems implementing the PCI PRI extension or Arm SMMU stall
>> model. Infrastructure for reporting these recoverable page faults was
>> recently added to the IOMMU core for SVA virtualisation. Add a page fault
>> handler for host SVA.
>>
>> IOMMU driver can now instantiate several fault workqueues and link them to
>> IOPF-capable devices. Drivers can choose between a single global
>> workqueue, one per IOMMU device, one per low-level fault queue, one per
>> domain, etc.
>>
>> When it receives a fault event, supposedly in an IRQ handler, the IOMMU
>> driver reports the fault using iommu_report_device_fault(), which calls
>> the registered handler. The page fault handler then calls the mm fault
>> handler, and reports either success or failure with iommu_page_response().
>> When the handler succeeded, the IOMMU retries the access.
>>
>> The iopf_param pointer could be embedded into iommu_fault_param. But
>> putting iopf_param into the iommu_param structure allows us not to care
>> about ordering between calls to iopf_queue_add_device() and
>> iommu_register_device_fault_handler().
>>
>> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>
>
> Hi Jean-Phillipe,
>
> One question below on how we would end up with partial faults left when
> doing the queue remove. Code looks fine, but I'm not seeing how that
> would happen without buggy hardware... + we presumably have to rely on
> the hardware timing out on that request or it's dead anyway...
>> +/**
>> + * iopf_queue_remove_device - Remove producer from fault queue
>> + * @dev: device to remove
>> + *
>> + * Caller makes sure that no more fault is reported for this device, and no more
>> + * flush is scheduled for this device.
>> + *
>> + * Note: safe to call unconditionally on a cleanup path, even if the device
>> + * isn't registered to any IOPF queue.
>> + *
>> + * Return 0 if the device was attached to the IOPF queue
>> + */
>> +int iopf_queue_remove_device(struct device *dev)
>> +{
>> + struct iopf_context *fault, *next;
>> + struct iopf_device_param *iopf_param;
>> + struct iommu_param *param = dev->iommu_param;
>> +
>> + if (!param)
>> + return -EINVAL;
>> +
>> + mutex_lock(¶m->lock);
>> + iopf_param = param->iopf_param;
>> + if (iopf_param) {
>> + refcount_dec(&iopf_param->queue->refs);
>> + param->iopf_param = NULL;
>> + }
>> + mutex_unlock(¶m->lock);
>> + if (!iopf_param)
>> + return -EINVAL;
>> +
>> + list_for_each_entry_safe(fault, next, &iopf_param->partial, head)
>> + kfree(fault);
>> +
>
> Why would we end up here with partials still in the list? Buggy hardware?
Buggy hardware is one possibility. There also is the nasty case of PRI
queue overflow. If the PRI queue is full, then the SMMU discards new
Page Requests from the device. It may discard a 'last' PR of a group
that is already in iopf_param->partial. This group will never be freed
until this cleanup.
The spec dismisses PRIq overflows because the OS is supposed to allocate
PRI credits to devices such that they can't send more than the PRI queue
size. This isn't possible in Linux because we don't know exactly how
many PRI-capable devices will share a queue (the upper limit is 2**32,
and devices may be hotplugged well after we allocated the PRI queue). So
PRIq overflow is possible.
Ideally when detecting a PRIq overflow we need to immediately remove all
partial faults of all devices sharing this queue. Since I can't easily
test that case (my device doesn't do PRGs) and it's complicated code, I
left it as TODO in patch 39, and this is our only chance to free
orphaned page requests.
>> +void iopf_queue_free(struct iopf_queue *queue)
>> +{
>> +
>
> Nitpick : Blank line here doesn't add anything.
Ok
Thanks,
Jean
^ permalink raw reply
* Re: [PATCH net-next 7/7] net: dsa: qca8k: Remove rudundant parentheses
From: Andrew Lunn @ 2018-05-21 14:48 UTC (permalink / raw)
To: Michal Vokáč
Cc: netdev, michal.vokac, linux-kernel, devicetree, f.fainelli,
vivien.didelot, mark.rutland, robh+dt, davem
In-Reply-To: <1526909293-56377-8-git-send-email-michal.vokac@ysoft.com>
On Mon, May 21, 2018 at 03:28:13PM +0200, Michal Vokáč wrote:
> Fix warning reported by checkpatch.
>
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 07/40] iommu: Add a page fault handler
From: Jean-Philippe Brucker @ 2018-05-21 14:49 UTC (permalink / raw)
To: Jacob Pan
Cc: bharatku-gjFFaj9aHVfQT0dZR+AlfA, ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
kvm-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, xuzaibo-hv44wF8Li93QT0dZR+AlfA,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
devicetree-u79uwXL29TY76Z2rM5mHXA, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
rfranz-YGCgFSpz5w/QT0dZR+AlfA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
christian.koenig-5C7GfCeVMHo,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180518110434.150a0e64@jacob-builder>
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(¶m->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.
>> +
>> + 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?
> 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.
Thanks,
Jean
^ permalink raw reply
* Re: [PATCH v2 17/40] iommu/arm-smmu-v3: Link domains and devices
From: Jean-Philippe Brucker @ 2018-05-21 14:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: bharatku-gjFFaj9aHVfQT0dZR+AlfA, ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
kvm-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, xuzaibo-hv44wF8Li93QT0dZR+AlfA,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
devicetree-u79uwXL29TY76Z2rM5mHXA, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
rfranz-YGCgFSpz5w/QT0dZR+AlfA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
christian.koenig-5C7GfCeVMHo,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180517170748.00004927-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On 17/05/18 17:07, Jonathan Cameron wrote:
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -595,6 +595,11 @@ struct arm_smmu_device {
>> struct arm_smmu_master_data {
>> struct arm_smmu_device *smmu;
>> struct arm_smmu_strtab_ent ste;
>> +
>> + struct arm_smmu_domain *domain;
>> + struct list_head list; /* domain->devices */
>
> More meaningful name perhaps to avoid the need for the comment?
Sure
Thanks,
Jean
^ permalink raw reply
* Re: [PATCH v4 2/3] arm64: dts: renesas: draak: Describe CVBS input
From: Laurent Pinchart @ 2018-05-21 14:50 UTC (permalink / raw)
To: Jacopo Mondi
Cc: niklas.soderlund, horms, geert, magnus.damm, robh+dt,
linux-renesas-soc, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1526913942-15426-3-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thank you for the patch.
On Monday, 21 May 2018 17:45:41 EEST Jacopo Mondi wrote:
> Describe CVBS video input through analog video decoder ADV7180
> connected to video input interface VIN4.
>
> The video input signal path is shared with HDMI video input, and
> selected by on-board switches SW-53 and SW-54 with CVBS input selected
> by the default switches configuration.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> v3 -> v4:
> - Use 'adi,adv7180cp' compatible string in place of just 'adi,adv7180'
> as suggested by Laurent.
> - Re-structure CVBS input ports definition according to adv7180cp
> bindings: add port@0 for composite input, add port@3 for parallel
> video output.
> - Change node label to 'composite-in' as in Gose board bindings.
>
> v2 -> v3:
> - Add comment to describe the shared input video path.
> - Add my SoB and Niklas' R-b tags.
> ---
> arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68
> ++++++++++++++++++++++++++ 1 file changed, 68 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index 9d73de8..ad59032
> 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> @@ -59,6 +59,16 @@
> };
> };
>
> + composite-in {
> + compatible = "composite-video-connector";
> +
> + port {
> + composite_con_in: endpoint {
> + remote-endpoint = <&adv7180_in>;
> + };
> + };
> + };
> +
> memory@48000000 {
> device_type = "memory";
> /* first 128MB is reserved for secure area. */
> @@ -142,6 +152,11 @@
> groups = "usb0";
> function = "usb0";
> };
> +
> + vin4_pins_cvbs: vin4 {
> + groups = "vin4_data8", "vin4_sync", "vin4_clk";
> + function = "vin4";
> + };
> };
>
> &i2c0 {
> @@ -154,6 +169,39 @@
> reg = <0x50>;
> pagesize = <8>;
> };
> +
> + composite-in@20 {
> + compatible = "adi,adv7180cp";
> + reg = <0x20>;
> +
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + adv7180_in: endpoint {
> + remote-endpoint = <&composite_con_in>;
> + };
> + };
> +
> + port@3 {
> + reg = <3>;
> +
> + /*
> + * The VIN4 video input path is shared between
> + * CVBS and HDMI inputs through SW[49-53]
> + * switches.
> + *
> + * CVBS is the default selection, link it to
> + * VIN4 here.
> + */
> + adv7180_out: endpoint {
> + remote-endpoint = <&vin4_in>;
> + };
> + };
> + };
> + };
> };
>
> &i2c1 {
> @@ -246,3 +294,23 @@
> timeout-sec = <60>;
> status = "okay";
> };
> +
> +&vin4 {
> + pinctrl-0 = <&vin4_pins_cvbs>;
> + pinctrl-names = "default";
> +
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + vin4_in: endpoint {
> + remote-endpoint = <&adv7180_out>;
> + };
> + };
> + };
> +};
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH V7] ARM: dts: da850-evm: Enable LCD and Backlight
From: Sekhar Nori @ 2018-05-21 14:51 UTC (permalink / raw)
To: Adam Ford, linux-arm-kernel; +Cc: devicetree, khilman
In-Reply-To: <20180518203335.13878-1-aford173@gmail.com>
On Saturday 19 May 2018 02:03 AM, Adam Ford wrote:
> When using the board files the LCD works, but not with the DT.
> This adds enables the original da850-evm to work with the same
> LCD in device tree mode.
>
> The EVM has a gpio for the regulator and a PWM for dimming the
> backlight. The LCD and the vpif display pins are mutually
> exclusive, so if using the LCD, do not load the vpif driver.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> index 0e82bb988fde..a76c2ddfd23e 100644
> --- a/arch/arm/boot/dts/da850-evm.dts
> +++ b/arch/arm/boot/dts/da850-evm.dts
> @@ -27,6 +27,60 @@
> spi0 = &spi1;
> };
>
> + backlight: backlight-pwm {
> + pinctrl-names = "default";
> + pinctrl-0 = <&ecap2_pins>;
> + power-supply = <&backlight_lcd>;
> + compatible = "pwm-backlight";
> + pwms = <&ecap2 0 50000 0>;
It will be nice to add a comment here: "The PWM here corresponds to
production hardware. The schematic needs to be 1015171 (15 March 2010),
Rev A or newer."
> + brightness-levels = <0 10 20 30 40 50 60 70 80 90 99>;
> + default-brightness-level = <7>;
> + };
> +
> + panel {
> + compatible = "ti,tilcdc,panel";
> + pinctrl-names = "default";
> + pinctrl-0 = <&lcd_pins>;
> + /*
> + * The vpif and the LCD are mutually exclusive.
> + * To enable VPIF, change the status below to 'disabled' then
> + * then change the status of the vpif below to 'okay'
> + */
This results in checkpatch warning:
[PATCH V7] ARM: dts: da850-evm: Enable LCD and Backlight.eml:153: WARNING: Block comments should align the * on each line
[PATCH V7] ARM: dts: da850-evm: Enable LCD and Backlight.eml:239: WARNING: Block comments should align the * on each line
> &vpif {
> pinctrl-names = "default";
> pinctrl-0 = <&vpif_capture_pins>, <&vpif_display_pins>;
> - status = "okay";
> + /*
> + * The vpif and the LCD are mutually exclusive.
> + * To enable VPIF, disable the ti,tilcdc,panel then
> + * changed the status below to 'okay'
> + */
> + status = "disabled";
Are you able to see VPIF is disabled after this? Trying your patch, I
still see VPIF probing[1]. Also, only VPIF display has a conflict with
LCD, correct (capture should be fine)?
Thanks,
Sekhar
[1]
[ 34.739314] adv7343 0-002a: chip found @ 0x54 (DaVinci I2C adapter)
[ 34.881422] vpif_display vpif_display: Pixel details: Width = 720,Height = 480
[ 34.881506] vpif_display vpif_display: channel=5bf2d970,channel->video_dev=5bf2d970
[ 34.883374] vpif_display vpif_display: Pixel details: Width = 720,Height = 480
[ 34.883450] vpif_display vpif_display: channel=16041996,channel->video_dev=16041996
[ 35.370777] tvp514x 0-005d: tvp514x 0-005d decoder driver registered !!
[ 35.470088] vpif_capture vpif_capture: registered sub device tvp514x-0
[ 35.827492] tvp514x 0-005c: tvp514x 0-005c decoder driver registered !!
[ 35.866031] vpif_capture vpif_capture: registered sub device tvp514x-1
[ 35.953752] vpif_capture vpif_capture: VPIF capture driver initialized
# ls /dev/video
video0 video1 video2 video3
^ permalink raw reply
* Re: [PATCH v2 13/40] vfio: Add support for Shared Virtual Addressing
From: Jean-Philippe Brucker @ 2018-05-21 14:51 UTC (permalink / raw)
To: Jonathan Cameron
Cc: bharatku-gjFFaj9aHVfQT0dZR+AlfA, ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
kvm-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, xuzaibo-hv44wF8Li93QT0dZR+AlfA,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
devicetree-u79uwXL29TY76Z2rM5mHXA, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
rfranz-YGCgFSpz5w/QT0dZR+AlfA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
christian.koenig-5C7GfCeVMHo,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180517165845.00000cc9-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On 17/05/18 16:58, Jonathan Cameron wrote:
>> +static int vfio_iommu_bind_group(struct vfio_iommu *iommu,
>> + struct vfio_group *group,
>> + struct vfio_mm *vfio_mm)
>> +{
>> + int ret;
>> + bool enabled_sva = false;
>> + struct vfio_iommu_sva_bind_data data = {
>> + .vfio_mm = vfio_mm,
>> + .iommu = iommu,
>> + .count = 0,
>> + };
>> +
>> + if (!group->sva_enabled) {
>> + ret = iommu_group_for_each_dev(group->iommu_group, NULL,
>> + vfio_iommu_sva_init);
>> + if (ret)
>> + return ret;
>> +
>> + group->sva_enabled = enabled_sva = true;
>> + }
>> +
>> + ret = iommu_group_for_each_dev(group->iommu_group, &data,
>> + vfio_iommu_sva_bind_dev);
>> + if (ret && data.count > 1)
>
> Are we safe to run this even if data.count == 1? I assume that at
> that point we would always not have an iommu domain associated with the
> device so the initial check would error out.
If data.count == 1, then the first bind didn't succeed. But it's not
necessarily a domain missing, failure to bind may come from various
places. If this vfio_mm was already bound to another device then it
contains a valid PASID and it's safe to call unbind(). Otherwise we call
it with PASID -1 (VFIO_INVALID_PASID) and that's a bit of a grey area.
-1 is currently invalid everywhere, but in the future someone might
implement 32 bits of PASIDs, in which case a bond between this dev and
PASID -1 might exist. But I think it's safe for now, and whoever
redefines VFIO_INVALID_PASID when such implementation appears will also
fix this case.
> Just be nice to get rid of the special casing in this error path as then
> could just do it all under if (ret) and make it visually clearer these
> are different aspects of the same error path.
[...]
>> static long vfio_iommu_type1_ioctl(void *iommu_data,
>> unsigned int cmd, unsigned long arg)
>> {
>> @@ -1728,6 +2097,44 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>>
>> return copy_to_user((void __user *)arg, &unmap, minsz) ?
>> -EFAULT : 0;
>> +
>> + } else if (cmd == VFIO_IOMMU_BIND) {
>> + struct vfio_iommu_type1_bind bind;
>> +
>> + minsz = offsetofend(struct vfio_iommu_type1_bind, flags);
>> +
>> + if (copy_from_user(&bind, (void __user *)arg, minsz))
>> + return -EFAULT;
>> +
>> + if (bind.argsz < minsz)
>> + return -EINVAL;
>> +
>> + switch (bind.flags) {
>> + case VFIO_IOMMU_BIND_PROCESS:
>> + return vfio_iommu_type1_bind_process(iommu, (void *)arg,
>> + &bind);
>
> Can we combine these two blocks given it is only this case statement that is different?
That would be nicer, though I don't know yet what's needed for vSVA (by
Yi Liu on Cc), which will add statements to the switches.
Thanks,
Jean
>
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + } else if (cmd == VFIO_IOMMU_UNBIND) {
>> + struct vfio_iommu_type1_bind bind;
>> +
>> + minsz = offsetofend(struct vfio_iommu_type1_bind, flags);
>> +
>> + if (copy_from_user(&bind, (void __user *)arg, minsz))
>> + return -EFAULT;
>> +
>> + if (bind.argsz < minsz)
>> + return -EINVAL;
>> +
>> + switch (bind.flags) {
>> + case VFIO_IOMMU_BIND_PROCESS:
>> + return vfio_iommu_type1_unbind_process(iommu, (void *)arg,
>> + &bind);
>> + default:
>> + return -EINVAL;
>> + }
>> }
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox