[PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ

[moudy.ho]

On Fri, 2022-03-18 at 07:31 +0800, Miles Chen wrote:
> Hi Moudy,
> 
> >  #define MT2701_MUTEX0_MOD0			0x2c
> >  #define MT2701_MUTEX0_SOF0			0x30
> > @@ -180,6 +184,9 @@ struct mtk_mutex_ctx {
> >  	void __iomem			*regs;
> >  	struct mtk_mutex		mutex[10];
> >  	const struct mtk_mutex_data	*data;
> > +	phys_addr_t			addr;
> > +	struct cmdq_client_reg		cmdq_reg;
> > +	bool				has_gce_client_reg;
> >  };
> > 
> 
> ...snip...
> 
> >  static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] =
> > {
> > @@ -638,6 +645,29 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
> >  }
> >  EXPORT_SYMBOL_GPL(mtk_mutex_enable);
> >  
> > +void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
> > +{
> > +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > +						 mutex[mutex->id]);
> > +	struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
> > +
> > +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> > +
> > +	if (!mtx->has_gce_client_reg) {
> > +		dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been
> > set in dts");
> > +		return;
> > +	}
> > +
> > +	cmdq_pkt_write_mask(cmdq_pkt, mtx->cmdq_reg.subsys,
> > +			    mtx->addr + DISP_REG_MUTEX_EN(mutex->id),
> > +			    MTK_MUTEX_ENABLE, MTK_MUTEX_ENABLE);
> 
> We do not check if mtx->addr is valid.
> 
> > +#else
> > +	dev_dbg(mtx->dev, "Not support for enable MUTEX by CMDQ");
> > +#endif
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mutex_enable_by_cmdq);
> > +
> >  void mtk_mutex_disable(struct mtk_mutex *mutex)
> >  {
> >  	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > @@ -676,7 +706,7 @@ static int mtk_mutex_probe(struct
> > platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> >  	struct mtk_mutex_ctx *mtx;
> > -	struct resource *regs;
> > +	struct resource *regs, addr;
> >  	int i;
> >  
> >  	mtx = devm_kzalloc(dev, sizeof(*mtx), GFP_KERNEL);
> > @@ -697,6 +727,19 @@ static int mtk_mutex_probe(struct
> > platform_device *pdev)
> >  		}
> >  	}
> >  
> > +	if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
> > +		mtx->addr = 0L;
> 
> addr is phys_addr_t, which means that it holds a physical address.
> 0 may be a valid address in some platforms, so I think
> we should not set mtx->addr = 0L when of_address_to_resource fails.
> Maybe probe fail by returning dev_err_probe()?
> 
> Thanks,
> Miles

Hi Miles,

This does lead to the situation you mentioned, and sorry for the
deficient dts parsing flow. Maybe we should follow "gce-client" usage
and add a check condition when needed to avoid compatibility issues?

Thanks,
Moudy