On Thu, Sep 28, 2023 at 8:19 PM Jeff Johnson <quic_jjohnson@xxxxxxxxxxx> wrote:
>
> On 9/28/2023 2:20 AM, Bartosz Golaszewski wrote:
> > +void *qcom_scm_mem_alloc(size_t size, gfp_t gfp)
> > +{
> > + struct qcom_scm_mem_chunk *chunk;
> > + unsigned long vaddr;
>
> there are places below where you unnecessarily typecast this to its
> given type
>
Ah, it's a leftover from when this variable was of type void *. Thanks.
> > + int ret;
> > +
> > + if (!size)
> > + return ZERO_SIZE_PTR;
> > +
> > + size = roundup(size, 1 << PAGE_SHIFT);
> > +
> > + chunk = kzalloc(sizeof(*chunk), gfp);
> > + if (!chunk)
> > + return NULL;
> > +
> > + vaddr = gen_pool_alloc(qcom_scm_mem.pool, size);
> > + if (!vaddr) {
> > + kfree(chunk);
> > + return NULL;
> > + }
> > +
> > + chunk->paddr = gen_pool_virt_to_phys(qcom_scm_mem.pool,
> > + (unsigned long)vaddr);
>
> unnecessary typecast?
>
> > + chunk->size = size;
> > +
> > + scoped_guard(spinlock_irqsave, &qcom_scm_mem.lock) {
>
> my first exposure to this infrastructure..very cool now that I've
> wrapped my head around it! This helped for those also new to this:
> https://lwn.net/Articles/934679/
>
It's amazing but be careful with it. I used it wrong in one place and
got yelled at by Linus Torvalds personally already. :)
Bartosz
> > + ret = radix_tree_insert(&qcom_scm_mem.chunks, vaddr, chunk);
> > + if (ret) {
> > + gen_pool_free(qcom_scm_mem.pool, (unsigned long)vaddr,
>
> unnecessary typecast?
>
> > + chunk->size);
> > + kfree(chunk);
> > + return NULL;
> > + }
> > + }
> > +
> > + return (void *)vaddr;
> > +}
>
