Re: [PATCH v2, 3/3] mt8183: emi: add bandwidth driver support

[Xi Chen <xixi.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>]

On Tue, 2019-06-18 at 17:40 +0800, Hsin-Yi Wang wrote:
> On Fri, May 24, 2019 at 3:54 AM Xi Chen <xixi.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> 
> > +struct mtk_emi {
> > +       struct device *dev;
> > +       void __iomem *cen_emi_base;
> > +       void __iomem *chn_emi_base[MAX_CH];
> > +       void __iomem *emi_mpu_base;
> > +
> > +       struct timer_list emi_bw_timer;
> > +       struct timeval old_tv;
> > +
> > +       unsigned long long emi_bw_array[EMI_BW_ARRAY_SIZE];
> > +       int emi_bw_cur_idx;
> > +};
> > +
> > +static unsigned long long emi_get_max_bw_in_last_array(struct device *dev,
> > +       unsigned long long arr[], unsigned int size)
> > +{
> > +       unsigned int i = 0;
> > +       unsigned long long max = arr[0];
> > +
> > +       while (i < size) {
> > +               if (arr[i] > max)
> > +                       max = arr[i];
> > +               ++i;
> > +       }
> > +       return max;
> > +}
> Would it better that if we store max element in mtk_emi struct{}, so
> that we don't need to scan entire array everytime to find max? Though
> array size only 67.
> This max element can be update in emi_update_bw_array().
	Yes, a good idea. Sorry, v3 doesn't contain the patch. I will try to
improve the codes on v4.
> 
> > +unsigned long long mtk_emi_get_max_bw(struct device *dev)
> > +{
> > +       struct mtk_emi *emi;
> > +
> > +       if (!dev)
> > +               return 0;
> > +
> > +       emi = dev_get_drvdata(dev);
> > +       return emi_get_max_bw_in_last_array(dev,
> > +               emi->emi_bw_array, ARRAY_SIZE(emi->emi_bw_array));
> > +}
> > +EXPORT_SYMBOL(mtk_emi_get_max_bw);
> > +
> > +static void emi_update_bw_array(struct device *dev, unsigned int val)
> > +{
> > +       struct mtk_emi *emi = dev_get_drvdata(dev);
> > +
> > +       if (emi->emi_bw_cur_idx == emi->EMI_BW_ARRAY_SIZE) {
> > +               /* remove the first array element */
> > +               memmove(emi->emi_bw_array, emi->emi_bw_array + 1,
> > +                       sizeof(unsigned long long) * (emi->EMI_BW_ARRAY_SIZE - 1));
> > +               emi->emi_bw_array[emi->EMI_BW_ARRAY_SIZE - 1] = val;
> > +       } else
> > +               emi->emi_bw_array[emi->emi_bw_cur_idx++] = val;
> > +}
> > +
> Is the order of the emi_bw_array important?
> If not, update latest element don't need to be at the end, so we don't
> need O(n) shift everytime when inserting new element.
> If the order is important, we can also do insert in O(1), by adding a
> pointer that points to the oldest element in array. When insert a new
> element, replace oldest element with latest one and move the pointer
> one space backward.
  I understand your idea, but emi driver stores the last 67ms bandwidth,
and just the last 67ms, not all the time. So, we will move the array
elements every 1ms.
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek